diff --git a/README.md b/README.md index 5d0745b..96bbef2 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,7 @@ vim.lsp.config('dexter', { filetypes = { 'elixir', 'eelixir', 'heex' }, init_options = { followDelegates = true, -- jump through defdelegate to the target function + -- definitionStyle = "all", -- "all" returns all function heads; "first" jumps to the first one -- stdlibPath = "", -- override Elixir stdlib path (auto-detected) -- debug = false, -- verbose logging to stderr (view with :LspLog) }, @@ -270,6 +271,21 @@ If Zed shows a *"could not detect Elixir stdlib"* warning on startup — common Equivalently, set the `DEXTER_ELIXIR_LIB_ROOT` environment variable via `lsp.dexter.binary.env`. +To configure other LSP options, such as returning only the first matching function head, add them to the same `initialization_options` object (see [LSP options](#lsp-options)): + +```json +{ + "lsp": { + "dexter": { + "initialization_options": { + "followDelegates": true, + "definitionStyle": "first" + } + } + } +} +``` + ### Emacs The emacs instructions assume you're using **use-package**. @@ -566,6 +582,7 @@ If the persistent process can't start, dexter falls back to running `mix format` Dexter reads `initializationOptions` from your editor configuration: - **`followDelegates`** (boolean, default: `true`): follow `defdelegate` targets on lookup. +- **`definitionStyle`** (string, default: `"all"`): controls how many locations are returned when a function has multiple heads (clauses). `"all"` returns every definition site; `"first"` returns only the first one, which makes editors like Zed jump directly instead of showing a picker. - **`stdlibPath`** (string): override the Elixir stdlib directory to index. Defaults to auto-detection; use this if your install is non-standard. - **`debug`** (boolean, default: `false`): enable verbose logging for this editor session. Logs timing and resolution details for every definition, hover, references, and rename request to your editor's LSP log and to the workspace daemon's log (see [Debugging](#debugging)). Can also be enabled via the `DEXTER_DEBUG=true` environment variable. - **`maxTransientDocuments`** (integer, default: `50`): cap on how many lazily-loaded buffers the server retains in memory. When an LSP client (e.g. Claude Code) queries a file it never opened via `didOpen`, dexter reads it from disk and caches it. Editor-owned buffers are unaffected; only disk-loaded entries are subject to LRU eviction. Set to `0` to disable transient caching. diff --git a/internal/lsp/elixir.go b/internal/lsp/elixir.go index 6e3f525..ca6972c 100644 --- a/internal/lsp/elixir.go +++ b/internal/lsp/elixir.go @@ -68,6 +68,182 @@ func (tf *TokenizedFile) FullExpressionAtCursor(line, col int) CursorContext { return ctx } +// ArityAtCallsite returns the call arity at the given expression position, or +// -1 when arity can't be determined. Handles: +// - Foo.bar(a, b) → 2 +// - Foo.bar() → 0 +// - &Foo.bar/2 → 2 (capture syntax) +// - x |> Foo.bar(y) → 2 (pipe injects one implicit arg) +// - Foo.bar → -1 (no call suffix, arity unknown) +// +// line is 0-based. startCol/endCol are the expression's 0-based column bounds +// (as returned in CursorContext.ExprStart/ExprEnd). +func (tf *TokenizedFile) ArityAtCallsite(line, startCol, endCol int) int { + endOffset := parser.LineColToOffset(tf.lineStarts, line, endCol-1) + if endOffset >= 0 && parser.TokenAtOffset(tf.interp, endOffset) >= 0 { + // The interpolation stream intentionally contains only references, not + // the delimiters required to determine a call's arity. + return -1 + } + return arityAtCallsite(tf.tokens, tf.source, tf.lineStarts, line, startCol, endCol) +} + +func arityAtCallsite(tokens []parser.Token, source []byte, lineStarts []int, line, startCol, endCol int) int { + n := len(tokens) + if n == 0 || endCol <= 0 { + return -1 + } + + // Locate the last token of the expression (index of the char at endCol-1). + endOffset := parser.LineColToOffset(lineStarts, line, endCol-1) + if endOffset < 0 { + return -1 + } + endIdx := parser.TokenAtOffset(tokens, endOffset) + if endIdx < 0 { + return -1 + } + + w := parser.NewTokenWalker(source, tokens) + w.SetPos(endIdx + 1) + w.SkipToNextSig() + j := w.Pos() + + arity := -1 + switch { + case j < n && tokens[j].Kind == parser.TokOpenParen: + var closeIdx int + arity, closeIdx = countCallArgs(source, tokens, j) + if arity >= 0 { + w.SetPos(closeIdx + 1) + w.SkipToNextSig() + if w.CurrentKind() == parser.TokDo { + startOffset := parser.LineColToOffset(lineStarts, line, startCol) + startIdx := parser.TokenAtOffset(tokens, startOffset) + prev := w.PreviousSigPos(startIdx) + if prev >= 0 && tokenCanOwnFollowingExpression(tokens[prev].Kind) { + return -1 + } + arity++ + } + } + case j < n && tokens[j].Kind == parser.TokOther && + tokens[j].End-tokens[j].Start == 1 && source[tokens[j].Start] == '/': + // Capture syntax: &Foo.bar/2 + startOffset := parser.LineColToOffset(lineStarts, line, startCol) + startIdx := parser.TokenAtOffset(tokens, startOffset) + prev := w.PreviousSigPos(startIdx) + if prev < 0 || tokens[prev].Kind != parser.TokOther || + tokens[prev].End-tokens[prev].Start != 1 || source[tokens[prev].Start] != '&' { + return -1 + } + w.SetPos(j + 1) + w.SkipToNextSig() + k := w.Pos() + if k < n && tokens[k].Kind == parser.TokNumber { + if a, ok := parseNumberTokenArity(source, tokens[k]); ok { + arity = a + } + } + } + + if arity < 0 { + return -1 + } + + // Pipe adjustment: if the expression is the RHS of a |>, add one for the + // implicit first argument. + startOffset := parser.LineColToOffset(lineStarts, line, startCol) + if startOffset >= 0 { + startIdx := parser.TokenAtOffset(tokens, startOffset) + if prev := w.PreviousSigPos(startIdx); prev >= 0 && tokens[prev].Kind == parser.TokPipe { + return arity + 1 + } + } + + return arity +} + +// countCallArgs counts top-level arguments inside a parenthesized call, +// starting at openIdx which must be a TokOpenParen. It returns the arity and +// matching close-token index, or -1, -1 when the expression is unbalanced. +func countCallArgs(source []byte, tokens []parser.Token, openIdx int) (int, int) { + if openIdx >= len(tokens) || tokens[openIdx].Kind != parser.TokOpenParen { + return -1, -1 + } + w := parser.NewTokenWalker(source, tokens) + w.SetPos(openIdx) + w.Advance() + args := 0 + hasContent := false + keywordTail := false + for w.More() { + pos := w.Pos() + kind := w.CurrentKind() + switch kind { + case parser.TokCloseParen, parser.TokCloseBracket, parser.TokCloseBrace, parser.TokCloseAngle: + if w.Depth() == 1 && w.BlockDepth() == 0 { + if hasContent { + return args + 1, pos + } + return 0, pos + } + case parser.TokComma: + if w.Depth() == 1 && w.BlockDepth() == 0 { + // Elixir's trailing keyword syntax is one list argument even + // though its entries are separated by top-level commas. + if keywordTail { + w.Advance() + continue + } + args++ + hasContent = false + w.Advance() + continue + } + hasContent = true + case parser.TokColon: + prev := w.PreviousSigPos(pos) + if w.Depth() == 1 && w.BlockDepth() == 0 && prev > openIdx && tokens[prev].Kind == parser.TokIdent { + keywordTail = true + } + hasContent = true + case parser.TokEOL, parser.TokComment: + // skip + default: + hasContent = true + } + w.Advance() + } + return -1, -1 +} + +func tokenCanOwnFollowingExpression(kind parser.TokenKind) bool { + switch kind { + case parser.TokIdent, parser.TokModule, parser.TokNumber, parser.TokString, + parser.TokHeredoc, parser.TokSigil, parser.TokCharLiteral, parser.TokAtom, + parser.TokCloseParen, parser.TokCloseBracket, parser.TokCloseBrace, parser.TokCloseAngle: + return true + default: + return false + } +} + +func parseNumberTokenArity(source []byte, t parser.Token) (int, bool) { + text := source[t.Start:t.End] + n := 0 + for _, b := range text { + if b < '0' || b > '9' { + return 0, false + } + n = n*10 + int(b-'0') + if n > 255 { // arity fits in a byte in practice + return 0, false + } + } + return n, true +} + // FirstDefmodule returns the first defmodule name found, or "". func (tf *TokenizedFile) FirstDefmodule() string { for i := 0; i < tf.n; i++ { @@ -116,6 +292,59 @@ func (tf *TokenizedFile) FindTypeDefinition(functionName string) (int, bool) { return tf.findDefinition(functionName, true) } +// FindDefinitionLines returns all matching callable or type definition lines. +// An arity below zero keeps every arity. preferType selects the namespace to +// prefer when a type and callable share a name. +func (tf *TokenizedFile) FindDefinitionLines(functionName string, arity int, preferType bool) []int { + var functionLines, typeLines []int + w := parser.NewTokenWalker(tf.source, tf.tokens) + for w.More() { + i := w.Pos() + tok := w.Current() + w.Advance() + switch tok.Kind { + case parser.TokDef, parser.TokDefp, parser.TokDefmacro, parser.TokDefmacrop, + parser.TokDefguard, parser.TokDefguardp, parser.TokDefdelegate: + name, j, ok := parser.StaticDeclarationName(tf.source, tf.tokens, tf.n, i) + if !ok || name != functionName { + continue + } + maxArity, defaultCount := 0, 0 + pj := tokNextSig(tf.tokens, tf.n, j+1) + if pj < tf.n && tf.tokens[pj].Kind == parser.TokOpenParen { + maxArity, defaultCount, _, _ = parser.CollectParams(tf.source, tf.tokens, tf.n, pj) + } + if arity < 0 || (arity >= maxArity-defaultCount && arity <= maxArity) { + functionLines = append(functionLines, tok.Line) + } + + case parser.TokAttrType: + name, j, ok := parser.StaticDeclarationName(tf.source, tf.tokens, tf.n, i) + if !ok || name != functionName { + continue + } + typeArity := 0 + pj := tokNextSig(tf.tokens, tf.n, j+1) + if pj < tf.n && tf.tokens[pj].Kind == parser.TokOpenParen { + typeArity, _, _, _ = parser.CollectParams(tf.source, tf.tokens, tf.n, pj) + } + if arity < 0 || arity == typeArity { + typeLines = append(typeLines, tok.Line) + } + } + } + if preferType { + if len(typeLines) > 0 { + return typeLines + } + return functionLines + } + if len(functionLines) > 0 { + return functionLines + } + return typeLines +} + // findDefinition returns the line of the first matching definition. A module // may declare both a type and a function under one name — Ecto.Schema has // `@type schema` above `defmacro schema/2` — so file order alone cannot decide diff --git a/internal/lsp/elixir_test.go b/internal/lsp/elixir_test.go index 1ed4308..307698f 100644 --- a/internal/lsp/elixir_test.go +++ b/internal/lsp/elixir_test.go @@ -437,6 +437,78 @@ func TestExpressionAtCursor_ExprBounds(t *testing.T) { } } +func TestArityAtCallsite_KeywordTailCountsAsOneArgument(t *testing.T) { + code := "SharedLib.Repo.insert(changeset, returning: true, on_conflict: :replace)" + tf := NewTokenizedFile(code) + ctx := tf.ExpressionAtCursor(0, strings.Index(code, "insert")+2) + if got := tf.ArityAtCallsite(0, ctx.ExprStart, ctx.ExprEnd); got != 2 { + t.Fatalf("ArityAtCallsite() = %d, want 2", got) + } +} + +func TestArityAtCallsite_ComplexForms(t *testing.T) { + tests := []struct { + name string + code string + want int + }{ + { + name: "commas in fn body do not add arguments", + code: "SharedLib.Worker.run(fn left, right -> {left, right} end)", + want: 1, + }, + { + name: "trailing do block is a keyword list argument", + code: "SharedLib.Worker.run(:value) do\n :ok\nend", + want: 2, + }, + { + name: "inline do keyword tail is one argument", + code: "SharedLib.Worker.run(:value, do: :ok, else: :error)", + want: 2, + }, + { + name: "slash without capture is ambiguous", + code: "SharedLib.Worker.run / 2", + want: -1, + }, + { + name: "capture slash supplies arity", + code: "&SharedLib.Worker.run/2", + want: 2, + }, + { + name: "outer block ownership is ambiguous", + code: "if SharedLib.Worker.run(:value) do\n :ok\nend", + want: -1, + }, + { + name: "parenthesis-free call is ambiguous", + code: "SharedLib.Worker.run :value, mode: :fast", + want: -1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tf := NewTokenizedFile(tt.code) + ctx := tf.ExpressionAtCursor(0, strings.Index(tt.code, "run")+1) + if got := tf.ArityAtCallsite(0, ctx.ExprStart, ctx.ExprEnd); got != tt.want { + t.Fatalf("ArityAtCallsite() = %d, want %d", got, tt.want) + } + }) + } +} + +func TestArityAtCallsite_InterpolationIsAmbiguous(t *testing.T) { + code := `"#{SharedLib.Worker.run(:value)}"` + tf := NewTokenizedFile(code) + ctx := tf.ExpressionAtCursor(0, strings.Index(code, "run")+1) + if got := tf.ArityAtCallsite(0, ctx.ExprStart, ctx.ExprEnd); got != -1 { + t.Fatalf("ArityAtCallsite() = %d, want -1", got) + } +} + func TestCursorContext_Expr(t *testing.T) { tests := []struct { mod, fn, want string diff --git a/internal/lsp/name_navigation.go b/internal/lsp/name_navigation.go index 4e380f0..5baf28f 100644 --- a/internal/lsp/name_navigation.go +++ b/internal/lsp/name_navigation.go @@ -1,6 +1,9 @@ package lsp -import "github.com/remoteoss/dexter/internal/store" +import ( + "github.com/remoteoss/dexter/internal/beam" + "github.com/remoteoss/dexter/internal/store" +) // NameKind selects which Elixir namespace a canonical name refers to. type NameKind uint8 @@ -26,6 +29,8 @@ type NameLookupOptions struct { External bool FallbackToModule bool ExcludeStdlib bool + Arity int + ExactArity bool // ExactModule places a generated function only at its own module's // definition. Without it, a module that exists only as a BEAM, such as // Phoenix route helpers, resolves to the nearest lexical parent with source; @@ -52,8 +57,12 @@ func (s *Server) LookupName(module, function string, opts NameLookupOptions) ([] var results []store.LookupResult var err error - if opts.FollowDelegates { + if opts.FollowDelegates && opts.ExactArity { + results, err = s.store.LookupFollowDelegateByArity(module, function, opts.Arity) + } else if opts.FollowDelegates { results, err = s.store.LookupFollowDelegate(module, function) + } else if opts.ExactArity { + results, err = s.store.LookupFunctionByArity(module, function, opts.Arity) } else { results, err = s.store.LookupFunction(module, function) } @@ -65,10 +74,20 @@ func (s *Server) LookupName(module, function string, opts NameLookupOptions) ([] results = filterOutPrivate(results) } if len(results) == 0 { - results = filterLookupKind(s.lookupThroughUseOfWithFollow(module, function, opts.FollowDelegates), opts.Kind) + arity := -1 + if opts.ExactArity { + arity = opts.Arity + } + results = filterLookupKind(s.lookupThroughUseOfWithFollow(module, function, opts.FollowDelegates, arity), opts.Kind) } if len(results) == 0 && opts.Kind != NameKindType { if generated, found := s.generatedSymbol(module, "", function); found && len(generated) > 0 { + if opts.ExactArity { + generated = filterGeneratedFunctionsByArity(generated, opts.Arity) + } + if len(generated) == 0 { + return nil, nil + } if opts.ExactModule { if results, err = s.store.LookupModule(module); err != nil { return nil, err @@ -91,6 +110,16 @@ func (s *Server) LookupName(module, function string, opts NameLookupOptions) ([] return s.lookupLocations(results, opts.ExcludeStdlib), nil } +func filterGeneratedFunctionsByArity(functions []beam.Function, arity int) []beam.Function { + filtered := make([]beam.Function, 0, len(functions)) + for _, function := range functions { + if function.Arity == arity { + filtered = append(filtered, function) + } + } + return filtered +} + // ReferenceNames finds references after a frontend has resolved a canonical // module/function name. Cursor-specific alias and variable resolution stays in // the LSP adapter. diff --git a/internal/lsp/server.go b/internal/lsp/server.go index afc3241..acf31e9 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -147,6 +147,7 @@ type Server struct { clientLog *clientLog // forwards this session's log lines to its editor followDelegates bool debug bool + definitionStyle string // "all" (default) or "first": controls multi-head definition results mixBin string // resolved path to the mix binary beams map[string]*beamProcess // build root → persistent BEAM process @@ -221,6 +222,7 @@ func NewServerWithOptions(s *store.Store, projectRoot string, opts ServerOptions projectRoot: projectRoot, explicitRoot: projectRoot != "", followDelegates: true, + definitionStyle: "all", // Read here as well as in Initialize: the daemon's headless service // answers CLI and MCP calls and never receives an initialize request. debug: os.Getenv("DEXTER_DEBUG") == "true", @@ -800,6 +802,11 @@ func (s *Server) Initialize(ctx context.Context, params *protocol.InitializePara if v, ok := opts["maxTransientDocuments"].(float64); ok { s.docs.SetMaxTransient(int(v)) } + if v, ok := opts["definitionStyle"].(string); ok { + if v == "all" || v == "first" { + s.definitionStyle = v + } + } } if os.Getenv("DEXTER_DEBUG") == "true" { s.debug = true @@ -1124,6 +1131,7 @@ func (s *Server) Definition(ctx context.Context, params *protocol.DefinitionPara expr := tf.ResolveModuleExpr(exprCtx.Expr(), lineNum) moduleRef, functionName := ExtractModuleAndFunction(expr) + callArity := tf.ArityAtCallsite(lineNum, exprCtx.ExprStart, exprCtx.ExprEnd) if moduleRef != "" { if aliasParent, inBlock := tf.ExtractAliasBlockParent(lineNum); inBlock { @@ -1133,7 +1141,7 @@ func (s *Server) Definition(ctx context.Context, params *protocol.DefinitionPara aliases := tf.ExtractAliasesInScope(lineNum) s.mergeAliasesFromUseTokenized(tf, aliases) - s.debugf("Definition: expr=%q module=%q function=%q", expr, moduleRef, functionName) + s.debugf("Definition: expr=%q module=%q function=%q arity=%d", expr, moduleRef, functionName, callArity) // Bare identifier — check variable first (cheap tree-sitter lookup), then functions if moduleRef == "" { @@ -1174,15 +1182,16 @@ func (s *Server) Definition(ctx context.Context, params *protocol.DefinitionPara // Current module — return buffer location directly (works before indexing). // In a typespec the bare name is the type, everywhere else the function. if fullModule == currentModule { - find := tf.FindFunctionDefinition - if tf.InTypespec(lineNum) { - find = tf.FindTypeDefinition - } - if line, found := find(functionName); found { - return []protocol.Location{{ - URI: params.TextDocument.URI, - Range: lineRange(line - 1), - }}, nil + lines := tf.FindDefinitionLines(functionName, callArity, tf.InTypespec(lineNum)) + if len(lines) > 0 { + locations := make([]protocol.Location, 0, len(lines)) + for _, line := range lines { + locations = append(locations, protocol.Location{ + URI: params.TextDocument.URI, + Range: lineRange(line - 1), + }) + } + return s.applyDefinitionStyle(locations), nil } } @@ -1194,16 +1203,18 @@ func (s *Server) Definition(ctx context.Context, params *protocol.DefinitionPara Kind: kind, FollowDelegates: s.followDelegates, External: fullModule != extractEnclosingModuleFromTokens(tf.source, tf.tokens, lineNum), + Arity: callArity, + ExactArity: callArity >= 0, }) if err == nil && len(results) > 0 { s.debugf("Definition: found %d semantic result(s) for %s.%s", len(results), fullModule, functionName) - return nameLocationsToProtocol(results), nil + return s.applyDefinitionStyle(nameLocationsToProtocol(results)), nil } // Fallback for use-chain inline defs (not stored as module definitions) - if results := s.lookupThroughUse(text, functionName, aliases); len(results) > 0 { + if results := s.lookupThroughUseWithFollow(text, functionName, aliases, s.followDelegates, callArity); len(results) > 0 { s.debugf("Definition: found %d result(s) via current file use chain for %s", len(results), functionName) - return storeResultsToLocations(byKindForContext(tf, lineNum, results)), nil + return s.applyDefinitionStyle(storeResultsToLocations(byKindForContext(tf, lineNum, results))), nil } currentModule = s.store.LookupEnclosingModule(uriToPath(protocol.DocumentURI(docURI)), lineNum+1) @@ -1235,11 +1246,13 @@ func (s *Server) Definition(ctx context.Context, params *protocol.DefinitionPara FollowDelegates: s.followDelegates, External: fullModule != extractEnclosingModuleFromTokens(tf.source, tf.tokens, lineNum), FallbackToModule: true, + Arity: callArity, + ExactArity: callArity >= 0, }) if err != nil { return nil, nil } - return nameLocationsToProtocol(results), nil + return s.applyDefinitionStyle(nameLocationsToProtocol(results)), nil } results, err := s.LookupName(fullModule, "", NameLookupOptions{}) @@ -1266,6 +1279,13 @@ func nameLocationsToProtocol(results []NameLocation) []protocol.Location { return locations } +func (s *Server) applyDefinitionStyle(locations []protocol.Location) []protocol.Location { + if s.definitionStyle == "first" && len(locations) > 1 { + return locations[:1] + } + return locations +} + func storeResultsToLocations(results []store.LookupResult) []protocol.Location { type locKey struct { filePath string @@ -2488,10 +2508,10 @@ func (s *Server) parseUsingFile(filePath, moduleName string) *usingCacheEntry { // fullModule's source file. This handles qualified calls like M.func() where // func is not defined directly in M but is injected by a macro M uses. func (s *Server) lookupThroughUseOf(fullModule, functionName string) []store.LookupResult { - return s.lookupThroughUseOfWithFollow(fullModule, functionName, s.followDelegates) + return s.lookupThroughUseOfWithFollow(fullModule, functionName, s.followDelegates, -1) } -func (s *Server) lookupThroughUseOfWithFollow(fullModule, functionName string, followDelegates bool) []store.LookupResult { +func (s *Server) lookupThroughUseOfWithFollow(fullModule, functionName string, followDelegates bool, arity int) []store.LookupResult { modResults, err := s.store.LookupModule(fullModule) if err != nil || len(modResults) == 0 { return nil @@ -2500,7 +2520,7 @@ func (s *Server) lookupThroughUseOfWithFollow(fullModule, functionName string, f if !ok { return nil } - return s.lookupThroughUseWithFollow(fileText, functionName, ExtractAliases(fileText), followDelegates) + return s.lookupThroughUseWithFollow(fileText, functionName, ExtractAliases(fileText), followDelegates, arity) } // lookupThroughUse searches for functionName in definitions injected by `use` @@ -2508,15 +2528,15 @@ func (s *Server) lookupThroughUseOfWithFollow(fullModule, functionName string, f // priority over imported ones. Later `use` declarations shadow earlier ones. // Transitive use chains (use inside __using__ body) are followed recursively. func (s *Server) lookupThroughUse(text, functionName string, aliases map[string]string) []store.LookupResult { - return s.lookupThroughUseWithFollow(text, functionName, aliases, s.followDelegates) + return s.lookupThroughUseWithFollow(text, functionName, aliases, s.followDelegates, -1) } -func (s *Server) lookupThroughUseWithFollow(text, functionName string, aliases map[string]string, followDelegates bool) []store.LookupResult { +func (s *Server) lookupThroughUseWithFollow(text, functionName string, aliases map[string]string, followDelegates bool, arity int) []store.LookupResult { useCalls := ExtractUsesWithOpts(text, aliases) visited := make(map[string]bool) for i := len(useCalls) - 1; i >= 0; i-- { - if result := s.lookupInUsingEntryForWithFollow(useCalls[i].Module, functionName, useCalls[i].dispatchAtom(), useCalls[i].Opts, visited, followDelegates); result != nil { + if result := s.lookupInUsingEntryForWithFollow(useCalls[i].Module, functionName, useCalls[i].dispatchAtom(), useCalls[i].Opts, visited, followDelegates, arity); result != nil { return result } } @@ -2528,7 +2548,7 @@ func (s *Server) lookupThroughUseWithFollow(text, functionName string, aliases m // consumerOpts are the keyword args from the `use Module, key: Val` call and // are used to resolve dynamic imports like `import unquote(mod)`. func (s *Server) lookupInUsingEntry(moduleName, functionName string, consumerOpts map[string]string, visited map[string]bool) []store.LookupResult { - return s.lookupInUsingEntryForWithFollow(moduleName, functionName, "", consumerOpts, visited, s.followDelegates) + return s.lookupInUsingEntryForWithFollow(moduleName, functionName, "", consumerOpts, visited, s.followDelegates, -1) } // bodyFor picks the injected body for a `use` call. An ordinary __using__ has a @@ -2561,10 +2581,10 @@ func usingVisitKey(moduleName, which string) string { // lookupInUsingEntryFor is lookupInUsingEntry with the dispatch atom from the // `use` site (empty for an ordinary `use Module`). func (s *Server) lookupInUsingEntryFor(moduleName, functionName, which string, consumerOpts map[string]string, visited map[string]bool) []store.LookupResult { - return s.lookupInUsingEntryForWithFollow(moduleName, functionName, which, consumerOpts, visited, s.followDelegates) + return s.lookupInUsingEntryForWithFollow(moduleName, functionName, which, consumerOpts, visited, s.followDelegates, -1) } -func (s *Server) lookupInUsingEntryForWithFollow(moduleName, functionName, which string, consumerOpts map[string]string, visited map[string]bool, followDelegates bool) []store.LookupResult { +func (s *Server) lookupInUsingEntryForWithFollow(moduleName, functionName, which string, consumerOpts map[string]string, visited map[string]bool, followDelegates bool, arity int) []store.LookupResult { visitKey := usingVisitKey(moduleName, which) if visited[visitKey] { return nil @@ -2584,18 +2604,34 @@ func (s *Server) lookupInUsingEntryForWithFollow(moduleName, functionName, which if defs, ok := body.inlineDefs[functionName]; ok { var results []store.LookupResult for _, d := range defs { - results = append(results, store.LookupResult{FilePath: entry.filePath, Line: d.line}) + if arity >= 0 && d.arity != arity { + continue + } + results = append(results, store.LookupResult{ + FilePath: entry.filePath, + Line: d.line, + Kind: d.kind, + Arity: d.arity, + }) + } + if len(results) > 0 { + return results } - return results } // Static imports for j := len(body.imports) - 1; j >= 0; j-- { var results []store.LookupResult var err error - if followDelegates { + if followDelegates && arity >= 0 { + results, err = s.store.LookupFollowDelegateByArity(body.imports[j], functionName, arity) + results = publicOnly(results) + } else if followDelegates { results, err = s.store.LookupFollowDelegate(body.imports[j], functionName) results = publicOnly(results) + } else if arity >= 0 { + results, err = s.store.LookupFunctionByArity(body.imports[j], functionName, arity) + results = publicOnly(results) } else { results, err = s.store.LookupPublicFunction(body.imports[j], functionName) } @@ -2618,9 +2654,15 @@ func (s *Server) lookupInUsingEntryForWithFollow(moduleName, functionName, which case "import": var results []store.LookupResult var err error - if followDelegates { + if followDelegates && arity >= 0 { + results, err = s.store.LookupFollowDelegateByArity(mod, functionName, arity) + results = publicOnly(results) + } else if followDelegates { results, err = s.store.LookupFollowDelegate(mod, functionName) results = publicOnly(results) + } else if arity >= 0 { + results, err = s.store.LookupFunctionByArity(mod, functionName, arity) + results = publicOnly(results) } else { results, err = s.store.LookupPublicFunction(mod, functionName) } @@ -2628,7 +2670,7 @@ func (s *Server) lookupInUsingEntryForWithFollow(moduleName, functionName, which return results } case "use": - if result := s.lookupInUsingEntryForWithFollow(mod, functionName, "", nil, visited, followDelegates); result != nil { + if result := s.lookupInUsingEntryForWithFollow(mod, functionName, "", nil, visited, followDelegates, arity); result != nil { return result } } @@ -2637,7 +2679,7 @@ func (s *Server) lookupInUsingEntryForWithFollow(moduleName, functionName, which // Transitive uses: use Module inside the __using__ body (double-use chains) for k := len(body.transCalls) - 1; k >= 0; k-- { call := body.transCalls[k] - if result := s.lookupInUsingEntryForWithFollow(call.Module, functionName, call.dispatchAtom(), call.Opts, visited, followDelegates); result != nil { + if result := s.lookupInUsingEntryForWithFollow(call.Module, functionName, call.dispatchAtom(), call.Opts, visited, followDelegates, arity); result != nil { return result } } @@ -2645,7 +2687,7 @@ func (s *Server) lookupInUsingEntryForWithFollow(moduleName, functionName, which if body.hasTransCall(body.transUses[k]) { continue } - if result := s.lookupInUsingEntryForWithFollow(body.transUses[k], functionName, "", nil, visited, followDelegates); result != nil { + if result := s.lookupInUsingEntryForWithFollow(body.transUses[k], functionName, "", nil, visited, followDelegates, arity); result != nil { return result } } diff --git a/internal/lsp/server_test.go b/internal/lsp/server_test.go index 50f1959..e308284 100644 --- a/internal/lsp/server_test.go +++ b/internal/lsp/server_test.go @@ -245,6 +245,9 @@ func TestServer_InitializationOptions(t *testing.T) { if server.debug { t.Error("debug should default to false") } + if server.definitionStyle != "all" { + t.Errorf("definitionStyle: got %q, want %q", server.definitionStyle, "all") + } }) // Claude Code plugin template substitution yields strings, not booleans. @@ -254,12 +257,13 @@ func TestServer_InitializationOptions(t *testing.T) { opts map[string]interface{} wantFollowDel bool wantDebug bool + wantStyle string }{ - {"bool true/false", map[string]interface{}{"followDelegates": false, "debug": true}, false, true}, - {"string true/false", map[string]interface{}{"followDelegates": "false", "debug": "true"}, false, true}, - {"string 1/0", map[string]interface{}{"followDelegates": "0", "debug": "1"}, false, true}, - {"empty string leaves default", map[string]interface{}{"followDelegates": "", "debug": ""}, true, false}, - {"unsupported type leaves default", map[string]interface{}{"followDelegates": 1, "debug": 0}, true, false}, + {"bool true/false", map[string]interface{}{"followDelegates": false, "debug": true, "definitionStyle": "first"}, false, true, "first"}, + {"string true/false", map[string]interface{}{"followDelegates": "false", "debug": "true", "definitionStyle": "all"}, false, true, "all"}, + {"string 1/0", map[string]interface{}{"followDelegates": "0", "debug": "1"}, false, true, "all"}, + {"empty string leaves default", map[string]interface{}{"followDelegates": "", "debug": "", "definitionStyle": ""}, true, false, "all"}, + {"unsupported values leave default", map[string]interface{}{"followDelegates": 1, "debug": 0, "definitionStyle": "bogus"}, true, false, "all"}, } for _, tc := range cases { @@ -280,10 +284,303 @@ func TestServer_InitializationOptions(t *testing.T) { if server.debug != tc.wantDebug { t.Errorf("debug: got %v, want %v", server.debug, tc.wantDebug) } + if server.definitionStyle != tc.wantStyle { + t.Errorf("definitionStyle: got %q, want %q", server.definitionStyle, tc.wantStyle) + } }) } } +func TestServer_ApplyDefinitionStyle(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + locs := []protocol.Location{ + {URI: "file:///a.ex", Range: lineRange(0)}, + {URI: "file:///a.ex", Range: lineRange(5)}, + {URI: "file:///a.ex", Range: lineRange(9)}, + } + + // Default "all" returns everything + got := server.applyDefinitionStyle(locs) + if len(got) != 3 { + t.Errorf("expected 3 locations with style %q, got %d", "all", len(got)) + } + + // "first" returns only the first + server.definitionStyle = "first" + got = server.applyDefinitionStyle(locs) + if len(got) != 1 { + t.Errorf("expected 1 location with style %q, got %d", "first", len(got)) + } + if got[0].Range.Start.Line != 0 { + t.Errorf("expected first location (line 0), got line %d", got[0].Range.Start.Line) + } + + // Single location is unaffected by "first" + got = server.applyDefinitionStyle(locs[:1]) + if len(got) != 1 { + t.Errorf("expected 1 location with style %q and single input, got %d", "first", len(got)) + } + + // Empty slice is unaffected + got = server.applyDefinitionStyle(nil) + if len(got) != 0 { + t.Errorf("expected 0 locations for nil input, got %d", len(got)) + } +} + +// --- Duplicate-location reproductions (issue #38, knoebber's follow-up) --- +// +// knoebber reported that Zed's goto-definition opens the references picker even +// when "the function is defined once in another module", implying Dexter is +// returning more than one Location in cases where a human sees a single +// definition. These tests pin the scenarios we suspect: each asserts exactly 1 +// Location from Definition(). A failure here means the handler is returning +// duplicates for a single-definition call and likely reproduces the bug. + +// Sanity baseline: one def, one caller — must be a single Location. +func TestDefinition_SingleDef_ReturnsOneLocation(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + indexFile(t, server.store, server.projectRoot, "lib/math.ex", `defmodule MyApp.Math do + def add(a, b), do: a + b +end +`) + + callerPath := filepath.Join(server.projectRoot, "lib", "caller.ex") + callerContent := `defmodule MyApp.Caller do + def run, do: MyApp.Math.add(1, 2) +end +` + indexFile(t, server.store, server.projectRoot, "lib/caller.ex", callerContent) + callerURI := "file://" + callerPath + server.docs.Set(callerURI, callerContent) + + // Cursor on "add" in MyApp.Math.add(1, 2) + locs := definitionAt(t, server, callerURI, 1, 25) + if len(locs) != 1 { + t.Fatalf("expected exactly 1 location for single def, got %d: %+v", len(locs), locs) + } +} + +// Multi-arity: def foo/1 and def foo/2 both defined once each. A call to foo/1 +// should only return the foo/1 line — but LookupFunction ignores arity, so we +// expect this to currently return 2 locations (the bug). +func TestDefinition_MultiArity_ReturnsOneLocation(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + indexFile(t, server.store, server.projectRoot, "lib/math.ex", `defmodule MyApp.Math do + def square(x), do: x * x + + def square(x, factor), do: (x * x) * factor +end +`) + + callerPath := filepath.Join(server.projectRoot, "lib", "caller.ex") + callerContent := `defmodule MyApp.Caller do + def run, do: MyApp.Math.square(3) +end +` + indexFile(t, server.store, server.projectRoot, "lib/caller.ex", callerContent) + callerURI := "file://" + callerPath + server.docs.Set(callerURI, callerContent) + + // Cursor on "square" in MyApp.Math.square(3) + locs := definitionAt(t, server, callerURI, 1, 28) + if len(locs) != 1 { + t.Fatalf("expected exactly 1 location for square/1 call, got %d — "+ + "LookupFunction is not filtering by arity: %+v", len(locs), locs) + } +} + +func TestDefinition_CurrentModuleBareCallUsesArityAndStyle(t *testing.T) { + t.Run("selects matching arity", func(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + content := `defmodule MyApp.Current do + def calculate(value), do: value + def calculate(left, right), do: left + right + def run, do: calculate(1, 2) +end +` + path := filepath.Join(server.projectRoot, "lib", "current.ex") + indexFile(t, server.store, server.projectRoot, "lib/current.ex", content) + uri := "file://" + path + server.docs.Set(uri, content) + + locs := definitionAt(t, server, uri, 3, 17) + if len(locs) != 1 || locs[0].Range.Start.Line != 2 { + t.Fatalf("expected calculate/2 on line 2, got %+v", locs) + } + }) + + t.Run("returns all same-arity heads by default", func(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + content := `defmodule MyApp.Current do + def calculate(:first), do: 1 + def calculate(:second), do: 2 + def run, do: calculate(:first) +end +` + path := filepath.Join(server.projectRoot, "lib", "current.ex") + indexFile(t, server.store, server.projectRoot, "lib/current.ex", content) + uri := "file://" + path + server.docs.Set(uri, content) + + locs := definitionAt(t, server, uri, 3, 17) + if len(locs) != 2 { + t.Fatalf("expected both calculate/1 heads, got %+v", locs) + } + }) +} + +func TestDefinition_UseChainSelectsProviderByArity(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + indexFile(t, server.store, server.projectRoot, "lib/one.ex", `defmodule SharedLib.One do + def execute(value), do: value +end +`) + indexFile(t, server.store, server.projectRoot, "lib/two.ex", `defmodule SharedLib.Two do + def execute(left, right), do: {left, right} +end +`) + indexFile(t, server.store, server.projectRoot, "lib/injector.ex", `defmodule SharedLib.Injector do + defmacro __using__(_opts) do + quote do + import SharedLib.One + import SharedLib.Two + end + end +end +`) + + callerPath := filepath.Join(server.projectRoot, "lib", "consumer.ex") + callerContent := `defmodule MyApp.Consumer do + use SharedLib.Injector + def run, do: execute(:value) +end +` + indexFile(t, server.store, server.projectRoot, "lib/consumer.ex", callerContent) + callerURI := "file://" + callerPath + server.docs.Set(callerURI, callerContent) + + locs := definitionAt(t, server, callerURI, 2, 16) + if len(locs) != 1 || !strings.HasSuffix(string(locs[0].URI), "/lib/one.ex") { + t.Fatalf("expected SharedLib.One.execute/1, got %+v", locs) + } +} + +func TestDefinition_KeywordTailCountsAsOneArgument(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + indexFile(t, server.store, server.projectRoot, "lib/repo.ex", `defmodule SharedLib.Repo do + def insert(changeset, opts), do: {changeset, opts} + def insert(changeset, opts, metadata), do: {changeset, opts, metadata} +end +`) + + callerPath := filepath.Join(server.projectRoot, "lib", "caller.ex") + callerContent := `defmodule MyApp.Caller do + def run(changeset), do: SharedLib.Repo.insert(changeset, returning: true, on_conflict: :replace) +end +` + indexFile(t, server.store, server.projectRoot, "lib/caller.ex", callerContent) + callerURI := "file://" + callerPath + server.docs.Set(callerURI, callerContent) + + locs := definitionAt(t, server, callerURI, 1, 44) + if len(locs) != 1 { + t.Fatalf("expected exactly 1 location for insert/2, got %d: %+v", len(locs), locs) + } + if got := locs[0].Range.Start.Line; got != 1 { + t.Fatalf("expected keyword tail to resolve insert/2 on line 1, got line %d", got) + } +} + +// defdelegate + def with the same name in the same module. The caller writes +// Mod.do_thing(x); the human reads this as "one definition" (the delegate) but +// LookupFollowDelegate returns both the defdelegate line and the def line +// because allDelegates is false. +func TestDefinition_DelegateAndDefSameName_ReturnsOneLocation(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + indexFile(t, server.store, server.projectRoot, "lib/worker.ex", `defmodule MyApp.Worker do + def call(attrs), do: {:ok, attrs} +end +`) + + indexFile(t, server.store, server.projectRoot, "lib/api.ex", `defmodule MyApp.Api do + defdelegate do_thing(x), to: MyApp.Worker, as: :call + + def do_thing(x, y), do: {x, y} +end +`) + + callerPath := filepath.Join(server.projectRoot, "lib", "caller.ex") + callerContent := `defmodule MyApp.Caller do + def run, do: MyApp.Api.do_thing("hello") +end +` + indexFile(t, server.store, server.projectRoot, "lib/caller.ex", callerContent) + callerURI := "file://" + callerPath + server.docs.Set(callerURI, callerContent) + + // Cursor on "do_thing" in MyApp.Api.do_thing("hello") + locs := definitionAt(t, server, callerURI, 1, 27) + if len(locs) != 1 { + t.Fatalf("expected exactly 1 location for delegate-then-def call, got %d — "+ + "LookupFollowDelegate returns both the defdelegate and def rows: %+v", len(locs), locs) + } +} + +// Multiple heads of the same arity — the original PR #39 scenario. This is +// *not* a bug; it's what Jesse called "a feature". The test documents the +// current behavior: all heads returned with style="all", only first with +// style="first". +func TestDefinition_MultipleHeadsSameArity_StyleControlled(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + indexFile(t, server.store, server.projectRoot, "lib/accounts.ex", `defmodule MyApp.Accounts do + def fetch_user(%{id: id}), do: id + def fetch_user(email) when is_binary(email), do: email + def fetch_user(id) when is_integer(id), do: id +end +`) + + callerPath := filepath.Join(server.projectRoot, "lib", "caller.ex") + callerContent := `defmodule MyApp.Caller do + def run, do: MyApp.Accounts.fetch_user("nick@example.com") +end +` + indexFile(t, server.store, server.projectRoot, "lib/caller.ex", callerContent) + callerURI := "file://" + callerPath + server.docs.Set(callerURI, callerContent) + + // Cursor on "fetch_user" in MyApp.Accounts.fetch_user("...") + locs := definitionAt(t, server, callerURI, 1, 32) + if len(locs) < 2 { + t.Fatalf("expected multiple locations for 3 function heads with style=all, got %d", len(locs)) + } + + // With style="first", caller should see only the first head. + server.definitionStyle = "first" + locs = definitionAt(t, server, callerURI, 1, 32) + if len(locs) != 1 { + t.Fatalf("expected exactly 1 location with definitionStyle=first, got %d", len(locs)) + } +} + func definitionAt(t *testing.T, server *Server, uri string, line, col uint32) []protocol.Location { t.Helper() result, err := server.Definition(context.Background(), &protocol.DefinitionParams{ diff --git a/internal/parser/token_walk_test.go b/internal/parser/token_walk_test.go index 42db635..1f16039 100644 --- a/internal/parser/token_walk_test.go +++ b/internal/parser/token_walk_test.go @@ -35,6 +35,26 @@ func TestTrackBlockDepth(t *testing.T) { } } +func TestTokenWalker_PreviousSigPos(t *testing.T) { + source := []byte("left\n# comment\n|> right") + tokens := Tokenize(source) + w := NewTokenWalker(source, tokens) + right := -1 + for i, token := range tokens { + if TokenText(source, token) == "right" { + right = i + break + } + } + if right < 0 { + t.Fatal("right token not found") + } + prev := w.PreviousSigPos(right) + if prev < 0 || tokens[prev].Kind != TokPipe { + t.Fatalf("previous significant token = %d, want pipe", prev) + } +} + func TestAliasShortName(t *testing.T) { tests := []struct { in string diff --git a/internal/parser/token_walker.go b/internal/parser/token_walker.go index 1ab2538..ebf2a6b 100644 --- a/internal/parser/token_walker.go +++ b/internal/parser/token_walker.go @@ -124,6 +124,18 @@ func (w *TokenWalker) NextSigPos() int { return NextSigToken(w.Tokens, w.N, w.pos) } +// PreviousSigPos returns the previous significant token before before, or -1. +// EOL and comment tokens are skipped. +func (w *TokenWalker) PreviousSigPos(before int) int { + for i := before - 1; i >= 0; i-- { + kind := w.Tokens[i].Kind + if kind != TokEOL && kind != TokComment { + return i + } + } + return -1 +} + // Depth returns the current bracket depth. func (w *TokenWalker) Depth() int { return w.depth diff --git a/internal/store/store.go b/internal/store/store.go index 1e6f5fd..10702c6 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -1130,6 +1130,21 @@ func (s *Store) LookupPublicFunction(module, function string) ([]LookupResult, e ) } +// LookupFunctionByArity returns definitions for a function name, optionally +// filtered by arity. When arity < 0, behavior matches LookupFunction (all +// arities returned). When arity >= 0, results are filtered to exact matches, +// preventing callers from getting e.g. `foo/1` and `foo/2` rows for a call +// that only uses one of them. +func (s *Store) LookupFunctionByArity(module, function string, arity int) ([]LookupResult, error) { + if arity < 0 { + return s.LookupFunction(module, function) + } + return s.queryLookup( + "SELECT f.path, d.line, d.kind, d.arity, d.delegate_to, d.delegate_as FROM definitions d JOIN files f ON f.id = d.file_id WHERE d.module = ? AND d.function = ? AND d.arity = ? AND d.kind NOT IN ('module', 'defprotocol', 'defimpl', 'callback', 'macrocallback') ORDER BY CASE WHEN d.kind IN ('type', 'opaque') THEN 1 ELSE 0 END, d.line", + module, function, arity, + ) +} + // CallbackResult holds a @callback or @macrocallback definition with its arity. type CallbackResult struct { FilePath string @@ -1688,42 +1703,68 @@ func (s *Store) NextFunctionLine(filePath string, startLine int) int { } func (s *Store) LookupFollowDelegate(module, function string) ([]LookupResult, error) { - return s.lookupFollowDelegate(module, function, 0) + return s.LookupFollowDelegateByArity(module, function, -1) } -func (s *Store) lookupFollowDelegate(module, function string, depth int) ([]LookupResult, error) { +// LookupFollowDelegateByArity is like LookupFollowDelegate but filters by +// arity when arity >= 0. Delegate-following is partitioned per arity, so a +// module that mixes `defdelegate foo/1, to: X` with `def foo/2` still follows +// the /1 delegate for a /1 call without being blocked by the non-delegate /2 +// row. +func (s *Store) LookupFollowDelegateByArity(module, function string, arity int) ([]LookupResult, error) { + return s.lookupFollowDelegate(module, function, arity, 0) +} + +func (s *Store) lookupFollowDelegate(module, function string, arity, depth int) ([]LookupResult, error) { if depth > 5 { return nil, nil } - results, err := s.LookupFunction(module, function) + results, err := s.LookupFunctionByArity(module, function, arity) if err != nil { return nil, err } + if len(results) == 0 { + return nil, nil + } - // If all results are defdelegates, follow them to the target - allDelegates := len(results) > 0 + // Group by arity. Within each arity group, follow if every row is a + // delegate. This handles the defdelegate/1 + def/2 mix correctly. + byArity := make(map[int][]LookupResult, len(results)) + order := make([]int, 0, len(results)) for _, r := range results { - if r.Kind != "defdelegate" || r.DelegateTo == "" { - allDelegates = false - break + if _, seen := byArity[r.Arity]; !seen { + order = append(order, r.Arity) } + byArity[r.Arity] = append(byArity[r.Arity], r) } - if allDelegates { - targetModule := results[0].DelegateTo - targetFunc := function - if results[0].DelegateAs != "" { - targetFunc = results[0].DelegateAs - } - targetResults, err := s.lookupFollowDelegate(targetModule, targetFunc, depth+1) - if err != nil { - return nil, err + var out []LookupResult + for _, a := range order { + group := byArity[a] + allDelegates := true + for _, r := range group { + if r.Kind != "defdelegate" || r.DelegateTo == "" { + allDelegates = false + break + } } - if len(targetResults) > 0 { - return targetResults, nil + if allDelegates { + targetModule := group[0].DelegateTo + targetFunc := function + if group[0].DelegateAs != "" { + targetFunc = group[0].DelegateAs + } + targetResults, err := s.lookupFollowDelegate(targetModule, targetFunc, a, depth+1) + if err != nil { + return nil, err + } + if len(targetResults) > 0 { + out = append(out, targetResults...) + continue + } } + out = append(out, group...) } - - return results, nil + return out, nil }