fix(rust): qualify generic/lifetime impl methods by the implementing type, not the trait (#1588) (#1596)
Fixes #1588. ## What was wrong The receiver of an `impl` block — the name that qualifies its methods, owns the `contains` edge, and sources the `implements` edge — was found positionally: the **last bare `type_identifier` child** of the `impl_item`. That works for `impl Source for FileSource`. But once the implementing type carries parameters it parses as a `generic_type`, and the only bare identifier left is the **trait's**: ```rust impl Source for FileSource → FileSource::read ✓ impl<T> Source for BufSource<T> → Source::read ✗ (should be BufSource::read) impl<'a> Iterator for Parents<'a> → Iterator::next ✗ impl Trait for &Foo → Trait::method ✗ ``` Two consequences, both reproduced on `main`: - `BufSource::read` did not exist in the graph, so `resolveMethodOnType("BufSource", "read")` and "who calls `BufSource::read`" had no answer, and every generic implementation of a trait collapsed onto the same trait-qualified name. - Because the impl's method carried the trait's qualified name, the interface-impl synthesizer treated the impl **body** as a second trait declaration and emitted a dispatch edge from it (`Source::read -> FileSource::read`, registered at the generic impl's line — a body of `{ 0 }` containing no call at all). The native kernel (`rustlang.rs`) mirrored the positional rule deliberately, bug-for-bug, to hold byte-parity with the TS walker — its header said "preserve, never fix via the grammar's trait:/type: fields". So the fix has to land on both sides at once. ## What this does Both extractors now read the grammar's **named fields** instead of scanning children. One shared rule (`rustImplTypeName` in `languages/rust.ts`, `impl_type_name` in the kernel), applied to `impl_item.type`: | implementing type | node | receiver | |---|---|---| | `Foo` | `type_identifier` | `Foo` | | `Foo<T>` / `Foo<'a>` | `generic_type` → its `type` field | `Foo` | | `m::Foo` | `scoped_type_identifier` → its `name` field | `Foo` (was: no receiver) | | `&Foo` / `&'a mut Foo` | `reference_type` → its `type` field | `Foo` | | `(A, B)`, `dyn Tr`, `*const T`, `u32`, fn types | anything else | none — extracted as plain functions, exactly as before | The `implements` back-reference reads `impl_item.trait` (full text, so `fmt::Display` and `From<u32>` keep their spelling) and bails when the field is absent (inherent impl). Everything else — the no-scope impl quirk, the source-order `contains` owner scan, method extraction — is untouched; the `contains` edge simply lands on the implementing type now instead of the trait. The kernel header comment, the parity test's description, and the two design docs that documented the quirk as "preserve" are updated to say what changed. ## Measured on ripgrep (110 `.rs` files, `main` build vs this branch) | | main | this PR | |---|---|---| | nodes / methods | 4029 / 2202 | 4029 / 2202 | | impl methods qualified by a **trait** name (node outside that trait's extent) | 61 | **0** | | `Iterator::*` methods | 2 | 0 | | duplicate method qualified names | 77 | 42 | | synthesized `interface-impl` edges originating **outside** any trait declaration (the phantom fan-outs) | 38 | **0** | | synthesized `interface-impl` edges originating at a real trait declaration | 33 | **52** | | plain (non-heuristic) `calls` edges | 9098 | 9098 | So the synthesizer lost every phantom edge and *gained* 19 legitimate fan-outs to implementations it could not previously see as implementations. `contains` edges went 5237 → 5224: the 13 removed were trait→impl-method edges produced by the mis-qualification. The issue's repro now gives `BufSource::read` at line 12, `BufSource -> Source`, and both synthesized edges registered at the declaration (line 2) — identical on the kernel path and with `CODEGRAPH_KERNEL=0`. (The remaining `UsesFile::go -> BufSource::read` exact-match guess there is the separate `self.field.method()` receiver problem, #1585, which stacks on this.) ## Tests - `__tests__/extraction.test.ts` (Rust Extraction): method qualified names for generic / lifetime / reference / scoped / generic-trait impls; the trait's qualified name names exactly one node; `implements` refs come from the implementing type for every shape; the `contains` edge lands on the type; tuple / `dyn` impls keep producing plain functions with no `implements` ref. - `__tests__/resolution.test.ts` (end-to-end): `Source::read` names only the declaration; dispatch fans out to **both** `FileSource::read` and `BufSource::read`, every synthesized edge registered at line 2; neither impl body sprouts a synthesized call. - `__tests__/fixtures/kernel-parity/torture.rs` grows all the new impl shapes; `kernel-rustlang-parity` (LF + CRLF) passes against the rebuilt kernel. - `CODEGRAPH_KERNEL_EXPECT=1 npx vitest run __tests__/kernel-*.test.ts` — all 15 suites, 147 tests pass. - Full `npm test`: 3180 passed, 9 skipped, 1 failed — `mcp-daemon.test.ts > daemon idle-times-out after the last client disconnects`, a 30 s timing test that passed on re-run in isolation (the machine was running four parallel suites and kernel builds at the time); unrelated to extraction. Re-index after upgrading to pick up the corrected names. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LxZj6W6Y1SHXwvpT3uwJpK
This commit is contained in:
Colby Mchenry
@@ -32,6 +32,45 @@ function extractRustReturnType(node: SyntaxNode, source: string): string | undef
|
||||
return last === 'Self' ? 'self' : last;
|
||||
}
|
||||
|
||||
/**
|
||||
* The implementing type's simple name for an `impl` block, read from the
|
||||
* grammar's `type` field (#1588). Mirrored byte-for-byte by the native
|
||||
* kernel's `impl_type_name` (codegraph-kernel/src/rustlang.rs) — change both.
|
||||
*
|
||||
* `impl<T> Source for BufSource<T>`, `impl<'a> Iterator for Parents<'a>`,
|
||||
* `impl Trait for &Foo`, `impl Trait for m::Foo` all yield the implementing
|
||||
* TYPE (`BufSource`, `Parents`, `Foo`, `Foo`). The previous rule took the last
|
||||
* bare `type_identifier` child of the `impl_item`; once the implementing type
|
||||
* carries parameters it parses as a `generic_type`, so the only bare
|
||||
* identifier left was the TRAIT's — every parameterized impl's methods were
|
||||
* qualified by the trait (`Source::read`), unaddressable by their type and
|
||||
* colliding with the trait's own declaration.
|
||||
*
|
||||
* Shapes that name no single type (tuples, `dyn Trait`, pointers, primitives,
|
||||
* function types…) yield undefined: no receiver, and the fn is extracted
|
||||
* exactly as before.
|
||||
*/
|
||||
export function rustImplTypeName(typeNode: SyntaxNode | null, source: string): string | undefined {
|
||||
if (!typeNode) return undefined;
|
||||
switch (typeNode.type) {
|
||||
case 'type_identifier':
|
||||
case 'identifier':
|
||||
return getNodeText(typeNode, source);
|
||||
// `Foo<T>` — the `type` field is the bare (or scoped) name, never the args.
|
||||
case 'generic_type':
|
||||
return rustImplTypeName(getChildByField(typeNode, 'type'), source);
|
||||
// `m::Foo` — the last segment is the type's name.
|
||||
case 'scoped_type_identifier':
|
||||
case 'scoped_identifier':
|
||||
return rustImplTypeName(getChildByField(typeNode, 'name'), source);
|
||||
// `&Foo` / `&'a mut Foo` — the referenced type.
|
||||
case 'reference_type':
|
||||
return rustImplTypeName(getChildByField(typeNode, 'type'), source);
|
||||
default:
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
export const rustExtractor: LanguageExtractor = {
|
||||
// `function_signature_item` is a trait method DECLARATION (`fn render(&self);`,
|
||||
// no body). Extracting it makes a trait's method set first-class, which
|
||||
@@ -88,32 +127,10 @@ export const rustExtractor: LanguageExtractor = {
|
||||
let parent = node.parent;
|
||||
while (parent) {
|
||||
if (parent.type === 'impl_item') {
|
||||
// For `impl Type { ... }` — the type is a direct type_identifier child
|
||||
// For `impl Trait for Type { ... }` — the type is the LAST type_identifier
|
||||
// (the first is part of the trait path)
|
||||
const children = parent.namedChildren;
|
||||
// Find all direct type_identifier children (not nested in scoped paths)
|
||||
const typeIdents = children.filter(
|
||||
(c: SyntaxNode) => c.type === 'type_identifier'
|
||||
);
|
||||
if (typeIdents.length > 0) {
|
||||
// Last type_identifier is always the implementing type
|
||||
const typeNode = typeIdents[typeIdents.length - 1]!;
|
||||
return source.substring(typeNode.startIndex, typeNode.endIndex);
|
||||
}
|
||||
// Handle generic types: impl<T> MyStruct<T> { ... }
|
||||
const genericType = children.find(
|
||||
(c: SyntaxNode) => c.type === 'generic_type'
|
||||
);
|
||||
if (genericType) {
|
||||
const innerType = genericType.namedChildren.find(
|
||||
(c: SyntaxNode) => c.type === 'type_identifier'
|
||||
);
|
||||
if (innerType) {
|
||||
return source.substring(innerType.startIndex, innerType.endIndex);
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
// The grammar names the implementing type directly (the `type` field)
|
||||
// for both `impl Type { … }` and `impl Trait for Type { … }` — see
|
||||
// rustImplTypeName for why the old positional scan was wrong (#1588).
|
||||
return rustImplTypeName(getChildByField(parent, 'type'), source);
|
||||
}
|
||||
parent = parent.parent;
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@ import { isGeneratedFile } from './generated-detection';
|
||||
import type { LanguageExtractor, ExtractorContext } from './tree-sitter-types';
|
||||
import { EXTRACTORS } from './languages';
|
||||
import { stripCppTemplateArgs } from './languages/c-cpp';
|
||||
import { rustImplTypeName } from './languages/rust';
|
||||
import { LiquidExtractor } from './liquid-extractor';
|
||||
import { RazorExtractor } from './razor-extractor';
|
||||
import { SvelteExtractor } from './svelte-extractor';
|
||||
@@ -5717,38 +5718,20 @@ export class TreeSitterExtractor {
|
||||
* For plain `impl Type { ... }` (no trait), no inheritance edge is needed.
|
||||
*/
|
||||
private extractRustImplItem(node: SyntaxNode): void {
|
||||
// Check if this is `impl Trait for Type` by looking for a `for` keyword
|
||||
const hasFor = node.children.some(
|
||||
(c: SyntaxNode) => c.type === 'for' && !c.isNamed
|
||||
);
|
||||
if (!hasFor) return;
|
||||
// `impl Trait for Type` carries the trait in the grammar's `trait` field;
|
||||
// an inherent `impl Type { … }` has none and needs no inheritance edge.
|
||||
const traitNode = getChildByField(node, 'trait');
|
||||
if (!traitNode) return;
|
||||
|
||||
// In `impl Trait for Type`, the type_identifiers are:
|
||||
// first = Trait name, last = implementing Type name
|
||||
// Also handle generic types like `impl<T> Trait for MyStruct<T>`
|
||||
const typeIdents = node.namedChildren.filter(
|
||||
(c: SyntaxNode) => c.type === 'type_identifier' || c.type === 'generic_type' || c.type === 'scoped_type_identifier'
|
||||
);
|
||||
if (typeIdents.length < 2) return;
|
||||
// Full text, so a scoped path (`std::fmt::Display`) and a generic trait
|
||||
// (`From<u32>`) keep their spelling.
|
||||
const traitName = getNodeText(traitNode, this.source);
|
||||
|
||||
const traitNode = typeIdents[0]!;
|
||||
const typeNode = typeIdents[typeIdents.length - 1]!;
|
||||
|
||||
// Get the trait name (handle scoped paths like std::fmt::Display)
|
||||
const traitName = traitNode.type === 'scoped_type_identifier'
|
||||
? this.source.substring(traitNode.startIndex, traitNode.endIndex)
|
||||
: getNodeText(traitNode, this.source);
|
||||
|
||||
// Get the implementing type name (extract inner type_identifier for generics)
|
||||
let typeName: string;
|
||||
if (typeNode.type === 'generic_type') {
|
||||
const inner = typeNode.namedChildren.find(
|
||||
(c: SyntaxNode) => c.type === 'type_identifier'
|
||||
);
|
||||
typeName = inner ? getNodeText(inner, this.source) : getNodeText(typeNode, this.source);
|
||||
} else {
|
||||
typeName = getNodeText(typeNode, this.source);
|
||||
}
|
||||
// The implementing type from the `type` field (#1588). The old positional
|
||||
// scan took the LAST type-shaped child, which for a parameterized
|
||||
// implementing type (`BufSource<T>`, `Parents<'a>`, `&Foo`) was the trait.
|
||||
const typeName = rustImplTypeName(getChildByField(node, 'type'), this.source);
|
||||
if (!typeName) return;
|
||||
|
||||
// Find the struct/type node for the implementing type
|
||||
const typeNodeId = this.findNodeByName(typeName);
|
||||
|
||||
Reference in New Issue
Block a user