From 55039df548cf847926c327ef216afd9aa6f0ded9 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Tue, 25 Aug 2026 11:09:30 -0500 Subject: [PATCH] fix(erlang): give same-name different-arity functions separate arity-qualified nodes (#1610) Arity is part of an Erlang function's identity: f/1 and f/2 are unrelated definitions. The extractor merged consecutive same-name fun_decls regardless of arity, so both landed in one node (or two nodes with colliding qualified names when interleaved), the wrong -spec/signature/span was attributed, the everyday f/N -> f/N+1 delegation became a self-loop, and -export([f/1]) marked every arity exported. - Each (name, arity) group now gets its own node; qualifiedName carries the canonical spelling (mod::f/1). Node names stay bare for search. - -export and preceding -spec attribution are per-arity. - Call/fun refs carry the call-site arity (f/1, mod::f/2, fun mod:f/1, gen_server handle_call/3 + handle_cast/2, static MFA lists); the matcher resolves them only to a def of exactly that arity, same-file first, and refuses to guess a sibling arity (silent beats wrong). Arity-less dynamic MFA refs resolve only when a single arity exists. - The behaviour dispatch synthesizer selects the implementer node of the site's arity, and erlangArityAt now skips <> literal commas per its own docstring (#1358) - dispatch sites passing binaries no longer miscount. - Explore/node symbol lookups accept the written mod:fn/3 spelling against the new arity-qualified names. Cowboy: nodes 3668 -> 3748 (+80 arity splits, no explosion), behaviour dispatch edges 38 -> 44, cowboy_req header/2 and header/3 split with the right specs and a real /2 -> /3 delegation edge. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LxZj6W6Y1SHXwvpT3uwJpK --- CHANGELOG.md | 4 + __tests__/erlang-arity-resolution.test.ts | 147 +++++++++++++++++ .../erlang-behaviour-synthesizer.test.ts | 41 +++++ __tests__/extraction.test.ts | 155 ++++++++++++++---- src/db/queries.ts | 7 +- src/extraction/languages/erlang.ts | 77 ++++++--- src/extraction/tree-sitter.ts | 45 +++-- src/mcp/tools.ts | 22 ++- src/resolution/callback-synthesizer.ts | 34 +++- src/resolution/index.ts | 5 +- src/resolution/name-matcher.ts | 75 +++++++++ 11 files changed, 537 insertions(+), 75 deletions(-) create mode 100644 __tests__/erlang-arity-resolution.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b2d07fb3..250fcd123 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- Erlang functions that share a name but differ in arity are now separate symbols with the language's own `module:fun/arity` identity, so the everyday `f/1` delegating to `f/2` shows as a real call edge instead of a self-loop, each arity keeps its own `-spec` and source span, `-export([f/1])` marks exactly that arity as public, and asking `codegraph_explore` for a symbol the way Erlang spells it — `cowboy_req:header/3` — returns that definition. Re-index Erlang projects after upgrading. Thanks @Dshuishui. (#1610) (Erlang) + +- Erlang behaviour dispatch no longer miscounts a call site's arity when an argument is a binary literal like `<<1,2,3>>` — the commas inside were counted as argument separators, which silently dropped (or could mislink) the dispatch edge to the behaviour callback. (#1358) (Erlang) + - Indexing no longer hangs on a Swift Vapor project containing a call with a long argument list. A single `.get(...)`-style call with many labeled arguments and no `use:` handler — the shape generated request builders produce — could stall `codegraph index`, `codegraph sync`, and the MCP server indefinitely. Route detection now handles such files in milliseconds, and every previously-recognized route shape still parses exactly as before. Thanks @maxmilian. (#1544) (Swift) - `codegraph status` now sees new files inside brand-new directories. Git reports an entirely-untracked directory as a single collapsed entry, so source files created there — a freshly scaffolded `frontend/`, for example — were missing from the pending-changes report, which could claim everything was up to date while those files had not yet been indexed. Thanks @maxmilian. (#1213) diff --git a/__tests__/erlang-arity-resolution.test.ts b/__tests__/erlang-arity-resolution.test.ts new file mode 100644 index 000000000..9351f81b3 --- /dev/null +++ b/__tests__/erlang-arity-resolution.test.ts @@ -0,0 +1,147 @@ +/** + * Erlang arity-aware resolution (#1610). + * + * Arity is part of a function's identity: `f/1` and `f/2` are unrelated + * definitions. Extraction gives each arity its own node (`mod::f/1`) and + * stamps refs with the call-site arity; resolution must land each ref on the + * def of exactly that arity — the everyday `header/2 -> header/3` delegation + * must be a real edge, never a self-loop — and refuse to guess a sibling + * arity when the named one doesn't exist. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { CodeGraph } from '../src'; + +describe('erlang arity-aware resolution', () => { + let dir: string; + beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'erlang-arity-')); }); + afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); + + async function callEdges(d: string): Promise> { + const cg = await CodeGraph.init(d, { silent: true }); + await cg.indexAll(); + const db = (cg as any).db.db; + const rows = db + .prepare( + `SELECT s.qualified_name sq, t.qualified_name tq + FROM edges e JOIN nodes s ON s.id = e.source JOIN nodes t ON t.id = e.target + WHERE e.kind IN ('calls','references') AND s.kind = 'function'` + ) + .all(); + cg.destroy(); + return rows; + } + + it('resolves the f/N -> f/N+1 delegation to a real edge, not a self-loop', async () => { + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'deleg.erl'), + `-module(deleg). +-export([header/2]). + +header(Name, Req) -> + header(Name, Req, undefined). + +-spec header(binary(), map(), any()) -> any(). +header(Name, Headers, Default) -> + maps:get(Name, Headers, Default). +` + ); + const edges = await callEdges(dir); + expect(edges).toContainEqual({ sq: 'deleg::header/2', tq: 'deleg::header/3' }); + // No self-loop in either direction. + expect(edges.some((e) => e.sq === e.tq && e.sq.startsWith('deleg::header'))).toBe(false); + }); + + it('resolves remote calls to the called arity and refuses a sibling arity', async () => { + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'store.erl'), + `-module(store). +-export([get/1, get/2]). + +get(K) -> get(K, undefined). +get(K, Default) -> {K, Default}. +` + ); + fs.writeFileSync( + path.join(dir, 'src', 'client.erl'), + `-module(client). +-export([fetch/1, broken/1]). + +fetch(K) -> + store:get(K, nil). + +broken(K) -> + store:get(K, nil, extra). +` + ); + const edges = await callEdges(dir); + expect(edges).toContainEqual({ sq: 'client::fetch/1', tq: 'store::get/2' }); + // store:get/3 doesn't exist — the ref must resolve to NOTHING, not /1 or /2. + expect(edges.some((e) => e.sq === 'client::broken/1' && e.tq.startsWith('store::get'))).toBe(false); + }); + + it('resolves an arity-less dynamic MFA ref only when exactly one arity exists', async () => { + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'single.erl'), + `-module(single). +-export([work/1]). + +work(X) -> X. +` + ); + fs.writeFileSync( + path.join(dir, 'src', 'multi.erl'), + `-module(multi). +-export([job/1, job/2]). + +job(X) -> X. +job(X, Y) -> {X, Y}. +` + ); + fs.writeFileSync( + path.join(dir, 'src', 'spawner.erl'), + `-module(spawner). +-export([go/1]). + +go(Args) -> + erlang:spawn(single, work, Args), + erlang:spawn(multi, job, Args). +` + ); + const edges = await callEdges(dir); + // `Args` is dynamic, so both refs are arity-less. single:work has exactly + // one arity — it resolves; multi:job has two — silent beats wrong. + expect(edges).toContainEqual({ sq: 'spawner::go/1', tq: 'single::work/1' }); + expect(edges.some((e) => e.sq === 'spawner::go/1' && e.tq.startsWith('multi::job'))).toBe(false); + }); + + it('lands `fun mod:f/1` references on the written arity', async () => { + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'lib_m.erl'), + `-module(lib_m). +-export([bump/1, bump/2]). + +bump(X) -> X + 1. +bump(X, N) -> X + N. +` + ); + fs.writeFileSync( + path.join(dir, 'src', 'user_m.erl'), + `-module(user_m). +-export([run/1]). + +run(L) -> + lists:map(fun lib_m:bump/1, L). +` + ); + const edges = await callEdges(dir); + expect(edges).toContainEqual({ sq: 'user_m::run/1', tq: 'lib_m::bump/1' }); + expect(edges.some((e) => e.sq === 'user_m::run/1' && e.tq === 'lib_m::bump/2')).toBe(false); + }); +}); diff --git a/__tests__/erlang-behaviour-synthesizer.test.ts b/__tests__/erlang-behaviour-synthesizer.test.ts index d5d33f4ef..f3e11fc47 100644 --- a/__tests__/erlang-behaviour-synthesizer.test.ts +++ b/__tests__/erlang-behaviour-synthesizer.test.ts @@ -187,4 +187,45 @@ on_event(Ev) -> {seen, Ev}. const rows = await synthEdges(dir); expect(rows.map((r) => path.basename(r.tf))).toEqual(['public_impl.erl']); }); + + it('counts dispatch-site arity across <> literals (#1358)', async () => { + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'codec_behaviour.erl'), + `-module(codec_behaviour). + +-callback decode(binary(), list()) -> term(). + +-export([run/3]). + +run(Mod, Bin, Opts) -> + Mod:decode(Bin, Opts). +` + ); + fs.writeFileSync( + path.join(dir, 'src', 'json_codec.erl'), + `-module(json_codec). +-behaviour(codec_behaviour). +-export([decode/2]). + +decode(Bin, _Opts) -> Bin. +` + ); + // The dispatch site passes a binary literal whose commas previously + // inflated the computed arity (4 instead of 2), so the edge was dropped. + fs.writeFileSync( + path.join(dir, 'src', 'probe.erl'), + `-module(probe). +-export([go/1]). + +go(Mod) -> + Mod:decode(<<1,2,3>>, []). +` + ); + + const rows = await synthEdges(dir); + const fromProbe = rows.filter((r) => r.source === 'go').map((r) => `${path.basename(r.tf)}:${r.target}`); + expect(fromProbe).toEqual(['json_codec.erl:decode']); + expect(rows.every((r) => r.via === 'codec_behaviour:decode/2' || r.source !== 'go')).toBe(true); + }); }); diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 6bc48032e..b9427e0e8 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -10179,7 +10179,81 @@ helper() -> ok. const ns = result.nodes.find((n) => n.kind === 'namespace'); expect(ns?.name).toBe('my_server'); const start = result.nodes.find((n) => n.kind === 'function' && n.name === 'start'); - expect(start?.qualifiedName).toBe('my_server::start'); + // Arity is part of an Erlang function's identity — qualifiedName carries it (#1610). + expect(start?.qualifiedName).toBe('my_server::start/0'); + }); + + it('should give same-name different-arity functions separate arity-qualified nodes (#1610)', () => { + const code = `-module(gap). +-export([f/1, f/2]). + +f(X) -> X + 1. +f(X, Y) -> X + Y. +`; + const result = extractFromSource('src/gap.erl', code); + const fns = result.nodes.filter((n) => n.kind === 'function' && n.name === 'f'); + expect(fns).toHaveLength(2); + expect(fns.map((n) => n.qualifiedName).sort()).toEqual(['gap::f/1', 'gap::f/2']); + const f1 = fns.find((n) => n.qualifiedName === 'gap::f/1')!; + const f2 = fns.find((n) => n.qualifiedName === 'gap::f/2')!; + expect([f1.startLine, f1.endLine]).toEqual([4, 4]); + expect([f2.startLine, f2.endLine]).toEqual([5, 5]); + expect(f1.signature).toBe('f(X)'); + expect(f2.signature).toBe('f(X, Y)'); + }); + + it('should split interleaved same-name defs by arity with distinct qualified names', () => { + const code = `-module(inter). + +f(X) -> X + 1; +f(Y) -> Y. +g() -> ok. +f(X, Y) -> X + Y. +`; + const result = extractFromSource('src/inter.erl', code); + const fs = result.nodes.filter((n) => n.kind === 'function' && n.name === 'f'); + expect(fs).toHaveLength(2); + expect(fs.map((n) => n.qualifiedName).sort()).toEqual(['inter::f/1', 'inter::f/2']); + // Clauses of the same arity still merge into one span. + const f1 = fs.find((n) => n.qualifiedName === 'inter::f/1')!; + expect([f1.startLine, f1.endLine]).toEqual([3, 4]); + }); + + it('should flag exported per arity (#1610)', () => { + const code = `-module(m). +-export([f/1]). + +f(X) -> X. +f(X, Y) -> {X, Y}. +`; + const result = extractFromSource('src/m.erl', code); + expect(result.nodes.find((n) => n.qualifiedName === 'm::f/1')?.isExported).toBe(true); + expect(result.nodes.find((n) => n.qualifiedName === 'm::f/2')?.isExported).toBe(false); + }); + + it('should attach a -spec sitting between two arities to the arity it names (#1610)', () => { + const code = `-module(deleg). +-export([header/2, header/3]). + +header(Name, Req) -> + header(Name, Req, undefined). + +-spec header(binary(), map(), any()) -> any(). +header(Name, Headers, Default) -> + maps:get(Name, Headers, Default). +`; + const result = extractFromSource('src/deleg.erl', code); + const h2 = result.nodes.find((n) => n.qualifiedName === 'deleg::header/2')!; + const h3 = result.nodes.find((n) => n.qualifiedName === 'deleg::header/3')!; + expect(h2.signature).toBe('header(Name, Req)'); + expect(h3.signature).toBe('-spec header(binary(), map(), any()) -> any().'); + expect([h2.startLine, h2.endLine]).toEqual([4, 5]); + expect(h3.startLine).toBe(8); + // The delegation call carries the callee's arity — no more self-loop. + const calls = result.unresolvedReferences + .filter((r) => r.referenceKind === 'calls') + .map((r) => r.referenceName); + expect(calls).toContain('header/3'); }); it('should flag exported functions and honor -compile(export_all)', () => { @@ -10314,11 +10388,31 @@ prepare(X) -> X. `; const result = extractFromSource('src/m.erl', code); const calls = result.unresolvedReferences.filter((r) => r.referenceKind === 'calls').map((r) => r.referenceName); - expect(calls).toContain('prepare'); - // `mod:fn(...)` is emitted as `mod::fn` — the same shape the module - // namespace gives every function's qualifiedName, so it resolves via - // the qualified-name matcher. - expect(calls).toContain('other_mod::process'); + expect(calls).toContain('prepare/1'); + // `mod:fn(...)` is emitted as `mod::fn/arity` — the same shape the + // module namespace + arity suffix gives every function's qualifiedName, + // so it resolves via the qualified-name matcher (#1610). + expect(calls).toContain('other_mod::process/1'); + }); + + it('should carry written arity on fun references and static MFA lists (#1610)', () => { + const code = `-module(m). +-export([go/0]). + +go() -> + lists:map(fun bump/1, [1]), + Prod = fun other_mod:produce/2, + proc_lib:spawn_link(?MODULE, work, [a, b]), + Prod. + +bump(X) -> X + 1. +work(_A, _B) -> ok. +`; + const result = extractFromSource('src/m.erl', code); + const refs = result.unresolvedReferences; + expect(refs.some((r) => r.referenceKind === 'references' && r.referenceName === 'bump/1')).toBe(true); + expect(refs.some((r) => r.referenceKind === 'references' && r.referenceName === 'other_mod::produce/2')).toBe(true); + expect(refs.some((r) => r.referenceKind === 'calls' && r.referenceName === 'work/2')).toBe(true); }); it('should not emit calls for dynamic dispatch (var module / var fun)', () => { @@ -10331,9 +10425,9 @@ run(Mod, F) -> `; const result = extractFromSource('src/m.erl', code); const calls = result.unresolvedReferences.filter((r) => r.referenceKind === 'calls').map((r) => r.referenceName); - expect(calls).not.toContain('handle'); - expect(calls).not.toContain('Mod::handle'); - expect(calls).not.toContain('F'); + expect(calls.some((c) => c.startsWith('handle'))).toBe(false); + expect(calls.some((c) => c.startsWith('Mod::'))).toBe(false); + expect(calls.some((c) => c === 'F' || c.startsWith('F/'))).toBe(false); }); it('should connect gen_server self-calls to the module handlers', () => { @@ -10361,9 +10455,10 @@ handle_cast({put, K, V}, S) -> {noreply, maps:put(K, V, S)}. const result = extractFromSource('src/kv_store.erl', code); const calls = result.unresolvedReferences.filter((r) => r.referenceKind === 'calls').map((r) => r.referenceName); // ?SERVER (defined as ?MODULE), ?MODULE, and the module's own atom all - // count as self — public API wrappers connect to their handlers. - expect(calls.filter((c) => c === 'kv_store::handle_call')).toHaveLength(2); - expect(calls).toContain('kv_store::handle_cast'); + // count as self — public API wrappers connect to their handlers, at + // OTP's fixed handler arities (#1610). + expect(calls.filter((c) => c === 'kv_store::handle_call/3')).toHaveLength(2); + expect(calls).toContain('kv_store::handle_cast/2'); }); it('should connect gen_server calls to a registered-name module, directly or via an atom macro', () => { @@ -10383,8 +10478,8 @@ evict(Key) -> // OTP's {local, ?MODULE} convention names a server after its module — // a cross-module registered name targets that module's handlers. A name // matching no module simply never resolves downstream. - expect(calls).toContain('kv_store::handle_call'); - expect(calls).toContain('kv_store::handle_cast'); + expect(calls).toContain('kv_store::handle_call/3'); + expect(calls).toContain('kv_store::handle_cast/2'); }); it('should not connect gen_server calls with dynamic targets', () => { @@ -10417,10 +10512,10 @@ monitor_loop(_P) -> ok. `; const result = extractFromSource('src/m.erl', code); const calls = result.unresolvedReferences.filter((r) => r.referenceKind === 'calls').map((r) => r.referenceName); - expect(calls).toContain('request_process'); // ?MODULE → bare, same-file resolution - expect(calls).toContain('monitor_loop'); - expect(calls).toContain('other_mod::handle'); - expect(calls).toContain('other_mod::tick'); + expect(calls).toContain('request_process/2'); // ?MODULE → bare-with-arity, same-file resolution + expect(calls).toContain('monitor_loop/1'); + expect(calls).toContain('other_mod::handle/1'); + expect(calls).toContain('other_mod::tick/0'); }); it('should stay silent on dynamic spawn/apply (var module, fun value, or plain fun)', () => { @@ -10437,8 +10532,8 @@ helper() -> ok. const result = extractFromSource('src/m.erl', code); const calls = result.unresolvedReferences.filter((r) => r.referenceKind === 'calls').map((r) => r.referenceName); // The fun body's call is still walked; no phantom MFA targets appear. - expect(calls).toContain('helper'); - expect(calls.filter((c) => c !== 'spawn' && c !== 'apply' && c !== 'helper')).toHaveLength(0); + expect(calls).toContain('helper/0'); + expect(calls.filter((c) => !['spawn/3', 'spawn/1', 'apply/3', 'helper/0'].includes(c))).toHaveLength(0); }); it('should treat ?MODULE:fn calls as local calls', () => { @@ -10452,7 +10547,7 @@ work() -> ok. `; const result = extractFromSource('src/m.erl', code); const calls = result.unresolvedReferences.filter((r) => r.referenceKind === 'calls').map((r) => r.referenceName); - expect(calls).toContain('work'); + expect(calls).toContain('work/0'); }); it('should capture fun name/arity values as function references', () => { @@ -10467,8 +10562,8 @@ notify(_P) -> ok. `; const result = extractFromSource('src/m.erl', code); const refs = result.unresolvedReferences.filter((r) => r.referenceKind === 'references').map((r) => r.referenceName); - expect(refs).toContain('notify'); - expect(refs).toContain('m::notify'); + expect(refs).toContain('notify/1'); + expect(refs).toContain('m::notify/1'); }); it('should reference records used in bodies and argument patterns', () => { @@ -10504,8 +10599,8 @@ second(X) -> X. const calls = result.unresolvedReferences.filter( (r) => r.referenceKind === 'calls' && r.fromNodeId === handle?.id ).map((r) => r.referenceName); - expect(calls).toContain('first'); - expect(calls).toContain('second'); + expect(calls).toContain('first/1'); + expect(calls).toContain('second/1'); }); }); @@ -10527,8 +10622,8 @@ analyze(Path) -> expect(fns).toContain('main'); expect(fns).toContain('analyze'); const calls = result.unresolvedReferences.filter((r) => r.referenceKind === 'calls').map((r) => r.referenceName); - expect(calls).toContain('analyze'); - expect(calls).toContain('io::format'); + expect(calls).toContain('analyze/1'); + expect(calls).toContain('io::format/2'); }); it('should link an app resource file to its callback module and dependency apps', () => { @@ -10574,7 +10669,7 @@ do_thing(X) -> const refsFrom = (id?: string) => result.unresolvedReferences.filter((r) => r.fromNodeId === id).map((r) => `${r.referenceKind}:${r.referenceName}`); // The body's remote call belongs to the macro node — true exactly once. - expect(refsFrom(macro?.id)).toContain('calls:audit_logger::log'); + expect(refsFrom(macro?.id)).toContain('calls:audit_logger::log/2'); // The use site joins the call chain: do_thing -calls→ LOG_AUDIT. expect(refsFrom(doThing?.id)).toContain('calls:LOG_AUDIT'); }); @@ -10607,7 +10702,7 @@ prepare() -> ok. const result = extractFromSource('src/m.erl', code); const refs = result.unresolvedReferences.map((r) => r.referenceName); // The nested call inside the macro's arguments still attributes to check/0. - expect(refs).toContain('prepare'); + expect(refs).toContain('prepare/0'); // ?assertEqual (an OTP header macro) is emitted and simply never resolves… expect(refs).toContain('assertEqual'); // …but predefined macros have no definition to link. @@ -10627,7 +10722,7 @@ prepare() -> ok. const alias = result.nodes.find((n) => n.kind === 'constant' && n.name === 'ALIAS'); const refsFrom = (id?: string) => result.unresolvedReferences.filter((r) => r.fromNodeId === id).map((r) => `${r.referenceKind}:${r.referenceName}`); - expect(refsFrom(target?.id)).toContain('calls:target_fn'); + expect(refsFrom(target?.id)).toContain('calls:target_fn/0'); expect(refsFrom(alias?.id)).toContain('references:TARGET'); }); }); diff --git a/src/db/queries.ts b/src/db/queries.ts index a0f8bc541..af19b14cf 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -145,8 +145,11 @@ interface UnresolvedRefRow { * refs against newly-added node names. */ function referenceNameTail(referenceName: string): string { - const idx = Math.max(referenceName.lastIndexOf('.'), referenceName.lastIndexOf(':')); - return idx >= 0 ? referenceName.slice(idx + 1) : referenceName; + // Erlang refs carry a written arity (`f/1`, `mod::fn/2` — #1610); the tail a + // new symbol's plain name could match is the arity-less function name. + const base = referenceName.replace(/\/\d{1,3}$/, '') || referenceName; + const idx = Math.max(base.lastIndexOf('.'), base.lastIndexOf(':')); + return idx >= 0 ? base.slice(idx + 1) : base; } /** diff --git a/src/extraction/languages/erlang.ts b/src/extraction/languages/erlang.ts index 57e6d2573..2f9f1b4bb 100644 --- a/src/extraction/languages/erlang.ts +++ b/src/extraction/languages/erlang.ts @@ -9,8 +9,11 @@ import type { LanguageExtractor, ExtractorContext } from '../tree-sitter-types'; // extractor, so every symbol-bearing top-level form is dispatched through the // visitNode hook below instead: // - a function's name lives on its CLAUSE, not the fun_decl, and the grammar -// emits one fun_decl PER CLAUSE — consecutive same-name fun_decl forms are -// merged into a single function node here; +// emits one fun_decl PER CLAUSE — consecutive same-name same-ARITY +// fun_decl forms (clauses of one function) are merged into a single +// function node here. Arity is part of an Erlang function's identity +// (`f/1` and `f/2` are unrelated definitions — #1610), so each arity gets +// its own node, qualified `mod::f/1` / `mod::f/2`; // - type-position expressions (-spec/-type/-callback bodies, record field // types) parse as `call` nodes, so descending into them would mint bogus // call refs to type names (`pid()`, `term()`); the hook consumes those @@ -19,9 +22,10 @@ import type { LanguageExtractor, ExtractorContext } from '../tree-sitter-types'; // the generic extractStruct would skip as a forward declaration. // Calls (local `f(X)`, remote `mod:f(X)`, `fun f/1` references, and record // usages) are handled by the erlang branch in extractCall — remote calls are -// emitted as `mod::f`, which matches the qualifiedName the module namespace -// produces (see packageTypes below), so cross-module resolution rides the -// standard qualified-name matcher. +// emitted as `mod::f/2` (arity counted at the call site), byte-identical to +// the qualifiedName above, so cross-module resolution rides the standard +// qualified-name matcher; local calls are emitted `f/2` and resolved by the +// erlang arity step in matchReference. /** Text of an atom with quoted-atom quotes stripped (`'EXIT'` → `EXIT`). */ function atomText(node: SyntaxNode, source: string): string { @@ -35,19 +39,27 @@ function collapseWs(text: string): string { // --- Per-file memos. Extraction is file-sequential within a worker, so a // single-entry memo keyed by filePath is safe (and resets naturally). --- -/** Exported function names for the current file ('all' for -compile(export_all)). */ +/** + * Exported `name/arity` keys for the current file ('all' for + * -compile(export_all)). Keyed by arity because `-export([f/1])` exports + * exactly f/1 — f/2 in the same module stays private (#1610). A malformed + * `fa` with no arity node falls back to the bare name key. + */ let exportsFile = ''; let exportsMemo: Set | 'all' = new Set(); /** - * Clause-merge state: the previous fun_decl's name and node id. A fun_decl - * whose clause repeats that name is a continuation clause (or a same-name - * different-arity definition — deliberately grouped under one node, the way - * overloads are elsewhere) and attaches to the existing node instead of - * creating a duplicate. + * Clause-merge state: the previous fun_decl's name, arity, and node id. A + * fun_decl whose clause repeats that (name, arity) is a continuation clause of + * the SAME function and attaches to the existing node instead of creating a + * duplicate. A same-name DIFFERENT-arity fun_decl is an unrelated function + * (Erlang identity is `name/arity`) and gets its own node (#1610). Keying on + * adjacency stays safe: clauses of one function must be adjacent in Erlang — + * a non-adjacent redefinition of the same name/arity is a compile error. */ let lastFnFile = ''; let lastFnName = ''; +let lastFnArity = -1; let lastFnId = ''; function moduleExports(node: SyntaxNode, source: string, filePath: string): Set | 'all' { @@ -69,7 +81,12 @@ function moduleExports(node: SyntaxNode, source: string, filePath: string): Set< for (const fa of form.namedChildren) { if (fa.type !== 'fa') continue; const fun = getChildByField(fa, 'fun'); - if (fun) result.add(atomText(fun, source)); + if (!fun) continue; + const name = atomText(fun, source); + const arityNode = getChildByField(fa, 'arity'); + const arityValue = arityNode ? getChildByField(arityNode, 'value') : null; + const arity = arityValue ? getNodeText(arityValue, source) : null; + result.add(arity !== null ? `${name}/${arity}` : name); } } } @@ -78,13 +95,27 @@ function moduleExports(node: SyntaxNode, source: string, filePath: string): Set< return result; } -/** The -spec directly above a function (comments may sit between), if it names it. */ -function precedingSpec(node: SyntaxNode, name: string, source: string): SyntaxNode | null { +/** Argument count of a clause/sig: the `args` (expr_args) field's named-child count. */ +function nodeArity(withArgs: SyntaxNode): number { + const args = getChildByField(withArgs, 'args'); + return args ? args.namedChildCount : 0; +} + +/** + * The -spec directly above a function (comments may sit between), if it names + * it AND matches its arity — the spec for `header/3` sitting between the + * `header/2` and `header/3` definitions must attach to /3 only (#1610). A + * spec whose sigs can't be read (defensive) is accepted on the name alone. + */ +function precedingSpec(node: SyntaxNode, name: string, arity: number, source: string): SyntaxNode | null { let prev = node.previousNamedSibling; while (prev && prev.type === 'comment') prev = prev.previousNamedSibling; if (prev?.type === 'spec') { const specFun = getChildByField(prev, 'fun'); - if (specFun && atomText(specFun, source) === name) return prev; + if (specFun && atomText(specFun, source) === name) { + const sigs = prev.namedChildren.filter((c) => c.type === 'type_sig'); + if (sigs.length === 0 || sigs.some((sig) => nodeArity(sig) === arity)) return prev; + } } return null; } @@ -104,10 +135,11 @@ function handleFunDecl(node: SyntaxNode, ctx: ExtractorContext): boolean { if (!nameNode) return true; const name = atomText(nameNode, ctx.source); if (!name) return true; + const arity = nodeArity(first); - // Continuation clause: extend the existing node's span and attribute this - // clause's calls to it. - if (ctx.filePath === lastFnFile && name === lastFnName && lastFnId) { + // Continuation clause of the SAME function (same name AND arity): extend the + // existing node's span and attribute this clause's calls to it. + if (ctx.filePath === lastFnFile && name === lastFnName && arity === lastFnArity && lastFnId) { for (let i = ctx.nodes.length - 1; i >= 0; i--) { const n = ctx.nodes[i]; if (n && n.id === lastFnId) { @@ -121,16 +153,20 @@ function handleFunDecl(node: SyntaxNode, ctx: ExtractorContext): boolean { return true; } - const spec = precedingSpec(node, name, ctx.source); + const spec = precedingSpec(node, name, arity, ctx.source); const exports = moduleExports(node, ctx.source, ctx.filePath); const fn = ctx.createNode('function', name, node, { docstring: getPrecedingDocstring(spec ?? node, ctx.source), signature: spec ? collapseWs(getNodeText(spec, ctx.source)).slice(0, 300) : clauseHeader(first, ctx.source), - isExported: exports === 'all' || exports.has(name), + isExported: exports === 'all' || exports.has(`${name}/${arity}`) || exports.has(name), }); if (!fn) return true; + // Arity is part of the function's identity — carry it on the qualified name + // (`mod::f/2`), the canonical Erlang spelling and the only persisted slot. + // The node NAME stays bare so name search and bare-name matching still work. + fn.qualifiedName = `${fn.qualifiedName}/${arity}`; ctx.pushScope(fn.id); // The whole clause is walked (not just the body) so record patterns in the // arguments and guard calls contribute references too. @@ -138,6 +174,7 @@ function handleFunDecl(node: SyntaxNode, ctx: ExtractorContext): boolean { ctx.popScope(); lastFnFile = ctx.filePath; lastFnName = name; + lastFnArity = arity; lastFnId = fn.id; return true; } diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 8d71d7f18..c270351ae 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -3759,15 +3759,18 @@ export class TreeSitterExtractor { // Erlang: a local call is `call(expr: atom, args)`; a remote call nests it // under `remote(module: remote_module, fun: call)` — the module qualifier - // lives on the PARENT. Remote calls are emitted as `mod::fn`, which is - // byte-identical to the qualifiedName the module namespace gives every - // function (see packageTypes in languages/erlang.ts), so they resolve via - // matchByQualifiedName. A var/macro callee or module (`F(X)`, `?M(X)`, - // `Mod:handle(X)`) has no static target — except `?MODULE:fn(X)`, which the - // bare name + same-file preference resolves correctly. `fun name/1` / - // `fun mod:name/1` values are function REFERENCES (callback registration), - // and record construction/update/index/field-access are `references` to the - // record's struct node. + // lives on the PARENT. Arity is part of a function's identity (#1610), so + // refs carry the call-site arity: remote calls are emitted as `mod::fn/2`, + // byte-identical to the qualifiedName the module namespace + arity suffix + // gives every function (see languages/erlang.ts), so they resolve via + // matchByQualifiedName; local calls are emitted `fn/2` and resolved by the + // erlang arity step in matchReference (same-file first). A var/macro callee + // or module (`F(X)`, `?M(X)`, `Mod:handle(X)`) has no static target — + // except `?MODULE:fn(X)`, which the bare-name-with-arity + same-file + // preference resolves correctly. `fun name/1` / `fun mod:name/1` values + // are function REFERENCES (callback registration) carrying their own + // written arity, and record construction/update/index/field-access are + // `references` to the record's struct node. if (this.language === 'erlang') { const line = node.startPosition.row + 1; const column = node.startPosition.column; @@ -3799,9 +3802,13 @@ export class TreeSitterExtractor { moduleExpr.type === 'macro_call_expr' ? getChildByField(moduleExpr, 'name') : null; if (!macroName || getNodeText(macroName, this.source) !== 'MODULE') return; } + // Arity from the call site's own argument list — part of the callee's + // identity, and what disambiguates `f/1` from `f/2` (#1610). + const callArgsNode = getChildByField(node, 'args'); + const callArity = callArgsNode ? callArgsNode.namedChildCount : 0; this.unresolvedReferences.push({ fromNodeId: callerId, - referenceName: calleeName, + referenceName: `${calleeName}/${callArity}`, referenceKind: 'calls', line, column, @@ -3824,9 +3831,10 @@ export class TreeSitterExtractor { const target = argsNode?.namedChild(0) ?? null; const targetModule = target ? this.resolveErlangGenServerTarget(target) : null; if (targetModule) { + // OTP fixes the handler arities: handle_call/3, handle_cast/2. this.unresolvedReferences.push({ fromNodeId: callerId, - referenceName: `${targetModule}::${fnBare === 'cast' ? 'handle_cast' : 'handle_call'}`, + referenceName: `${targetModule}::${fnBare === 'cast' ? 'handle_cast/2' : 'handle_call/3'}`, referenceKind: 'calls', line, column, @@ -3857,9 +3865,17 @@ export class TreeSitterExtractor { getChildByField(m, 'name') !== null && getNodeText(getChildByField(m, 'name')!, this.source) === 'MODULE'; if (m.type !== 'atom' && !isLocalModule) continue; + // Arity of the spawned/applied function = the length of the + // static args-list literal directly after the (M, F) pair, when + // present (`spawn_link(?MODULE, request_process, [Req, Env])` → + // /2). A var/absent list leaves the ref arity-less; the + // qualified matcher then resolves it only when the module + // defines exactly one arity of that name. + const mfaList = argExprs[i + 2]; + const arityTail = mfaList?.type === 'list' ? `/${mfaList.namedChildCount}` : ''; this.unresolvedReferences.push({ fromNodeId: callerId, - referenceName: isLocalModule ? erlAtom(f) : `${erlAtom(m)}::${erlAtom(f)}`, + referenceName: (isLocalModule ? erlAtom(f) : `${erlAtom(m)}::${erlAtom(f)}`) + arityTail, referenceKind: 'calls', line: f.startPosition.row + 1, column: f.startPosition.column, @@ -3880,6 +3896,11 @@ export class TreeSitterExtractor { if (moduleAtom?.type !== 'atom') return; refName = `${erlAtom(moduleAtom)}::${refName}`; } + // `fun f/1` writes its arity — carry it so the ref lands on the + // matching arity's node (#1610). + const funArityNode = getChildByField(node, 'arity'); + const funArityValue = funArityNode ? getChildByField(funArityNode, 'value') : null; + if (funArityValue) refName = `${refName}/${getNodeText(funArityValue, this.source)}`; this.unresolvedReferences.push({ fromNodeId: callerId, referenceName: refName, diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 4ad7e64b6..157893b4d 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -122,9 +122,14 @@ const CONTAINER_NODE_KINDS = new Set([ 'class', 'struct', 'union', 'interface', 'trait', 'protocol', 'enum', 'namespace', 'module', ]); -/** Last `::` / `.` / `/`-separated segment of a qualified symbol. */ +/** + * Last `::` / `.` / `/`-separated segment of a qualified symbol. An Erlang + * arity tail (`mod::fn/3`, `fn/3`) is stripped first — the useful last segment + * is the function name, never the digits (#1610). + */ function lastQualifierPart(symbol: string): string { - const parts = symbol.split(/::|[./]/).filter((p) => p.length > 0); + const noArity = symbol.replace(/\/\d{1,3}$/, '') || symbol; + const parts = noArity.split(/::|[./]/).filter((p) => p.length > 0); return parts[parts.length - 1] ?? symbol; } @@ -6694,6 +6699,19 @@ export class ToolHandler { * Python — `stage_apply::run` matches a `run` in `stage_apply.rs`) */ private matchesSymbol(node: Node, symbol: string): boolean { + // Erlang arity spelling (`fn/3`, `mod:fn/3` → normalized `mod.fn/3`): when + // the node's qualifiedName carries an arity (`mod::fn/3`, #1610), the + // written arity must match it exactly; the remaining comparison then runs + // on the arity-less spelling. A node with no arity in its qualifiedName + // keeps the original symbol (a `/` there means a path-ish name instead). + const aritySpelling = /^(.+)\/(\d{1,3})$/.exec(symbol); + if (aritySpelling) { + const nodeArity = /\/(\d{1,3})$/.exec(node.qualifiedName ?? '')?.[1]; + if (nodeArity !== undefined) { + if (nodeArity !== aritySpelling[2]) return false; + symbol = aritySpelling[1]!; + } + } // Simple name match if (node.name === symbol) return true; // File basename match (e.g., "product-card" matches "product-card.liquid") diff --git a/src/resolution/callback-synthesizer.ts b/src/resolution/callback-synthesizer.ts index 60b389937..0d53829b7 100644 --- a/src/resolution/callback-synthesizer.ts +++ b/src/resolution/callback-synthesizer.ts @@ -3008,6 +3008,10 @@ const ERLANG_BEHAVIOUR_FANOUT_CAP = 24; */ function erlangArityAt(src: string, openIdx: number): number { let depth = 1; + // `<<1,2,3>>` binary literals: commas inside are element separators, not + // argument separators. Tracked separately from bracket depth because the + // single-char `<`/`>` comparison operators must stay inert (#1358). + let binDepth = 0; let commas = 0; let sawArg = false; const limit = Math.min(src.length, openIdx + 4000); @@ -3028,13 +3032,15 @@ function erlangArityAt(src: string, openIdx: number): number { sawArg = true; continue; } + if (ch === '<' && src[i + 1] === '<') { binDepth++; i++; sawArg = true; continue; } + if (ch === '>' && src[i + 1] === '>' && binDepth > 0) { binDepth--; i++; continue; } if (ch === '(' || ch === '[' || ch === '{') { depth++; sawArg = true; continue; } if (ch === ')' || ch === ']' || ch === '}') { depth--; if (depth === 0) return sawArg ? commas + 1 : 0; continue; } - if (ch === ',' && depth === 1) { commas++; continue; } + if (ch === ',' && depth === 1 && binDepth === 0) { commas++; continue; } if (!/\s/.test(ch)) sawArg = true; } return -1; @@ -3265,12 +3271,18 @@ async function erlangBehaviourDispatchEdges(queries: QueryBuilder, ctx: Resoluti } if (declaringBehaviours.size === 0) return []; - // Implementer target lookup, lazy per (behaviour, fn): implementers come - // from the `implements` edges extraction resolved, and the target is the - // implementer module's own exported `fn` function node. + // Implementer target lookup, lazy per (behaviour, fn, arity): implementers + // come from the `implements` edges extraction resolved, and the target is + // the implementer module's own exported `fn` node OF THE SITE'S ARITY — + // function qualifiedNames carry arity (`mod::fn/2`, #1610), so the arity the + // dispatch site used selects among same-named definitions. const targetCache = new Map(); - const targetsOf = (behaviour: Node, fn: string): Node[] => { - const cacheKey = `${behaviour.id}#${fn}`; + const qnArity = (qn: string): number => { + const m = /\/(\d{1,3})$/.exec(qn); + return m ? Number(m[1]) : -1; + }; + const targetsOf = (behaviour: Node, fn: string, arity: number): Node[] => { + const cacheKey = `${behaviour.id}#${fn}/${arity}`; let targets = targetCache.get(cacheKey); if (targets) return targets; targets = []; @@ -3279,7 +3291,13 @@ async function erlangBehaviourDispatchEdges(queries: QueryBuilder, ctx: Resoluti if (!impl || impl.language !== 'erlang' || impl.kind !== 'namespace') continue; const fnNode = ctx .getNodesInFile(impl.filePath) - .find((n) => n.kind === 'function' && n.name === fn && n.isExported !== false); + .find( + (n) => + n.kind === 'function' && + n.name === fn && + qnArity(n.qualifiedName) === arity && + n.isExported !== false, + ); if (fnNode) targets.push(fnNode); } targetCache.set(cacheKey, targets); @@ -3308,7 +3326,7 @@ async function erlangBehaviourDispatchEdges(queries: QueryBuilder, ctx: Resoluti const behaviours = declaringBehaviours.get(`${fn}/${arity}`); if (!behaviours || behaviours.length !== 1) continue; // unknown or ambiguous const behaviour = behaviours[0]!; - const targets = targetsOf(behaviour, fn); + const targets = targetsOf(behaviour, fn, arity); if (targets.length === 0 || targets.length > ERLANG_BEHAVIOUR_FANOUT_CAP) continue; const line = safe.slice(0, m.index).split('\n').length; const disp = enclosingFn(nodesInFile, line); diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 01f615b28..7b4bccc18 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -886,10 +886,13 @@ export class ReferenceResolver { // indexed under the bare name, so the existence check strips the dot. // Nix static path imports (`import ./x.nix`) name a FILE, not a symbol — // they bypass the symbol-existence check and resolve via resolveViaImport. - const existenceName = + let existenceName = ref.language === 'arkts' && ref.referenceName.startsWith('.') ? ref.referenceName.slice(1) : ref.referenceName; + // Erlang refs carry the call-site arity (`f/1`, `mod::f/2` — #1610); the + // name index stores bare names, so existence is checked arity-less. + if (ref.language === 'erlang') existenceName = existenceName.replace(/\/\d{1,3}$/, ''); const tPre = this.profileStages ? process.hrtime.bigint() : 0n; const preFilterPass = isNixPathImportRef(ref) || diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 651051466..50ade7c06 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -503,6 +503,35 @@ export function matchByQualifiedName( } } + // Erlang qualified refs (#1610): every erlang function's qualifiedName + // carries its arity (`mod::f/2`), and refs carry the call-site arity when it + // is statically known. + if (ref.language === 'erlang' && ref.referenceName.includes('::')) { + // A ref WITH arity that missed the exact lookup names an arity that isn't + // defined (or a module out of repo). Never fall through to the partial + // match — its "last segment" would be the arity digits — and never settle + // for a sibling arity: silent beats wrong. + if (/\/\d{1,3}$/.test(ref.referenceName)) return null; + // An arity-LESS qualified ref (dynamic MFA whose args list wasn't a + // static literal): resolve only when the module defines exactly ONE arity + // of that function; several arities with no signal is a guess. + const base = ref.referenceName.slice(ref.referenceName.lastIndexOf('::') + 2); + const prefix = `${ref.referenceName}/`; + const arityCands = keepForRef(context.getNodesByName(base)).filter( + (n) => + n.qualifiedName.startsWith(prefix) && /^\d{1,3}$/.test(n.qualifiedName.slice(prefix.length)), + ); + if (arityCands.length === 1) { + return { + original: ref, + targetNodeId: arityCands[0]!.id, + confidence: 0.85, + resolvedBy: 'qualified-name', + }; + } + return null; + } + // Try partial qualified name match — again preferring the call site's own // file when more than one symbol's qualifiedName ends with the reference. const parts = ref.referenceName.split(/[:.]/); @@ -2295,6 +2324,52 @@ export function matchReference( }; } + // Erlang call/fun refs carry the call-site arity (`f/1` — #1610) because + // arity is part of the function's identity and every erlang function's + // qualifiedName carries it (`mod::f/1`). Resolve ONLY to a definition of + // that exact arity: the call site's own file first (a local call targets its + // own module by language semantics; `-import`ed functions ride the + // cross-file branch), and when no definition of that arity exists anywhere, + // resolve to NOTHING rather than a sibling arity — the real target may be + // macro-generated or out of repo, and a wrong-arity edge is worse than none. + if ( + ref.language === 'erlang' && + !ref.referenceName.includes('::') && + (ref.referenceKind === 'calls' || ref.referenceKind === 'references') + ) { + const am = /^(.+)\/(\d{1,3})$/.exec(ref.referenceName); + if (am) { + // endsWith is length-anchored, so `/1` cannot match `…/11`. + const arityTail = `/${am[2]}`; + const candidates = context + .getNodesByName(am[1]!) + .filter( + (n) => + n.language === 'erlang' && n.kind === 'function' && n.qualifiedName.endsWith(arityTail), + ); + if (candidates.length > 0) { + const sameFile = candidates.find((n) => n.filePath === ref.filePath); + if (sameFile) { + return { original: ref, targetNodeId: sameFile.id, confidence: 0.95, resolvedBy: 'exact-match' }; + } + if (candidates.length === 1) { + return { original: ref, targetNodeId: candidates[0]!.id, confidence: 0.8, resolvedBy: 'exact-match' }; + } + const best = findBestMatch(ref, candidates, context); + if (best) { + const proximity = computePathProximity(ref.filePath, best.filePath); + return { + original: ref, + targetNodeId: best.id, + confidence: proximity >= 30 ? 0.7 : 0.4, + resolvedBy: 'exact-match', + }; + } + } + return null; + } + } + // Try strategies in order of confidence let result: ResolvedRef | null;