diff --git a/CHANGELOG.md b/CHANGELOG.md index f44be8edd..b6a245640 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- A Rust method call on a field — `self.inner.run()` — now resolves to the method on that field's type instead of to whatever same-named method happened to sit nearest. Rust has no implicit `self`, so every call on a field is written this way, and the receiver was being dropped: the call became a bare `run`, which then matched a same-named method in the calling file, in the calling type, or on an unrelated type altogether. Calls on fields whose type is external — `Vec`, `Arc>`, a type from another crate — were the worst affected, because the nearest project method is never the right answer for them; `self.items.len()` could be recorded as a call to the enclosing type's own `len`, a self-recursive edge the source never had. On ripgrep and tokio this removed roughly half of all self-recursive call edges, and corrected hundreds more that pointed at the wrong type. Fields whose type cannot be established stay unresolved rather than being guessed, so callers, impact and flow answers no longer include fabricated dependencies. `Box`, `Rc` and `Arc` resolve through to what they wrap; `Option`, `Mutex` and other containers keep their own methods. (#1585) - Naming a file by its path in a `codegraph_explore` query now works reliably: the path is resolved against the index and that file is guaranteed a place at the top of the answer. Previously the path was broken into fragments — bracketed route segments like SvelteKit's `[id]` made this worst — and pieces like `page` or `runs` matched every sibling file, so the file you actually named could be crowded out of the answer entirely. A path that doesn't match any indexed file is now called out instead of silently ignored. - Plainly-worded `codegraph_explore` questions now find camelCase code: a query like "auto-scroll to bottom" can reach a function named `scrollFeedToBottom`, because query words are matched against the words inside identifiers, not just whole names. - Variables and constants now count when `codegraph_explore` picks its starting symbols, so state held in plain variables — `$state`-style variables in Svelte, for example — no longer gets overlooked. diff --git a/__tests__/resolution.test.ts b/__tests__/resolution.test.ts index 637b4a9d0..2cb34b713 100644 --- a/__tests__/resolution.test.ts +++ b/__tests__/resolution.test.ts @@ -2858,6 +2858,298 @@ func (mx *Mux) dispatch() { }, 30000); }); + describe('Rust field receiver calls (#1585)', () => { + // `self.inner.run()` used to emit a BARE `run` ref: the receiver is a + // field_expression, not a plain identifier, so it never reached the + // qualified branch. Rust has no implicit `self`, so EVERY call on a field + // takes that shape, and the bare name exact-matched whatever same-named + // method sat nearest — including the calling method itself, fabricating a + // self-recursive edge. Field receivers now resolve exclusively via + // validated field inference: external field types produce NO edge, + // in-project ones produce the correct edge. + it('an external field type produces no edge; an in-project one resolves past a same-file decoy', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1585-')); + try { + fs.writeFileSync( + path.join(tmpDir, 'Cargo.toml'), + '[package]\nname = "repro"\nversion = "0.1.0"\nedition = "2021"\n' + ); + fs.mkdirSync(path.join(tmpDir, 'src')); + fs.writeFileSync(path.join(tmpDir, 'src', 'lib.rs'), 'pub mod inner;\npub mod outer;\n'); + fs.writeFileSync( + path.join(tmpDir, 'src', 'inner.rs'), + `pub struct Inner { + pub n: usize, +} + +impl Inner { + pub fn run(&mut self) { + self.n += 1; + } +} +` + ); + // Decoy::run sits in the CALLER's file, so file proximity elects it + // over Inner::run — the exact wrong answer this resolves away from. + fs.writeFileSync( + path.join(tmpDir, 'src', 'outer.rs'), + `use crate::inner::Inner; + +pub struct Decoy { + pub flag: bool, +} + +impl Decoy { + pub fn run(&mut self) { + self.flag = true; + } +} + +pub struct Outer { + pub inner: Inner, + pub items: Vec, +} + +impl Outer { + pub fn go(&mut self) { + self.inner.run(); + } + + pub fn count(&self) -> usize { + self.items.len() + } +} +` + ); + + const cg = CodeGraph.initSync(tmpDir); + await cg.indexAll(); + + // The in-project field resolves to ITS type's method, not the decoy's. + const innerRun = (await cg.searchNodes('run', { limit: 10 })).find( + (r) => r.node.kind === 'method' && r.node.qualifiedName === 'Inner::run' + ); + expect(innerRun).toBeDefined(); + expect((await cg.getCallers(innerRun!.node.id)).map((c) => c.node.name)).toContain('go'); + + const decoyRun = (await cg.searchNodes('run', { limit: 10 })).find( + (r) => r.node.kind === 'method' && r.node.qualifiedName === 'Decoy::run' + ); + expect(decoyRun).toBeDefined(); + expect((await cg.getCallers(decoyRun!.node.id)).map((c) => c.node.name)).not.toContain('go'); + + // `items: Vec` is external: `self.items.len()` must bind to + // nothing rather than to the enclosing type's own `count`. + const count = (await cg.searchNodes('count', { limit: 10 })).find( + (r) => r.node.kind === 'method' + ); + expect(count).toBeDefined(); + expect((await cg.getCallees(count!.node.id)).map((c) => c.node.name)).toHaveLength(0); + cg.close(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }, 30000); + + // The receiver is the field's type CONSTRUCTOR — `Core<'a, u8>` answers + // with `Core`'s method — except for the smart pointers that Deref, whose + // single type argument is unwrapped. Containers that own their methods + // (`Option`, `Mutex`, `Vec`) must NOT be unwrapped: their method belongs to + // the container, and following the argument would fabricate an edge. + it('deref wrappers unwrap to their argument; containers and generics keep their constructor', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1585b-')); + try { + fs.writeFileSync( + path.join(tmpDir, 'Cargo.toml'), + '[package]\nname = "wrappers"\nversion = "0.1.0"\nedition = "2021"\n' + ); + fs.mkdirSync(path.join(tmpDir, 'src')); + fs.writeFileSync(path.join(tmpDir, 'src', 'lib.rs'), 'pub mod inner;\npub mod outer;\n'); + fs.writeFileSync( + path.join(tmpDir, 'src', 'inner.rs'), + `pub struct Inner { + pub n: usize, +} + +impl Inner { + pub fn tick(&self) -> usize { + self.n + } + + pub fn lock(&self) -> usize { + self.n + } +} + +pub struct Core<'a, T> { + pub tag: &'a str, + pub item: T, +} + +impl<'a, T> Core<'a, T> { + pub fn roll(&self) -> usize { + 0 + } +} +` + ); + fs.writeFileSync( + path.join(tmpDir, 'src', 'outer.rs'), + `use crate::inner::{Core, Inner}; +use std::sync::{Arc, Mutex}; + +pub struct Outer<'a> { + pub boxed: Box, + pub guarded: Arc>, + pub core: Core<'a, u8>, +} + +impl<'a> Outer<'a> { + pub fn via_box(&self) -> usize { + self.boxed.tick() + } + + pub fn via_mutex(&self) -> usize { + self.guarded.lock() + } + + pub fn via_generic(&self) -> usize { + self.core.roll() + } +} +` + ); + + const cg = CodeGraph.initSync(tmpDir); + await cg.indexAll(); + + const find = async (qualified: string) => + (await cg.searchNodes(qualified.split('::').pop()!, { limit: 10 })).find( + (r) => r.node.kind === 'method' && r.node.qualifiedName === qualified + ); + + // Box derefs: the call reaches Inner. + const tick = await find('Inner::tick'); + expect(tick).toBeDefined(); + expect((await cg.getCallers(tick!.node.id)).map((c) => c.node.name)).toContain('via_box'); + + // Arc> stops at Mutex, which owns `lock` — Inner::lock is + // a same-named decoy that must not be bound. + const lock = await find('Inner::lock'); + expect(lock).toBeDefined(); + expect((await cg.getCallers(lock!.node.id)).map((c) => c.node.name)).not.toContain( + 'via_mutex' + ); + + // A plain generic keeps its constructor: Core<'a, u8> answers with Core. + const roll = await find('Core::roll'); + expect(roll).toBeDefined(); + expect((await cg.getCallers(roll!.node.id)).map((c) => c.node.name)).toContain( + 'via_generic' + ); + cg.close(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }, 30000); + + // A tuple struct names its fields by position, so `self.0` reads the first + // type out of the declaration. Positions must not be interchangeable: two + // fields of different types answer their own methods. + it('tuple-struct fields resolve by position', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1585c-')); + try { + fs.writeFileSync( + path.join(tmpDir, 'Cargo.toml'), + '[package]\nname = "tuples"\nversion = "0.1.0"\nedition = "2021"\n' + ); + fs.mkdirSync(path.join(tmpDir, 'src')); + fs.writeFileSync(path.join(tmpDir, 'src', 'lib.rs'), 'pub mod def;\npub mod imp;\n'); + fs.writeFileSync( + path.join(tmpDir, 'src', 'def.rs'), + `pub struct First { + pub n: usize, +} + +impl First { + pub fn go(&self) -> usize { + 1 + } +} + +pub struct Second { + pub n: usize, +} + +impl Second { + pub fn go(&self) -> usize { + 2 + } +} + +pub struct Pair(pub First, pub Second); + +pub struct Opaque(pub Vec); +` + ); + fs.writeFileSync( + path.join(tmpDir, 'src', 'imp.rs'), + `use crate::def::{Opaque, Pair}; + +pub struct Decoy; + +impl Decoy { + pub fn go(&self) -> usize { + 99 + } + + pub fn len(&self) -> usize { + 99 + } +} + +impl Pair { + pub fn take_first(&self) -> usize { + self.0.go() + } + + pub fn take_second(&self) -> usize { + self.1.go() + } +} + +impl Opaque { + pub fn size(&self) -> usize { + self.0.len() + } +} +` + ); + + const cg = CodeGraph.initSync(tmpDir); + await cg.indexAll(); + + const callersOf = async (qualified: string) => { + const node = (await cg.searchNodes(qualified.split('::').pop()!, { limit: 10 })).find( + (r) => r.node.kind === 'method' && r.node.qualifiedName === qualified + ); + expect(node).toBeDefined(); + return (await cg.getCallers(node!.node.id)).map((c) => c.node.name); + }; + + expect(await callersOf('First::go')).toEqual(['take_first']); + expect(await callersOf('Second::go')).toEqual(['take_second']); + // A same-named method in the calling file stays out of both. + expect(await callersOf('Decoy::go')).toHaveLength(0); + // `Opaque(Vec)` is external: no edge rather than the decoy's len. + expect(await callersOf('Decoy::len')).toHaveLength(0); + cg.close(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }, 30000); + }); + describe('Imported singleton instance-method calls (#1292)', () => { // `reproStore.notifyJoinGuildStatus()` after `import { reproStore }` used // to emit its calls edge to the CONSTANT (resolvedBy:'import'), while the diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 8d71d7f18..58f7878a3 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -4517,6 +4517,32 @@ export class TreeSitterExtractor { // Go receivers resolve strictly via validated field-hop // inference (see matchGoFieldChainCall) or stay unresolved. calleeName = `${getNodeText(receiver, this.source).replace(/\s+/g, '')}.${methodName}`; + } else if ( + this.language === 'rust' && + receiver && + receiver.type === 'field_expression' && + /^self\.(?:[A-Za-z_]\w*|\d+)$/.test(getNodeText(receiver, this.source).replace(/\s+/g, '')) + ) { + // Rust field receiver `self.inner.run(...)`, or `self.0.run(...)` + // on a tuple struct: keep the receiver so resolution can infer the + // field's declared type from the enclosing type's declaration. + // Rust has no implicit `self`, so EVERY call on a field is written + // this way — and the receiver, being a field_expression rather + // than a plain identifier, never reached the qualified branch + // above. The bare method name that remained exact-matched an + // unrelated same-named method whenever the field's type is + // external (`Vec`, `Arc<…>`) or simply lives elsewhere, + // fabricating internal dependencies — including self-recursive + // edges the source never had. These receivers resolve strictly + // via validated field inference (see matchRustSelfFieldCall) or + // stay unresolved. + // + // A DEEPER chain (`self.a.b.run()`) keeps the bare-name behavior: + // every hop would have to type, and measured on tokio the hops + // that fail are mostly ones the bare name happened to get right, + // so making them exclusive costs more correct edges than it + // removes wrong ones. + calleeName = `${getNodeText(receiver, this.source).replace(/\s+/g, '')}.${methodName}`; } else { calleeName = methodName; } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 651051466..a45ce56c6 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1731,6 +1731,19 @@ export function matchMethodCall( return matchGoFieldChainCall(objectOrClass!, methodName!, ref, context); } + // Rust field receiver `self.field.method`: the enclosing type comes from the + // caller's own qualified name, the field's declared type from that type's + // declaration lines, and the method is VALIDATED on it by resolveMethodOnType. + // EXCLUSIVE, for the same reason as Go's chain above — when the field's type + // cannot be established or is external (`items: Vec`, `cmd: Command` — + // no project node), the ref stays unresolved rather than falling through to + // the bare-name strategies, which is what bound `self.items.len()` to an + // unrelated same-named method. These receivers were never emitted before this + // change, so there is no prior recall on the fallback path to preserve. + if (ref.language === 'rust' && dotMatch && /^self\.\w+$/.test(objectOrClass!)) { + return matchRustSelfFieldCall(objectOrClass!, methodName!, ref, context); + } + // Java/Kotlin: receiver may be a field whose name doesn't match the type by // Java naming convention (`userbo` → class `UserBO`, abbreviated). Look up // the field in the enclosing class to get its declared type, then resolve @@ -1992,6 +2005,176 @@ function matchGoFieldChainCall( return null; } +/** + * Smart pointers whose `Deref` target receives the method call, so the type + * ARGUMENT is the receiver rather than the pointer: `inner: Box` answers + * `self.inner.run()` with `Inner::run`. Deliberately just these three. The other + * common single-argument generics do NOT forward — `Option::as_ref`, + * `Mutex::lock` and `Vec::len` are the container's own methods, and + * following their argument is exactly the fabrication this matcher exists to + * prevent (`self.err.as_ref()` on an `Option` bound to an unrelated + * `as_ref` before this change). + */ +const RUST_DEREF_WRAPPERS = new Set(['Box', 'Rc', 'Arc']); + +/** + * The type a Rust field declaration hands the method call, or null when it + * names none we can follow. + * + * The outer constructor is normally the receiver: a field declared + * `core: Core<'s, M, S>` answers `self.core.roll()` with `Core::roll`, and its + * type arguments are parameters of that type, not a wrapper around it. The + * exception is a `Deref` smart pointer, whose argument is unwrapped instead + * (one level at a time, so `Arc>` lands on `Mutex` and stops — + * `Mutex` does not forward). + * + * A field whose receiver type is not a project type (`items: Vec`, + * `cmd: Command`) yields a name with no node in the graph, so resolveMethodOnType + * finds no method and the ref stays unresolved — the intended outcome. Shapes + * with no single constructor (tuples, slices, function pointers, `impl Trait`) + * are declined outright; `dyn Trait` resolves to the trait, which is where the + * graph records the method. + */ +function rustFieldTypeName(decl: string): string | null { + // The capture runs to end of line: keep only this field's own type, cutting + // at the first comma that is not inside the type's own brackets, then drop a + // trailing `}` from a single-line struct body. + let t = (splitRustTypeArgs(decl)[0] ?? '').replace(/\}.*$/, '').trim(); + t = t.replace(/^pub(\s*\([^)]*\))?\s+/, ''); // `pub` / `pub(crate)` on the field + for (let depth = 0; depth < 8; depth++) { + t = t.trim().replace(/^&(\s*'\w+)?\s*(mut\s+)?/, '').trim(); // `&`, `&'a `, `&mut ` + t = t.replace(/^dyn\s+/, '').trim(); // `dyn Trait` — the trait owns the method + const generic = t.match(/^([A-Za-z_][\w:]*)\s*<(.+)>$/s); + if (!generic) break; + const ctor = generic[1]!.split('::').pop()!; + if (!RUST_DEREF_WRAPPERS.has(ctor)) { + t = generic[1]!; // ordinary generic type: the constructor receives the call + break; + } + // A smart pointer forwards to its ONLY type argument; `Box` (custom + // allocator) and any multi-argument form are left to the constructor. + const args = splitRustTypeArgs(generic[2]!).filter((a) => !/^'/.test(a.trim())); + if (args.length !== 1) { t = generic[1]!; break; } + t = args[0]!; + } + t = t.trim(); + if (!/^[A-Za-z_][\w:]*$/.test(t)) return null; // tuples, slices, fn ptrs, impl Trait + const last = t.split('::').pop(); + return last && /^[A-Za-z_]\w*$/.test(last) ? last : null; +} + +/** Split a Rust type-argument list on top-level commas (`K, Vec<(A, B)>`). */ +function splitRustTypeArgs(args: string): string[] { + const out: string[] = []; + let depth = 0; + let start = 0; + for (let i = 0; i < args.length; i++) { + const ch = args[i]; + if (ch === '<' || ch === '(' || ch === '[') depth++; + else if (ch === '>' || ch === ')' || ch === ']') depth--; + else if (ch === ',' && depth === 0) { out.push(args.slice(start, i)); start = i + 1; } + } + out.push(args.slice(start)); + return out; +} + +/** + * Rust field receiver `self.field.method` (see the extraction-side comment on + * `field_expression` receivers). The enclosing type comes from the CALLER's own + * qualified name (`Outer::go` → `Outer`), the field's declared type from that + * struct's own declaration lines, and the method is VALIDATED on that type by + * resolveMethodOnType — so a mis-read declaration produces no edge rather than + * a wrong one. + * + * Returns null whenever any hop is uncertain: the caller is not type-qualified + * (a free function, or a generic `impl Trait for X` whose methods carry + * the TRAIT as their qualifier), the struct is not in the graph, the field is + * not declared on it, or its type is one we do not follow. Null leaves the ref + * unresolved, which is the point — the bare method name it would otherwise fall + * back to carries no receiver information at all. + */ +function matchRustSelfFieldCall( + receiver: string, + methodName: string, + ref: UnresolvedRef, + context: ResolutionContext +): ResolvedRef | null { + const field = receiver.slice('self.'.length); + if (!field) return null; + + // `self` names the type the enclosing method is defined on. + const caller = context.getNodeById?.(ref.fromNodeId); + const qualified = caller?.qualifiedName ?? ''; + const sep = qualified.lastIndexOf('::'); + if (sep <= 0) return null; + const selfType = qualified.slice(0, sep); + if (!/^[A-Za-z_]\w*$/.test(selfType)) return null; + + const fieldType = rustFieldDeclaredType(selfType, field, ref, context); + if (!fieldType) return null; + return resolveMethodOnType(fieldType, methodName, ref, context, 0.85, 'instance-method'); +} + +/** + * The type of `field` as declared on `ownerType`, or null when the type is not + * in the graph, the field is not declared on it, or its declaration names a + * type we do not follow (see rustFieldTypeName). + */ +function rustFieldDeclaredType( + ownerType: string, + field: string, + ref: UnresolvedRef, + context: ResolutionContext +): string | null { + const positional = /^\d+$/.test(field) ? Number(field) : -1; + const fieldEsc = field.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + // A field line is `name: Type,`. Capture to end of line and cut at the first + // TOP-LEVEL comma below, so a type that contains commas of its own + // (`map: HashMap`) survives, and a second field declared on + // the same line still does not bleed in. + const fieldTypeRe = new RegExp(`(?:^|[{,\\s])${fieldEsc}\\s*:\\s*(.+)$`); + + const owners = preferCallSiteFile(context.getNodesByName(ownerType), ref.filePath).filter( + (n) => (n.kind === 'struct' || n.kind === 'union' || n.kind === 'enum') && n.language === 'rust' + ); + for (const owner of owners) { + // Only the type's own declaration lines: a same-named binding elsewhere in + // the file cannot donate a field type. Comments are stripped per line, as + // a doc comment above a field otherwise donates a word from its prose. + // Lines come from the context's per-file cache — this runs for every + // `self.field.method()` ref, and re-splitting the file each time is the + // cost getFileLines exists to avoid. + const lines = context.getFileLines + ? context.getFileLines(owner.filePath) + : context.readFile(owner.filePath)?.split('\n') ?? null; + if (!lines) continue; + const declLines = lines + .slice(Math.max(0, owner.startLine - 1), owner.endLine) + .map((l) => l.replace(/\/\/.*$/, '')); + + // A tuple struct — `struct Wrapper(pub Inner, Other);` — names its fields + // by position, so `self.0` reads the Nth type out of the declaration + // instead of matching `name: Type`. + if (positional >= 0) { + const decl = declLines.join(' ').match(/\bstruct\s+\w+\s*(?:<[^>]*>)?\s*\(([^;]*)\)/); + if (!decl || !decl[1]) continue; + const raw = splitRustTypeArgs(decl[1])[positional]; + if (raw === undefined) continue; + const fieldType = rustFieldTypeName(raw); + if (fieldType) return fieldType; + continue; + } + + for (const line of declLines) { + const m = line.match(fieldTypeRe); + if (!m || !m[1]) continue; + const fieldType = rustFieldTypeName(m[1]); + if (fieldType) return fieldType; + } + } + return null; +} + /** * Split a camelCase or PascalCase string into words. */