Don't repeat suggestions for same symbol (#21140)

* Track seen module graphs so symbols from the same module aren't repeated
Add test case

* Track symbols instead of modules

* Don't show duplicate symbols in spell checker

Removes the declared location from the message. Since we don't show duplicates anymore it would be a bit misleading if we only show the location for the first declaration of the symbol

compiler/lookups.nim

@@ -15,7 +15,7 @@ when defined(nimPreviewSlimSystem):
 
 import
   intsets, ast, astalgo, idents, semdata, types, msgs, options,
-  renderer, nimfix/prettybase, lineinfos, modulegraphs, astmsgs
+  renderer, nimfix/prettybase, lineinfos, modulegraphs, astmsgs, sets
 
 proc ensureNoMissingOrUnusedSymbols(c: PContext; scope: PScope)
 
@@ -170,6 +170,7 @@ iterator allSyms*(c: PContext): (PSym, int, bool) =
   # really iterate over all symbols in all the scopes. This is expensive
   # and only used by suggest.nim.
   var isLocal = true
+
   var scopeN = 0
   for scope in allScopes(c.currentScope):
     if scope == c.topLevelScope: isLocal = false
@@ -184,6 +185,17 @@ iterator allSyms*(c: PContext): (PSym, int, bool) =
       assert s != nil
       yield (s, scopeN, isLocal)
 
+iterator uniqueSyms*(c: PContext): (PSym, int, bool) =
+  ## Like [allSyms] except only returns unique symbols (Uniqueness determined by line + name)
+  # Track seen symbols so we don't duplicate them.
+  # The int is for the symbols name, and line info is
+  # to be able to tell apart symbols with same name but on different lines
+  var seen = initHashSet[(TLineInfo, int)]()
+  for res in allSyms(c):
+    if not seen.containsOrIncl((res[0].info, res[0].name.id)):
+      yield res
+
+
 proc someSymFromImportTable*(c: PContext; name: PIdent; ambiguous: var bool): PSym =
   var marked = initIntSet()
   var symSet = OverloadableSyms
@@ -445,12 +457,13 @@ proc fixSpelling(c: PContext, n: PNode, ident: PIdent, result: var string) =
     let dist = editDistance(name0, sym.name.s.nimIdentNormalize)
     var msg: string
     msg.add "\n ($1, $2): '$3'" % [$dist, $depth, sym.name.s]
-    addDeclaredLoc(msg, c.config, sym) # `msg` needed for deterministic ordering.
     list.push SpellCandidate(dist: dist, depth: depth, msg: msg, sym: sym)
 
   if list.len == 0: return
   let e0 = list[0]
-  var count = 0
+  var
+    count = 0
+    last: PIdent = nil
   while true:
     # pending https://github.com/timotheecour/Nim/issues/373 use more efficient `itemsSorted`.
     if list.len == 0: break
@@ -466,8 +479,10 @@ proc fixSpelling(c: PContext, n: PNode, ident: PIdent, result: var string) =
     elif count >= c.config.spellSuggestMax: break
     if count == 0:
       result.add "\ncandidates (edit distance, scope distance); see '--spellSuggest': "
-    result.add e.msg
-    count.inc
+    if e.sym.name != last:
+      result.add e.msg
+      count.inc
+      last = e.sym.name
 
 proc errorUseQualifier(c: PContext; info: TLineInfo; s: PSym; amb: var bool): PSym =
   var err = "ambiguous identifier: '" & s.name.s & "'"

compiler/suggest.nim

@@ -289,7 +289,7 @@ proc suggestField(c: PContext, s: PSym; f: PNode; info: TLineInfo; outputs: var
                               s.getQuality, pm, c.inTypeContext > 0, 0))
 
 template wholeSymTab(cond, section: untyped) {.dirty.} =
-  for (item, scopeN, isLocal) in allSyms(c):
+  for (item, scopeN, isLocal) in uniqueSyms(c):
     let it = item
     var pm: PrefixMatch
     if cond:
@@ -362,7 +362,7 @@ proc suggestOperations(c: PContext, n, f: PNode, typ: PType, outputs: var Sugges
 
 proc suggestEverything(c: PContext, n, f: PNode, outputs: var Suggestions) =
   # do not produce too many symbols:
-  for (it, scopeN, isLocal) in allSyms(c):
+  for (it, scopeN, isLocal) in uniqueSyms(c):
     var pm: PrefixMatch
     if filterSym(it, f, pm):
       outputs.add(symToSuggest(c.graph, it, isLocal = isLocal, ideSug, n.info,
@@ -680,7 +680,7 @@ proc suggestSentinel*(c: PContext) =
   inc(c.compilesContextId)
   var outputs: Suggestions = @[]
   # suggest everything:
-  for (it, scopeN, isLocal) in allSyms(c):
+  for (it, scopeN, isLocal) in uniqueSyms(c):
     var pm: PrefixMatch
     if filterSymNoOpr(it, nil, pm):
       outputs.add(symToSuggest(c.graph, it, isLocal = isLocal, ideSug,

tests/misc/tspellsuggest.nim

@@ -5,21 +5,21 @@ discard """
   nimout: '''
 tspellsuggest.nim(45, 13) Error: undeclared identifier: 'fooBar'
 candidates (edit distance, scope distance); see '--spellSuggest':
- (1, 0): 'fooBar8' [var declared in tspellsuggest.nim(43, 9)]
- (1, 1): 'fooBar7' [var declared in tspellsuggest.nim(41, 7)]
- (1, 3): 'fooBar1' [var declared in tspellsuggest.nim(33, 5)]
- (1, 3): 'fooBar2' [let declared in tspellsuggest.nim(34, 5)]
- (1, 3): 'fooBar3' [const declared in tspellsuggest.nim(35, 7)]
- (1, 3): 'fooBar4' [proc declared in tspellsuggest.nim(36, 6)]
- (1, 3): 'fooBar5' [template declared in tspellsuggest.nim(37, 10)]
- (1, 3): 'fooBar6' [macro declared in tspellsuggest.nim(38, 7)]
- (1, 5): 'FooBar' [type declared in mspellsuggest.nim(5, 6)]
- (1, 5): 'fooBar4' [proc declared in mspellsuggest.nim(1, 6)]
- (1, 5): 'fooBar9' [var declared in mspellsuggest.nim(2, 5)]
- (1, 5): 'fooCar' [var declared in mspellsuggest.nim(4, 5)]
- (2, 5): 'FooCar' [type declared in mspellsuggest.nim(6, 6)]
- (2, 5): 'GooBa' [type declared in mspellsuggest.nim(7, 6)]
- (3, 0): 'fooBarBaz' [const declared in tspellsuggest.nim(44, 11)]
+ (1, 0): 'fooBar8'
+ (1, 1): 'fooBar7'
+ (1, 3): 'fooBar1'
+ (1, 3): 'fooBar2'
+ (1, 3): 'fooBar3'
+ (1, 3): 'fooBar4'
+ (1, 3): 'fooBar5'
+ (1, 3): 'fooBar6'
+ (1, 5): 'FooBar'
+ (1, 5): 'fooBar4'
+ (1, 5): 'fooBar9'
+ (1, 5): 'fooCar'
+ (2, 5): 'FooCar'
+ (2, 5): 'GooBa'
+ (3, 0): 'fooBarBaz'
 '''
 """
 

tests/misc/tspellsuggest2.nim

@@ -5,18 +5,18 @@ discard """
   nimout: '''
 tspellsuggest2.nim(45, 13) Error: undeclared identifier: 'fooBar'
 candidates (edit distance, scope distance); see '--spellSuggest':
- (1, 0): 'fooBar8' [var declared in tspellsuggest2.nim(43, 9)]
- (1, 1): 'fooBar7' [var declared in tspellsuggest2.nim(41, 7)]
- (1, 3): 'fooBar1' [var declared in tspellsuggest2.nim(33, 5)]
- (1, 3): 'fooBar2' [let declared in tspellsuggest2.nim(34, 5)]
- (1, 3): 'fooBar3' [const declared in tspellsuggest2.nim(35, 7)]
- (1, 3): 'fooBar4' [proc declared in tspellsuggest2.nim(36, 6)]
- (1, 3): 'fooBar5' [template declared in tspellsuggest2.nim(37, 10)]
- (1, 3): 'fooBar6' [macro declared in tspellsuggest2.nim(38, 7)]
- (1, 5): 'FooBar' [type declared in mspellsuggest.nim(5, 6)]
- (1, 5): 'fooBar4' [proc declared in mspellsuggest.nim(1, 6)]
- (1, 5): 'fooBar9' [var declared in mspellsuggest.nim(2, 5)]
- (1, 5): 'fooCar' [var declared in mspellsuggest.nim(4, 5)]
+ (1, 0): 'fooBar8'
+ (1, 1): 'fooBar7'
+ (1, 3): 'fooBar1'
+ (1, 3): 'fooBar2'
+ (1, 3): 'fooBar3'
+ (1, 3): 'fooBar4'
+ (1, 3): 'fooBar5'
+ (1, 3): 'fooBar6'
+ (1, 5): 'FooBar'
+ (1, 5): 'fooBar4'
+ (1, 5): 'fooBar9'
+ (1, 5): 'fooCar'
 '''
 """
 

tests/misc/tspellsuggest3.nim

@@ -0,0 +1,21 @@
+discard """
+  # pending bug #16521 (bug 12) use `matrix`
+  cmd: "nim c --spellsuggest:4 --hints:off $file"
+  action: "reject"
+  nimout: '''
+tspellsuggest3.nim(21, 1) Error: undeclared identifier: 'fooBar'
+candidates (edit distance, scope distance); see '--spellSuggest':
+ (1, 2): 'FooBar'
+ (1, 2): 'fooBar4'
+ (1, 2): 'fooBar9'
+ (1, 2): 'fooCar'
+'''
+"""
+
+import ./mspellsuggest
+import ./mspellsuggest
+import ./mspellsuggest
+import ./mspellsuggest
+
+
+fooBar