fix(erlang): give same-name different-arity functions separate arity-qualified nodes (#1610) (#1615)
Fixes #1610. Also fixes #1358 (the `<<binary>>` arity miscount in behaviour dispatch, reported separately and hit by the same code path). ## Problem Arity is part of an Erlang function's identity — `f/1` and `f/2` are unrelated top-level definitions — but the extractor merged consecutive same-name `fun_decl`s regardless of arity. Reproduced on main exactly as reported: - adjacent `f(X) -> …. f(X, Y) -> ….` → **one** node spanning both, with the first definition's signature; - interleaved `f/1, g/0, f/2` → two nodes with **identical** `qualified_name`; - `cowboy_req`'s `header(Name, Req) -> header(Name, Req, undefined).` → a **self-loop** `header → header`, with the `-spec` for `/3` swallowed by the merged span; - `-export([f/1])` marked every arity exported. ## Fix - **One node per (name, arity).** Clauses of the same name+arity still merge (that part of the old behavior was correct); a different arity starts a new node. `qualifiedName` carries the canonical spelling — `mod::f/1` — while the node **name stays bare** so search and bare-name matching are unchanged. - **`-export` and `-spec` are per-arity.** `-export([f/1])` exports exactly `f/1`; a spec sitting between two arities attaches to the arity its signature names. - **Refs carry the call-site arity** wherever it's statically known: local `f/1`, remote `mod::f/2`, `fun f/1` / `fun mod:f/1` values, `gen_server` dispatch (`handle_call/3`, `handle_cast/2`), and spawn/apply MFA lists (`spawn_link(?MODULE, work, [A, B])` → `work/2`). - **The matcher resolves only to the named arity** — same file first (a local call targets its own module) — and when no definition of that arity exists it resolves to **nothing** rather than a sibling arity: silent beats wrong. An arity-less dynamic-MFA ref resolves only when the module defines exactly one arity of that name. - **Behaviour dispatch** selects the implementer node of the site's arity, and the arity counter now skips `<<1,2,3>>` binary-literal commas per its own docstring (#1358) — `Mod:decode(<<1,2,3>>, Opts)` counts 2, not 4. - **`codegraph_explore` / `codegraph_node`** accept the written `mod:fn/3` spelling against the new arity-qualified names (the issue's measured `cowboy_stream_h:request_process/3` shape). ## Validation Minimal fixtures (all three reported shapes) now index as `gap::f/1` + `gap::f/2`, distinct `inter::f/1`/`inter::f/2`, and a real `deleg::header/2 → deleg::header/3` edge with no self-loop. Cowboy (fresh `--depth 1` clone, this build vs unmodified main build): | | main | this PR | |---|---|---| | nodes | 3,668 | 3,748 (+80 — the arity splits; no explosion) | | erlang function nodes | 2,850 | 2,930 | | behaviour dispatch edges | 38 | **44** | | `cowboy_req::header` | one node, span 420–425, /3's spec lost | `header/2` (420–421, its own spec) + `header/3` (424–425, its spec) | | delegation | self-loop | `header/2 → header/3` | `calls` edges drop 6,059 → 5,656: a sample of every removed pair shows the false-positive class the issue predicted — out-of-repo/BIF calls (`length/1`, `error/1`, `quicer:*`) that previously name-matched onto unrelated same-named in-repo functions now stay unresolved. Tests: new arity coverage in extraction + a new arity-resolution integration suite + a #1358 binary-literal behaviour test; updated existing Erlang expectations to the arity-carrying spellings. Full suite: **3,018 passed, 0 failed**. No migration: an existing Erlang index picks the new shape up on its next re-index (`codegraph sync` / re-`init`). Erlang is wasm-only (not in the native kernel), so there is no kernel-parity surface. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LxZj6W6Y1SHXwvpT3uwJpK
This commit is contained in:
@@ -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<string> | '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<string> | '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;
|
||||
}
|
||||
|
||||
@@ -3760,15 +3760,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;
|
||||
@@ -3800,9 +3803,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,
|
||||
@@ -3825,9 +3832,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,
|
||||
@@ -3858,9 +3866,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,
|
||||
@@ -3881,6 +3897,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,
|
||||
|
||||
Reference in New Issue
Block a user