From 5a7b9755685f69fb611227b238d3b2d30105ae2d Mon Sep 17 00:00:00 2001
From: Bung <crc32@qq.com>
Date: Tue, 10 Oct 2023 06:32:24 +0800
Subject: [PATCH] fix(suggest): file level cmds don't leak internal syms (#944)

## Summary

Suggest commands related to the whole file (i.e. outline), and not
cursor positions, no longer include symbols from inside template and
generic routines.

## Details

`semIdeForTemplateOrGenericCheck`  is used to detect whether the cursor
is within a generic routine or template body, before this change it
didn't validate which suggest command was invoked ( `ideCmd` ). This
resulted in  `safeSemExpr`  being run and potentially reporting symbols
from inside such routines. This led to commands such as  `outline`
containing symbols from the routine bodies polluting module level
results.

`semIdeForTemplateOrGenericCheck`  has now been updated to check the
command against the newly added  `ideLocCmds`  constant, which includes
commands that require cursor position tracking ( `ideSug` ,  `ideCon` ,
`ideDef` ,  `ideUse` ,  `ideDus` ), preventing the unexpected analysis
and pollution of module level results.

A regression test was added to ensure that accidentally providing cursor
tracking information to module level commands doesn't change the
results.

---------

Co-authored-by: zerbina <100542850+zerbina@users.noreply.github.com>
Co-authored-by: Saem Ghani <saemghani+github@gmail.com>
---
 compiler/front/options.nim            |  4 ++++
 compiler/sem/sem.nim                  | 19 ++++++++++---------
 nimsuggest/tests/toutline_generic.nim | 27 +++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 9 deletions(-)
 create mode 100644 nimsuggest/tests/toutline_generic.nim

diff --git a/compiler/front/options.nim b/compiler/front/options.nim
index 3b86b993cd5..eecc965d139 100644
--- a/compiler/front/options.nim
+++ b/compiler/front/options.nim
@@ -309,6 +309,10 @@ type
     when defined(nimDebugUnreportedErrors):
       unreportedErrors*: OrderedTable[NodeId, PNode]
 
+const 
+  IdeLocCmds* = {ideSug, ideCon, ideDef, ideUse, ideDus}
+    ## IDE commands requiring source locations, related `MsgConfig.trackPos`
+
 template `[]`*(conf: ConfigRef, idx: FileIndex): TFileInfo =
   conf.m.fileInfos[idx.uint32]
 
diff --git a/compiler/sem/sem.nim b/compiler/sem/sem.nim
index 965db07ac34..34ffb25f640 100644
--- a/compiler/sem/sem.nim
+++ b/compiler/sem/sem.nim
@@ -164,19 +164,20 @@ proc deltaTrace(stopProc, indent: string, entries: seq[StackTraceEntry])
     echo:
       "$1| $2 $3($4)" % [indent, $e.procname, $e.filename, $e.line]
 
-template semIdeForTemplateOrGenericCheck(conf, n, requiresCheck) =
-  # we check quickly if the node is where the cursor is
+template semIdeForTemplateOrGenericCheck(conf, n, cursorInBody) =
+  # use only for idetools support; detecting cursor in generic or template body
+  # if so call `semIdeForTemplateOrGeneric` for semantic checking
   when defined(nimsuggest):
-    if n.info.fileIndex == conf.m.trackPos.fileIndex and n.info.line == conf.m.trackPos.line:
-      requiresCheck = true
+    if conf.ideCmd in IdeLocCmds and
+       n.info.fileIndex == conf.m.trackPos.fileIndex and
+       n.info.line == conf.m.trackPos.line:
+      cursorInBody = true
 
 template semIdeForTemplateOrGeneric(c: PContext; n: PNode;
-                                    requiresCheck: bool) =
-  # use only for idetools support; this is pretty slow so generics and
-  # templates perform some quick check whether the cursor is actually in
-  # the generic or template.
+                                    cursorInBody: bool) =
+  # provide incomplete information for idetools support in generic or template
   when defined(nimsuggest):
-    if c.config.cmd == cmdIdeTools and requiresCheck:
+    if c.config.cmd == cmdIdeTools and cursorInBody:
       #if optIdeDebug in gGlobalOptions:
       #  echo "passing to safeSemExpr: ", renderTree(n)
       discard safeSemExpr(c, n)
diff --git a/nimsuggest/tests/toutline_generic.nim b/nimsuggest/tests/toutline_generic.nim
new file mode 100644
index 00000000000..5f16357a3da
--- /dev/null
+++ b/nimsuggest/tests/toutline_generic.nim
@@ -0,0 +1,27 @@
+# Regression test for symbols within template and generic bodies being
+# wrongfully reported by `outline`.
+
+proc generic[T]() =
+  var x = 0 #[!]#
+  return # <- when no cursor is provided, this triggered `x` being reported
+
+template templ() =
+  var x = 0 #[!]#
+  return # <- when no cursor is provided, this triggered `x` being reported
+
+# try both with and without a specified cursor position, they should be the same
+
+discard """
+$nimsuggest --tester $file
+>outline $1
+outline;;skProc;;toutline_generic.generic;;*;;4;;5;;"";;100
+outline;;skTemplate;;toutline_generic.templ;;*;;8;;9;;"";;100
+
+>outline $2
+outline;;skProc;;toutline_generic.generic;;*;;4;;5;;"";;100
+outline;;skTemplate;;toutline_generic.templ;;*;;8;;9;;"";;100
+
+>outline $path/toutline_generic.nim
+outline;;skProc;;toutline_generic.generic;;*;;4;;5;;"";;100
+outline;;skTemplate;;toutline_generic.templ;;*;;8;;9;;"";;100
+"""