Fix/improve handling of forward declarations in nimsuggest (#20493)

* Fix/improve handling of forward declarations in nimsuggest

- ideUse now works fine when invoked on the implementation
- implemented ideDeclaration to make cover lsp feature textDocument/declaration
- fixed performance issue related to deduplicating symbols. Now the
deduplication happens after the symbols are filtered. As a alternative we might
change the way cached symbols are stored(e. g. use set).
- I also fixed the way globalSymbols work. Now it will sort the responses based
on the match location to make sure that the results are sorted in user friendly way.

* Update nimsuggest/nimsuggest.nim

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

6 files changed, 119 insertions, 13 deletions

diff --git a/compiler/modulegraphs.nim b/compiler/modulegraphs.nim
index 0df72c43b..9fa530a47 100644
--- a/compiler/modulegraphs.nim
+++ b/compiler/modulegraphs.nim
@@ -643,11 +643,11 @@ proc `==`*(a, b: SymInfoPair): bool =
   result = a.sym == b.sym and a.info.exactEquals(b.info)
 
 proc fileSymbols*(graph: ModuleGraph, fileIdx: FileIndex): seq[SymInfoPair] =
-  result = graph.suggestSymbols.getOrDefault(fileIdx, @[]).deduplicate
+  result = graph.suggestSymbols.getOrDefault(fileIdx, @[])
 
 iterator suggestSymbolsIter*(g: ModuleGraph): SymInfoPair =
   for xs in g.suggestSymbols.values:
-    for x in xs.deduplicate:
+    for x in xs:
       yield x
 
 iterator suggestErrorsIter*(g: ModuleGraph): Suggest =
diff --git a/compiler/options.nim b/compiler/options.nim
index d52b1cdef..94776cc59 100644
--- a/compiler/options.nim
+++ b/compiler/options.nim
@@ -192,7 +192,7 @@ type
   IdeCmd* = enum
     ideNone, ideSug, ideCon, ideDef, ideUse, ideDus, ideChk, ideChkFile, ideMod,
     ideHighlight, ideOutline, ideKnown, ideMsg, ideProject, ideGlobalSymbols,
-    ideRecompile, ideChanged, ideType
+    ideRecompile, ideChanged, ideType, ideDeclaration
 
   Feature* = enum  ## experimental features; DO NOT RENAME THESE!
     implicitDeref,
@@ -1026,6 +1026,7 @@ proc `$`*(c: IdeCmd): string =
   of ideMsg: "msg"
   of ideProject: "project"
   of ideGlobalSymbols: "globalSymbols"
+  of ideDeclaration: "declaration"
   of ideRecompile: "recompile"
   of ideChanged: "changed"
   of ideType: "type"
diff --git a/compiler/suggest.nim b/compiler/suggest.nim
index 9d4089707..b5f787296 100644
--- a/compiler/suggest.nim
+++ b/compiler/suggest.nim
@@ -164,7 +164,7 @@ proc symToSuggest*(g: ModuleGraph; s: PSym, isLocal: bool, section: IdeCmd, info
     result.tokenLen = 0
   else:
     let infox =
-      if useSuppliedInfo or section in {ideUse, ideHighlight, ideOutline}:
+      if useSuppliedInfo or section in {ideUse, ideHighlight, ideOutline, ideDeclaration}:
         info
       else:
         s.info
diff --git a/nimsuggest/nimsuggest.nim b/nimsuggest/nimsuggest.nim
index 4fee8b2a2..d4c0f7545 100644
--- a/nimsuggest/nimsuggest.nim
+++ b/nimsuggest/nimsuggest.nim
@@ -9,6 +9,7 @@
 
 import compiler/renderer
 import strformat
+import algorithm
 import tables
 import std/sha1
 import times
@@ -149,7 +150,7 @@ proc listEpc(): SexpNode =
     argspecs = sexp("file line column dirtyfile".split(" ").map(newSSymbol))
     docstring = sexp("line starts at 1, column at 0, dirtyfile is optional")
   result = newSList()
-  for command in ["sug", "con", "def", "use", "dus", "chk", "mod", "globalSymbols", "recompile", "saved", "chkFile"]:
+  for command in ["sug", "con", "def", "use", "dus", "chk", "mod", "globalSymbols", "recompile", "saved", "chkFile", "declaration"]:
     let
       cmd = sexp(command)
       methodDesc = newSList()
@@ -455,6 +456,7 @@ proc execCmd(cmd: string; graph: ModuleGraph; cachedMsgs: CachedMsgs) =
   of "project": conf.ideCmd = ideProject
   of "changed": conf.ideCmd = ideChanged
   of "globalsymbols": conf.ideCmd = ideGlobalSymbols
+  of "declaration": conf.ideCmd = ideDeclaration
   of "chkfile": conf.ideCmd = ideChkFile
   of "recompile": conf.ideCmd = ideRecompile
   of "type": conf.ideCmd = ideType
@@ -711,12 +713,28 @@ proc recompilePartially(graph: ModuleGraph, projectFileIdx = InvalidFileIdx) =
     except Exception as e:
       myLog fmt "Failed clean recompilation:\n {e.msg} \n\n {e.getStackTrace()}"
 
+func deduplicateSymInfoPair[SymInfoPair](xs: seq[SymInfoPair]): seq[SymInfoPair] =
+  # xs contains duplicate items and we want to filter them by range because the
+  # sym may not match. This can happen when xs contains the same definition but
+  # with different signature becase suggestSym might be called multiple times
+  # for the same symbol (e. g. including/excluding the pragma)
+  result = @[]
+  for itm in xs.reversed:
+    var found = false
+    for res in result:
+      if res.info.exactEquals(itm.info):
+        found = true
+        break
+    if not found:
+      result.add(itm)
+  result.reverse()
+
 proc findSymData(graph: ModuleGraph, file: AbsoluteFile; line, col: int):
     ref SymInfoPair =
   let
     fileIdx = fileInfoIdx(graph.config, file)
     trackPos = newLineInfo(fileIdx, line, col)
-  for s in graph.fileSymbols(fileIdx):
+  for s in graph.fileSymbols(fileIdx).deduplicateSymInfoPair:
     if isTracked(s.info, trackPos, s.sym.name.s.len):
       new(result)
       result[] = s
@@ -746,6 +764,10 @@ const
   # kinds for ideOutline and ideGlobalSymbols
   searchableSymKinds = {skField, skEnumField, skIterator, skMethod, skFunc, skProc, skConverter, skTemplate}
 
+proc symbolEqual(left, right: PSym): bool =
+  # More relaxed symbol comparison
+  return left.info.exactEquals(right.info) and left.name == right.name
+
 proc executeNoHooksV3(cmd: IdeCmd, file: AbsoluteFile, dirtyfile: AbsoluteFile, line, col: int;
     graph: ModuleGraph) =
   let conf = graph.config
@@ -786,7 +808,7 @@ proc executeNoHooksV3(cmd: IdeCmd, file: AbsoluteFile, dirtyfile: AbsoluteFile,
     graph.unmarkAllDirty()
 
   # these commands require partially compiled project
-  elif cmd in {ideSug, ideOutline, ideHighlight, ideDef, ideChkFile, ideType} and
+  elif cmd in {ideSug, ideOutline, ideHighlight, ideDef, ideChkFile, ideType, ideDeclaration} and
        (graph.needsCompilation(fileIndex) or cmd == ideSug):
     # for ideSug use v2 implementation
     if cmd == ideSug:
@@ -814,9 +836,12 @@ proc executeNoHooksV3(cmd: IdeCmd, file: AbsoluteFile, dirtyfile: AbsoluteFile,
   of ideUse, ideDus:
     let symbol = graph.findSymData(file, line, col)
     if not symbol.isNil:
+      var res: seq[SymInfoPair] = @[]
       for s in graph.suggestSymbolsIter:
-        if s.sym == symbol.sym:
-          graph.suggestResult(s.sym, s.info)
+        if s.sym.symbolEqual(symbol.sym):
+          res.add(s)
+      for s in res.deduplicateSymInfoPair():
+        graph.suggestResult(s.sym, s.info)
   of ideHighlight:
     let sym = graph.findSymData(file, line, col)
     if not sym.isNil:
@@ -836,6 +861,7 @@ proc executeNoHooksV3(cmd: IdeCmd, file: AbsoluteFile, dirtyfile: AbsoluteFile,
     let
       module = graph.getModule fileIndex
       symbols = graph.fileSymbols(fileIndex)
+        .deduplicateSymInfoPair
         .filterIt(it.sym.info.exactEquals(it.info) and
                     (it.sym.owner == module or
                      it.sym.kind in searchableSymKinds))
@@ -852,16 +878,58 @@ proc executeNoHooksV3(cmd: IdeCmd, file: AbsoluteFile, dirtyfile: AbsoluteFile,
     for error in errors:
       suggestResult(graph.config, error)
   of ideGlobalSymbols:
-    var counter = 0
+    var
+      counter = 0
+      res: seq[SymInfoPair] = @[]
+
     for s in graph.suggestSymbolsIter:
       if (sfGlobal in s.sym.flags or s.sym.kind in searchableSymKinds) and
           s.sym.info == s.info:
         if contains(s.sym.name.s, file.string):
           inc counter
-          graph.suggestResult(s.sym, s.info)
-        # stop after first 100 results
-        if counter > 100:
+          res = res.filterIt(not it.info.exactEquals(s.info))
+          res.add s
+          # stop after first 1000 matches...
+          if counter > 1000:
+            break
+
+    # ... then sort them by weight ...
+    res.sort() do (left, right: SymInfoPair) -> int:
+      let
+        leftString = left.sym.name.s
+        rightString = right.sym.name.s
+        leftIndex = leftString.find(file.string)
+        rightIndex = rightString.find(file.string)
+
+      if leftIndex == rightIndex:
+        result = cmp(toLowerAscii(leftString),
+                     toLowerAscii(rightString))
+      else:
+        result = cmp(leftIndex, rightIndex)
+
+    # ... and send first 100 results
+    if res.len > 0:
+      for i in 0 .. min(100, res.len - 1):
+        let s = res[i]
+        graph.suggestResult(s.sym, s.info)
+
+  of ideDeclaration:
+    let s = graph.findSymData(file, line, col)
+    if not s.isNil:
+      # find first mention of the symbol in the file containing the definition.
+      # It is either the definition or the declaration.
+      var first: SymInfoPair
+      for symbol in graph.fileSymbols(s.sym.info.fileIndex).deduplicateSymInfoPair:
+        if s.sym.symbolEqual(symbol.sym):
+          first = symbol
           break
+
+      if s.info.exactEquals(first.info):
+        # we are on declaration, go to definition
+        graph.suggestResult(first.sym, first.sym.info, ideDeclaration)
+      else:
+        # we are on definition or usage, look for declaration
+        graph.suggestResult(first.sym, first.info, ideDeclaration)
   else:
     myLog fmt "Discarding {cmd}"
 
diff --git a/nimsuggest/tests/tv3_forward_definition.nim b/nimsuggest/tests/tv3_forward_definition.nim
new file mode 100644
index 000000000..14c0dc2e2
--- /dev/null
+++ b/nimsuggest/tests/tv3_forward_definition.nim
@@ -0,0 +1,23 @@
+proc de#[!]#mo(): int
+
+proc de#[!]#mo(): int = 5
+
+let a = de#[!]#mo()
+
+discard """
+$nimsuggest --v3 --tester $file
+>use $1
+use	skProc	tv3_forward_definition.demo	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	1	5	""	100
+def	skProc	tv3_forward_definition.demo	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	3	5	""	100
+use	skProc	tv3_forward_definition.demo	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	5	8	""	100
+>use $2
+use	skProc	tv3_forward_definition.demo	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	1	5	""	100
+def	skProc	tv3_forward_definition.demo	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	3	5	""	100
+use	skProc	tv3_forward_definition.demo	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	5	8	""	100
+>declaration $1
+declaration	skProc	tv3_forward_definition.demo	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	3	5	""	100
+>declaration $2
+declaration	skProc	tv3_forward_definition.demo	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	1	5	""	100
+>declaration $3
+declaration	skProc	tv3_forward_definition.demo	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	1	5	""	100
+"""
diff --git a/nimsuggest/tests/tv3_globalSymbols.nim b/nimsuggest/tests/tv3_globalSymbols.nim
new file mode 100644
index 000000000..85814cadd
--- /dev/null
+++ b/nimsuggest/tests/tv3_globalSymbols.nim
@@ -0,0 +1,14 @@
+# Tests the order of the matches
+proc Btoken(): int = 5
+proc tokenA(): int = 5
+proc token(): int = 5
+proc BBtokenA(): int = 5
+
+discard """
+$nimsuggest --v3 --tester $file
+>globalSymbols token
+def	skProc	tv3_globalSymbols.token	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	4	5	""	100
+def	skProc	tv3_globalSymbols.tokenA	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	3	5	""	100
+def	skProc	tv3_globalSymbols.Btoken	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	2	5	""	100
+def	skProc	tv3_globalSymbols.BBtokenA	proc (): int{.noSideEffect, gcsafe, locks: 0.}	$file	5	5	""	100
+"""