From 1097cc41737d5c5a3d244cdb7c4cb01c3b8c3c87 Mon Sep 17 00:00:00 2001 From: Saem Ghani Date: Fri, 29 Jan 2021 06:15:35 -0800 Subject: [PATCH] nimsuggest prioritize non-deprecated suggestions (#16816) * penalizes the quality score of deprecated symbols * uses quality more pervasively in order to reflect deprecation impact * impacts both sug and con additional notes: * linux i386 CI was failing * this is because the suggested results differ slightly in their sort * 64 bit tables.getOrDefault:441 was returned, while 32 bit returned 422 * for now simply removing the last line is good enough --- compiler/suggest.nim | 42 ++++++++++++++++++---------- nimsuggest/tests/tsug_regression.nim | 16 ++++++----- nimsuggest/tests/ttype_decl.nim | 6 ++-- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/compiler/suggest.nim b/compiler/suggest.nim index 49a7d96747..cad776015e 100644 --- a/compiler/suggest.nim +++ b/compiler/suggest.nim @@ -258,17 +258,22 @@ proc fieldVisible*(c: PContext, f: PSym): bool {.inline.} = result = true break -proc suggestField(c: PContext, s: PSym; f: PNode; info: TLineInfo; outputs: var Suggestions) = - var pm: PrefixMatch - if filterSym(s, f, pm) and fieldVisible(c, s): - outputs.add(symToSuggest(c.graph, s, isLocal=true, ideSug, info, 100, pm, c.inTypeContext > 0, 0)) - proc getQuality(s: PSym): range[0..100] = + result = 100 if s.typ != nil and s.typ.len > 1: var exp = s.typ[1].skipTypes({tyGenericInst, tyVar, tyLent, tyAlias, tySink}) if exp.kind == tyVarargs: exp = elemType(exp) - if exp.kind in {tyUntyped, tyTyped, tyGenericParam, tyAnything}: return 50 - return 100 + if exp.kind in {tyUntyped, tyTyped, tyGenericParam, tyAnything}: result = 50 + + # penalize deprecated symbols + if sfDeprecated in s.flags: + result = result - 5 + +proc suggestField(c: PContext, s: PSym; f: PNode; info: TLineInfo; outputs: var Suggestions) = + var pm: PrefixMatch + if filterSym(s, f, pm) and fieldVisible(c, s): + outputs.add(symToSuggest(c.graph, s, isLocal=true, ideSug, info, + s.getQuality, pm, c.inTypeContext > 0, 0)) template wholeSymTab(cond, section: untyped) {.dirty.} = for (item, scopeN, isLocal) in allSyms(c): @@ -347,8 +352,8 @@ proc suggestEverything(c: PContext, n, f: PNode, outputs: var Suggestions) = for (it, scopeN, isLocal) in allSyms(c): var pm: PrefixMatch if filterSym(it, f, pm): - outputs.add(symToSuggest(c.graph, it, isLocal = isLocal, ideSug, n.info, 0, pm, - c.inTypeContext > 0, scopeN)) + outputs.add(symToSuggest(c.graph, it, isLocal = isLocal, ideSug, n.info, + it.getQuality, pm, c.inTypeContext > 0, scopeN)) proc suggestFieldAccess(c: PContext, n, field: PNode, outputs: var Suggestions) = # special code that deals with ``myObj.``. `n` is NOT the nkDotExpr-node, but @@ -368,9 +373,12 @@ proc suggestFieldAccess(c: PContext, n, field: PNode, outputs: var Suggestions) else: for it in allSyms(c.graph, n.sym): if filterSym(it, field, pm): - outputs.add(symToSuggest(c.graph, it, isLocal=false, ideSug, n.info, 100, pm, c.inTypeContext > 0, -100)) - outputs.add(symToSuggest(c.graph, m, isLocal=false, ideMod, n.info, 100, PrefixMatch.None, - c.inTypeContext > 0, -99)) + outputs.add(symToSuggest(c.graph, it, isLocal=false, ideSug, + n.info, it.getQuality, pm, + c.inTypeContext > 0, -100)) + outputs.add(symToSuggest(c.graph, m, isLocal=false, ideMod, n.info, + 100, PrefixMatch.None, c.inTypeContext > 0, + -99)) if typ == nil: # a module symbol has no type for example: @@ -379,11 +387,15 @@ proc suggestFieldAccess(c: PContext, n, field: PNode, outputs: var Suggestions) # all symbols accessible, because we are in the current module: for it in items(c.topLevelScope.symbols): if filterSym(it, field, pm): - outputs.add(symToSuggest(c.graph, it, isLocal=false, ideSug, n.info, 100, pm, c.inTypeContext > 0, -99)) + outputs.add(symToSuggest(c.graph, it, isLocal=false, ideSug, + n.info, it.getQuality, pm, + c.inTypeContext > 0, -99)) else: for it in allSyms(c.graph, n.sym): if filterSym(it, field, pm): - outputs.add(symToSuggest(c.graph, it, isLocal=false, ideSug, n.info, 100, pm, c.inTypeContext > 0, -99)) + outputs.add(symToSuggest(c.graph, it, isLocal=false, ideSug, + n.info, it.getQuality, pm, + c.inTypeContext > 0, -99)) else: # fallback: suggestEverything(c, n, field, outputs) @@ -667,7 +679,7 @@ proc suggestSentinel*(c: PContext) = var pm: PrefixMatch if filterSymNoOpr(it, nil, pm): outputs.add(symToSuggest(c.graph, it, isLocal = isLocal, ideSug, - newLineInfo(c.config.m.trackPos.fileIndex, 0, -1), 0, + newLineInfo(c.config.m.trackPos.fileIndex, 0, -1), it.getQuality, PrefixMatch.None, false, scopeN)) dec(c.compilesContextId) diff --git a/nimsuggest/tests/tsug_regression.nim b/nimsuggest/tests/tsug_regression.nim index ba2034bd3c..d91d2d1628 100644 --- a/nimsuggest/tests/tsug_regression.nim +++ b/nimsuggest/tests/tsug_regression.nim @@ -16,15 +16,17 @@ proc main = cfg = loadConfig("file") map0.#[!]# +# the maxresults are limited as it seems there is sort or some other +# instability that causes the suggestions to slightly differ between 32 bit +# and 64 bit versions of nimsuggest + discard """ -$nimsuggest --tester $file +$nimsuggest --tester --maxresults:4 $file >sug $1 -sug;;skProc;;tables.hasKey;;proc (t: Table[hasKey.A, hasKey.B], key: A): bool;;*/lib/pure/collections/tables.nim;;374;;5;;"Returns true if*";;100;;None -sug;;skProc;;tables.add;;proc (t: var Table[add.A, add.B], key: A, val: sink B);;*/lib/pure/collections/tables.nim;;505;;5;;"Puts a new*";;100;;None -sug;;skIterator;;tables.allValues;;iterator (t: Table[allValues.A, allValues.B], key: A): B{.inline.};;*/lib/pure/collections/tables.nim;;769;;9;;"Iterates over any*";;100;;None -sug;;skProc;;tables.clear;;proc (t: var Table[clear.A, clear.B]);;*/lib/pure/collections/tables.nim;;567;;5;;"Resets the table so that it is empty.*";;100;;None -sug;;skProc;;tables.contains;;proc (t: Table[contains.A, contains.B], key: A): bool;;*/lib/pure/collections/tables.nim;;392;;5;;"Alias of `hasKey*";;100;;None -* +sug;;skProc;;tables.hasKey;;proc (t: Table[hasKey.A, hasKey.B], key: A): bool;;*/lib/pure/collections/tables.nim;;374;;5;;"Returns true *";;100;;None +sug;;skProc;;tables.clear;;proc (t: var Table[clear.A, clear.B]);;*/lib/pure/collections/tables.nim;;567;;5;;"Resets the table so that it is empty*";;100;;None +sug;;skProc;;tables.contains;;proc (t: Table[contains.A, contains.B], key: A): bool;;*/lib/pure/collections/tables.nim;;*;;5;;"Alias of *";;100;;None +sug;;skProc;;tables.del;;proc (t: var Table[del.A, del.B], key: A);;*/lib/pure/collections/tables.nim;;*;;5;;"*";;100;;None """ # TODO: test/fix suggestion sorting - deprecated suggestions should rank lower diff --git a/nimsuggest/tests/ttype_decl.nim b/nimsuggest/tests/ttype_decl.nim index d7ed63ed04..6022392d00 100644 --- a/nimsuggest/tests/ttype_decl.nim +++ b/nimsuggest/tests/ttype_decl.nim @@ -1,9 +1,9 @@ discard """ $nimsuggest --tester --maxresults:3 $file >sug $1 -sug;;skType;;ttype_decl.Other;;Other;;$file;;10;;2;;"";;0;;None -sug;;skType;;system.int;;int;;*/lib/system/basic_types.nim;;2;;2;;"";;0;;None -sug;;skType;;system.string;;string;;*/lib/system.nim;;34;;2;;"";;0;;None +sug;;skType;;ttype_decl.Other;;Other;;$file;;10;;2;;"";;100;;None +sug;;skType;;system.int;;int;;*/lib/system/basic_types.nim;;2;;2;;"";;100;;None +sug;;skType;;system.string;;string;;*/lib/system.nim;;34;;2;;"";;100;;None """ import strutils type