Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions internal/lsp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -3149,18 +3149,20 @@ func (s *Server) DocumentSymbol(ctx context.Context, params *protocol.DocumentSy
if i+1 < n && tokens[i+1].Kind == parser.TokColon {
continue
}
doIdx, nextPos, hasDoBlock := parser.ScanForwardToBlockDo(tokens, n, i+1)
doIdx, nextPos, hasDoBlock := parser.ScanForwardToMacroCallBlockDo(tokens, n, i+1)
if !hasDoBlock {
continue
}

lineIdx := tok.Line - 1
indent := tokCol(tok)

// Extract the argument between the macro name and `do` from source bytes
// Extract the argument between the macro name and `do` from source bytes.
// Collapse internal whitespace so a multi-line keyword-arg head renders
// as a single-line outline label.
label := macroName
argBytes := source[tok.End:tokens[doIdx].Start]
arg := strings.TrimSpace(string(argBytes))
arg := strings.Join(strings.Fields(string(argBytes)), " ")
if len(arg) >= 2 && arg[0] == '"' && arg[len(arg)-1] == '"' {
arg = arg[1 : len(arg)-1]
}
Expand Down
87 changes: 87 additions & 0 deletions internal/lsp/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4084,6 +4084,93 @@ end
}
}

// TestDocumentSymbol_NoLocalVarsOrCalls reproduces issue #69: local variable
// assignments and ordinary function calls inside a function body were surfacing
// as document symbols because the scan for a macro's `do` block crossed statement
// boundaries and latched onto a later `case ... do`.
func TestDocumentSymbol_NoLocalVarsOrCalls(t *testing.T) {
server, cleanup := setupTestServer(t)
defer cleanup()

content := `defmodule Web.OpportunitiesLive do
def handle_params(unsigned_params, _uri, socket) do
changeset = build_changeset(search_form_data, unsigned_params, scopes, context)

socket =
case Ecto.Changeset.apply_action(changeset, :update) do
{:ok, search_form_data} ->
paginate_async(socket, unsigned_params, [per_page: @per_page], fn page, per_page ->
params = build_es_query_params(search_form_data, page, per_page)

case Rpc.call("rb.search", [live_action]) do
{:ok, result} -> result
{:error, reason} -> {0, []}
end
end)

{:error, _changeset} ->
socket
end

{:noreply, socket}
end
end
`
docURI := "file:///test/opportunities_live.ex"
server.docs.Set(docURI, content)

symbols := documentSymbols(t, server, docURI)
handleParams := findSymbol(symbols, "handle_params/3")
if handleParams == nil {
t.Fatal("expected handle_params/3 symbol")
}

// The function body must not produce any child symbols: neither the
// `changeset = ...` assignment nor the `paginate_async(...)` call.
if len(handleParams.Children) != 0 {
t.Errorf("expected no children for handle_params/3, got %v", collectNames(handleParams.Children))
}

// Belt and suspenders: neither offending construct from the screenshot
// (issue #69) should surface as a symbol anywhere in the tree.
var walk func(syms []protocol.DocumentSymbol)
walk = func(syms []protocol.DocumentSymbol) {
for _, s := range syms {
if strings.Contains(s.Name, "build_changeset") || strings.Contains(s.Name, "paginate_async") {
t.Errorf("local var/call leaked into symbols: %q", s.Name)
}
walk(s.Children)
}
}
walk(symbols)
}

// TestDocumentSymbol_SplitLineMacroHead verifies that an ExUnit macro whose
// keyword-argument head spans multiple lines (trailing comma continuation) is
// still captured as a symbol.
func TestDocumentSymbol_SplitLineMacroHead(t *testing.T) {
server, cleanup := setupTestServer(t)
defer cleanup()

content := `defmodule MyAppTest do
use ExUnit.Case

test "creates a user",
tags: [:integration] do
assert true
end
end
`
docURI := "file:///test/my_split_test.exs"
server.docs.Set(docURI, content)

symbols := documentSymbols(t, server, docURI)
want := `test "creates a user", tags: [:integration]`
if findSymbol(symbols, want) == nil {
t.Errorf("expected split-line test macro %q to be captured; got %v", want, collectNames(symbols[0].Children))
}
}

func TestDocumentSymbol_BrokenCode(t *testing.T) {
server, cleanup := setupTestServer(t)
defer cleanup()
Expand Down
39 changes: 4 additions & 35 deletions internal/parser/parser_tokenized.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,41 +667,10 @@ func parseTextFromTokens(path string, source []byte, tokens []Token) ([]Definiti
// macro_name :atom
emit = true
default:
// Scan forward to see if TokDo follows the arguments.
// In Elixir, `do` can follow across EOLs and blank lines
// but not past an intervening statement. We track whether
// we've seen EOL at bracket depth 0: once we have, the
// only token that can continue the expression is `do`.
scanDepth := 0
seenEOLAtZero := false
for k := j; k < n; k++ {
switch tokens[k].Kind {
case TokDo:
if scanDepth == 0 {
emit = true
}
case TokEOL, TokComment:
if scanDepth == 0 {
seenEOLAtZero = true
}
case TokOpenParen, TokOpenBracket, TokOpenBrace:
scanDepth++
seenEOLAtZero = false
case TokCloseParen, TokCloseBracket, TokCloseBrace:
scanDepth--
case TokEOF:
k = n
default:
// At depth 0, after seeing EOL, any non-do
// token means a new statement started.
if scanDepth == 0 && seenEOLAtZero {
k = n // stop
}
}
if emit {
break
}
}
// Scan forward to see if a block-opening `do` follows the
// arguments, respecting bracket depth and statement boundaries.
_, _, hasDo := ScanForwardToMacroCallBlockDo(tokens, n, j)
emit = hasDo
}
}
if emit {
Expand Down
53 changes: 53 additions & 0 deletions internal/parser/token_walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,59 @@ func ScanForwardToBlockDo(tokens []Token, n, from int) (doIdx, nextPos int, hasD
return -1, n, false
}

// ScanForwardToMacroCallBlockDo reports whether a block-opening `do` follows a
// bare macro call head starting at `from` (the token just after the macro name).
//
// Unlike ScanForwardToBlockDo, it does not blindly scan to the next statement
// keyword. A bare macro call's `do` belongs to the same logical statement, so we
// track bracket depth — a `do` nested inside parens/brackets/braces opens a
// nested construct's block, not the macro's — and we treat an end-of-line at
// bracket depth zero as a statement separator: once one is seen, any token other
// than `do` begins a new statement and the scan stops. This prevents an
// assignment or plain function call (`changeset = build_changeset(...)`) from
// being mistaken for a macro-with-do-block just because a later statement on a
// following line happens to open a `do`.
//
// A line that ends in a comma at bracket depth zero is an exception: a dangling
// comma is never a valid statement terminator in Elixir, so it marks a multi-line
// keyword-argument head (`test "x",\n async: true do`) and the scan continues.
func ScanForwardToMacroCallBlockDo(tokens []Token, n, from int) (doIdx, nextPos int, hasDo bool) {
scanDepth := 0
seenEOLAtZero := false
lastSigKind := TokEOL
for k := from; k < n; k++ {
switch tokens[k].Kind {
case TokDo:
if scanDepth == 0 {
return k, k + 1, true
}
lastSigKind = TokDo
case TokEOL, TokComment:
// A trailing comma means the head continues on the next line, so this
// is not a statement boundary. Comments do not reset lastSigKind.
if scanDepth == 0 && lastSigKind != TokComma {
seenEOLAtZero = true
}
case TokOpenParen, TokOpenBracket, TokOpenBrace:
scanDepth++
seenEOLAtZero = false
lastSigKind = tokens[k].Kind
case TokCloseParen, TokCloseBracket, TokCloseBrace:
scanDepth--
lastSigKind = tokens[k].Kind
case TokEOF:
return -1, k, false
default:
// At depth 0, after an end-of-line, any non-do token starts a new statement.
if scanDepth == 0 && seenEOLAtZero {
return -1, k, false
}
lastSigKind = tokens[k].Kind
}
}
return -1, n, false
}
Comment thread
JesseHerrick marked this conversation as resolved.

// TrackBlockDepth updates the block depth counter for do/fn/end tokens.
func TrackBlockDepth(kind TokenKind, depth *int) {
switch kind {
Expand Down
81 changes: 81 additions & 0 deletions internal/parser/token_walk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,87 @@ func TestScanForwardToBlockDo_StopsAtStatementBoundary(t *testing.T) {
}
}

func TestScanForwardToMacroCallBlockDo(t *testing.T) {
// firstIdentAfter returns the index just after the first ident matching name.
firstIdentAfter := func(tokens []Token, source []byte, name string) int {
for i, tok := range tokens {
if tok.Kind == TokIdent && string(source[tok.Start:tok.End]) == name {
return i + 1
}
}
return -1
}

tests := []struct {
name string
source string
ident string
wantDo bool
}{
{
name: "macro with do on same line",
source: "describe \"users\" do\n :ok\nend\n",
ident: "describe",
wantDo: true,
},
{
name: "macro with do on its own continuation line",
source: "describe \"x\"\ndo\n :ok\nend\n",
ident: "describe",
wantDo: true,
},
{
name: "macro with multi-line keyword-arg head",
source: "test \"x\",\n async: true do\n :ok\nend\n",
ident: "test",
wantDo: true,
},
{
name: "macro with keyword head split across several lines",
source: "test \"x\",\n async: true,\n tags: [:foo] do\n :ok\nend\n",
ident: "test",
wantDo: true,
},
{
name: "non-trailing comma does not bridge to a later statement's do",
source: "foo a,\n b\n\nx = case y do\n :ok -> :ok\nend\n",
ident: "foo",
wantDo: false,
},
{
name: "assignment whose later statement opens a do",
source: "changeset = build_changeset(a, b)\n\nsocket =\n case foo(changeset) do\n :ok -> :ok\n end\n",
ident: "changeset",
wantDo: false,
},
{
name: "call whose argument fn body opens a do",
source: "paginate_async(socket, fn page ->\n case Rpc.call(x) do\n :ok -> :ok\n end\nend)\n",
ident: "paginate_async",
wantDo: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
source := []byte(tt.source)
tokens := Tokenize(source)
n := len(tokens)
from := firstIdentAfter(tokens, source, tt.ident)
if from < 0 {
t.Fatalf("ident %q not found in source", tt.ident)
}
doIdx, _, hasDo := ScanForwardToMacroCallBlockDo(tokens, n, from)
if hasDo != tt.wantDo {
t.Errorf("hasDo = %v, want %v (doIdx=%d)", hasDo, tt.wantDo, doIdx)
}
if hasDo && tokens[doIdx].Kind != TokDo {
t.Errorf("doIdx points to %v, want TokDo", tokens[doIdx].Kind)
}
})
}
}

func TestScanKeywordOptionValue(t *testing.T) {
source := []byte("alias Foo.Bar, as: Baz")
tokens := Tokenize(source)
Expand Down
Loading