Skip to content

Commit

Permalink
fix(suggest): file level cmds don't leak internal syms (#944)
Browse files Browse the repository at this point in the history
## 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>
  • Loading branch information
3 people authored Oct 9, 2023
1 parent 78ce106 commit 5a7b975
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 9 deletions.
4 changes: 4 additions & 0 deletions compiler/front/options.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
19 changes: 10 additions & 9 deletions compiler/sem/sem.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions nimsuggest/tests/toutline_generic.nim
Original file line number Diff line number Diff line change
@@ -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
"""

0 comments on commit 5a7b975

Please sign in to comment.