From de7b63ff57cf8724b62a78a76fd34c9515375f0f Mon Sep 17 00:00:00 2001 From: Jesse Herrick Date: Fri, 5 Jun 2026 18:55:02 -0400 Subject: [PATCH] Fix outline depth tracking --- internal/lsp/server.go | 8 ++- internal/lsp/server_test.go | 87 +++++++++++++++++++++++++++++ internal/parser/parser_tokenized.go | 39 ++----------- internal/parser/token_walk.go | 53 ++++++++++++++++++ internal/parser/token_walk_test.go | 81 +++++++++++++++++++++++++++ 5 files changed, 230 insertions(+), 38 deletions(-) diff --git a/internal/lsp/server.go b/internal/lsp/server.go index ec0d5cd..c2fd345 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -3149,7 +3149,7 @@ 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 } @@ -3157,10 +3157,12 @@ func (s *Server) DocumentSymbol(ctx context.Context, params *protocol.DocumentSy 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] } diff --git a/internal/lsp/server_test.go b/internal/lsp/server_test.go index db7c850..4477684 100644 --- a/internal/lsp/server_test.go +++ b/internal/lsp/server_test.go @@ -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() diff --git a/internal/parser/parser_tokenized.go b/internal/parser/parser_tokenized.go index f18b61e..cf8d527 100644 --- a/internal/parser/parser_tokenized.go +++ b/internal/parser/parser_tokenized.go @@ -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 { diff --git a/internal/parser/token_walk.go b/internal/parser/token_walk.go index 09321a9..b7bcf07 100644 --- a/internal/parser/token_walk.go +++ b/internal/parser/token_walk.go @@ -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 +} + // TrackBlockDepth updates the block depth counter for do/fn/end tokens. func TrackBlockDepth(kind TokenKind, depth *int) { switch kind { diff --git a/internal/parser/token_walk_test.go b/internal/parser/token_walk_test.go index 0bb92c2..8699683 100644 --- a/internal/parser/token_walk_test.go +++ b/internal/parser/token_walk_test.go @@ -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)