1
0
Эх сурвалжийг харах

fix(rust): keep self method calls on the correct owner (#1882)

Fixes #1861. Reuses L4XB's receiver retention and owner resolution from #1866, with duplicate-owner and edit/sync guards.

Co-authored-by: L4XB <lukas.buck@e-mail.de>
Colby Mchenry 6 өдөр өмнө
parent
commit
8f8081968e

+ 2 - 0
CHANGELOG.md

@@ -145,6 +145,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- Rust calls on `self` now stay with the enclosing type instead of linking to an unrelated type’s same-named method. Thanks @L4XB. (#1861)
+
 - Turning telemetry off now resets its identity and stops running processes from recording, sending, or restoring unsent data. (#1869)
 
 - Calls between JavaScript, JSX and TypeScript files keep their callers and callback flows.

+ 15 - 8
__tests__/extraction.test.ts

@@ -1481,7 +1481,7 @@ impl From<u32> for Own {
     ).toBe(true);
   });
 
-  it('keeps the owner-field shape for `self.<field>.<method>()` and collapses every other receiver (#1585)', () => {
+  it('keeps the owner shape for `self.<method>()` and `self.<field>.<method>()`, and collapses every other receiver (#1585, #1861)', () => {
     const code = `
 pub struct Outer { pub inner: Inner, pub deep: Deep }
 impl Outer {
@@ -1500,15 +1500,22 @@ impl Outer {
     const calls = result.unresolvedReferences
       .filter((r) => r.referenceKind === 'calls')
       .map((r) => r.referenceName);
-    // Exactly one call keeps the `self.<field>` prefix — the single-hop field
-    // receiver whose type the resolver can read off the owner struct.
-    expect(calls.filter((c) => c.startsWith('self.'))).toEqual(['self.inner.run']);
+    // Two shapes keep an owner the resolver can act on: the single-hop field
+    // receiver, whose type it reads off the owner struct (#1585), and the bare
+    // `self` receiver, whose type is the calling method's own owner (#1861).
+    // `self.make().run()` contributes `self.make` — the inner call — and its
+    // OUTER call collapses, because a method's return type is not read here.
+    expect(calls.filter((c) => c.startsWith('self.')).sort()).toEqual([
+      'self.inner.run',
+      'self.make',
+      'self.run',
+    ]);
     // A local receiver keeps its name as before…
     expect(calls).toContain('local.run');
-    // …and the deeper chain, the call receiver, the parenthesized receiver and
-    // the bare `self` receiver all still collapse to the method name.
-    expect(calls.filter((c) => c === 'run')).toHaveLength(4);
-    expect(calls).toContain('make');
+    // …and the deeper chain, the call receiver and the parenthesized receiver
+    // still collapse to the method name. `self.run()` no longer does, so this
+    // is three rather than four.
+    expect(calls.filter((c) => c === 'run')).toHaveLength(3);
     const outerRun = result.nodes.find((n) => n.qualifiedName === 'Outer::run');
     expect(outerRun).toBeDefined();
     const fieldRef = result.unresolvedReferences.find((r) => r.referenceName === 'self.inner.run');

+ 72 - 0
__tests__/resolution.test.ts

@@ -1258,6 +1258,78 @@ impl<T> Source for BufSource<T> {
       expect(callsFrom('Countdown::run').map((c) => c.target)).toEqual(['Countdown::run']);
     });
 
+    // ── Rust `self.<method>()` receivers (#1861) ──────────────────────────
+    it('resolves `self.method()` on the enclosing type, not on whichever same-named method sits nearer (#1861)', async () => {
+      // The issue's repro, one file: `Decoy::reset` sits between the call and
+      // the method it means, so a bare name ranked by file proximity picked
+      // the decoy — and the edge carried no provenance to say it was a guess.
+      writeRustCrate(tempDir, {
+        'lib.rs':
+          'pub struct Target { pub n: i32 }\n\nimpl Target {\n    pub fn reset(&mut self) { self.n = -1; }\n}\n\n' +
+          'pub struct Decoy { pub n: i32 }\n\nimpl Decoy {\n    pub fn reset(&mut self) { self.n = 0; }\n}\n\n' +
+          'impl Target {\n    pub fn run(&mut self) { self.reset(); }\n}\n',
+      });
+      cg = await CodeGraph.init(tempDir, { index: true });
+
+      expect(callsFrom('Target::run')).toEqual([
+        { target: 'Target::reset', resolvedBy: 'qualified-name', provenance: undefined },
+      ]);
+    });
+
+    it('decides the same way across directories, where proximity decided before (#1861)', async () => {
+      // Same code, only the layout changes. If the answer moved with the file
+      // tree, proximity was still deciding it.
+      writeRustCrate(tempDir, {
+        'lib.rs': 'pub mod near;\npub mod far;\n',
+        'near.rs': 'pub struct Decoy { pub n: i32 }\nimpl Decoy {\n    pub fn reset(&mut self) { self.n = 0; }\n}\n',
+        'far.rs':
+          'pub struct Target { pub n: i32 }\nimpl Target {\n    pub fn reset(&mut self) { self.n = -1; }\n}\n' +
+          'impl Target {\n    pub fn run(&mut self) { self.reset(); }\n}\n',
+      });
+      cg = await CodeGraph.init(tempDir, { index: true });
+
+      expect(callsFrom('Target::run').map((c) => c.target)).toEqual(['Target::reset']);
+    });
+
+    it('declines when the enclosing type has no such method, and does not change a receiver-less call (#1861)', async () => {
+      // The two ways this could overreach. `self.missing()` names nothing on
+      // the owner, so it must not fall back to some other type's `missing`.
+      //
+      // The receiver-less half is pinned as it BEHAVES, not as it should: a
+      // bare `reset()` is a free-function call, and it already resolved to
+      // `Target::reset` before this change — the mirror image of #1861, where
+      // a call with no receiver is given one. That is a separate defect in the
+      // bare-name strategy, measured on this branch's parent; the cell is here
+      // so this change is pinned to not make it worse.
+      writeRustCrate(tempDir, {
+        'lib.rs':
+          'pub fn reset() {}\n\n' +
+          'pub struct Other { pub n: i32 }\nimpl Other {\n    pub fn missing(&mut self) {}\n}\n\n' +
+          'pub struct Target { pub n: i32 }\nimpl Target {\n    pub fn reset(&mut self) { self.n = -1; }\n' +
+          '    pub fn free(&mut self) { reset(); }\n' +
+          '    pub fn absent(&mut self) { self.missing(); }\n}\n',
+      });
+      cg = await CodeGraph.init(tempDir, { index: true });
+
+      // Unchanged by this commit — see the note above.
+      expect(callsFrom('Target::free').map((c) => c.target)).toEqual(['Target::reset']);
+      // Nothing on the owner is named `missing`, so no edge at all.
+      expect(callsFrom('Target::absent')).toEqual([]);
+    });
+
+    it('resolves `self.method()` inside a trait impl to that impl (#1861)', async () => {
+      writeRustCrate(tempDir, {
+        'lib.rs':
+          'pub trait Run {\n    fn go(&mut self);\n}\n\n' +
+          'pub struct Decoy { pub n: i32 }\nimpl Decoy {\n    pub fn step(&mut self) { self.n = 0; }\n}\n\n' +
+          'pub struct Doer { pub n: i32 }\nimpl Doer {\n    pub fn step(&mut self) { self.n = 1; }\n}\n' +
+          'impl Run for Doer {\n    fn go(&mut self) { self.step(); }\n}\n',
+      });
+      cg = await CodeGraph.init(tempDir, { index: true });
+
+      expect(callsFrom('Doer::go').map((c) => c.target)).toEqual(['Doer::step']);
+    });
+
     it('resolves a trait-object field to the trait method and typed fields to the right implementation (#1585, #1588)', async () => {
       // The #1588 repro's second half: `UsesFile::go` / `UsesBuf::go` each
       // forward through a typed field, and a `Box<dyn Source>` field lands on

+ 68 - 0
__tests__/rust-self-owner.test.ts

@@ -0,0 +1,68 @@
+import { afterEach, beforeEach, expect, it } from 'vitest';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { CodeGraph } from '../src';
+
+let root: string;
+let cg: CodeGraph | undefined;
+beforeEach(() => { root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-rust-self-owner-')); });
+afterEach(() => { cg?.close(); cg = undefined; fs.rmSync(root, { recursive: true, force: true }); });
+async function index(files: Record<string, string>) {
+  fs.mkdirSync(path.join(root, 'src'));
+  fs.writeFileSync(path.join(root, 'Cargo.toml'), '[package]\nname="owners"\nversion="0.1.0"\nedition="2021"\n');
+  for (const [file, text] of Object.entries(files)) fs.writeFileSync(path.join(root, 'src', file), text);
+  cg = await CodeGraph.init(root, { index: true });
+}
+function targets(file: string) {
+  const caller = cg!.getNodesByKind('method').find(n => n.filePath === `src/${file}` && n.qualifiedName === 'Target::run');
+  expect(caller).toBeDefined();
+  return cg!.getOutgoingEdges(caller!.id).filter(e => e.kind === 'calls').map(e => {
+    const target = cg!.getNode(e.target)!;
+    return `${target.filePath}:${target.qualifiedName}`;
+  });
+}
+
+it('does not borrow a missing method from a same-named type in another module (#1861)', async () => {
+  await index({
+    'lib.rs': 'pub mod caller; pub mod decoy;',
+    'caller.rs': 'pub struct Target;\nimpl Target { pub fn run(&self) { self.reset(); } }',
+    'decoy.rs': 'pub struct Target;\nimpl Target { pub fn reset(&self) {} }',
+  });
+  expect(targets('caller.rs')).toEqual([]);
+});
+
+it('keeps a proven local owner despite a same-named type and method in another module (#1861)', async () => {
+  await index({
+    'lib.rs': 'pub mod caller; pub mod decoy;',
+    'caller.rs': 'pub struct Target;\nimpl Target { pub fn reset(&self) {} }\nimpl Target { pub fn run(&self) { self.reset(); } }',
+    'decoy.rs': 'pub struct Target;\nimpl Target { pub fn reset(&self) {} }',
+  });
+  expect(targets('caller.rs')).toEqual(['src/caller.rs:Target::reset']);
+  fs.writeFileSync(path.join(root, 'src/caller.rs'), 'pub struct Target;\nimpl Target { pub fn run(&self) { self.reset(); } }');
+  await cg!.sync();
+  expect(targets('caller.rs')).toEqual([]);
+  fs.writeFileSync(path.join(root, 'src/caller.rs'), 'pub struct Target;\nimpl Target { pub fn reset(&self) {} }\nimpl Target { pub fn run(&self) { self.reset(); } }');
+  await cg!.sync();
+  expect(targets('caller.rs')).toEqual(['src/caller.rs:Target::reset']);
+});
+
+it('keeps a unique owner whose impl is split across files (#1861)', async () => {
+  await index({
+    'lib.rs': 'pub mod caller;\npub struct Target;\nimpl Target { pub fn reset(&self) {} }',
+    'caller.rs': 'use crate::Target;\nimpl Target { pub fn run(&self) { self.reset(); } }',
+  });
+  expect(targets('caller.rs')).toEqual(['src/lib.rs:Target::reset']);
+});
+
+it('declines indistinguishable inline-module owners instead of claiming one (#1861)', async () => {
+  await index({ 'lib.rs': `mod a {
+    pub struct Target;
+    impl Target { pub fn reset(&self) {} }
+  }
+  mod b {
+    pub struct Target;
+    impl Target { pub fn run(&self) { self.reset(); } }
+  }` });
+  expect(targets('lib.rs')).toEqual([]);
+});

+ 10 - 2
codegraph-kernel/src/rustlang.rs

@@ -894,9 +894,17 @@ impl<'t> Walker<'t> {
                                     _ => callee_name = method_name.to_string(),
                                 }
                             }
+                            // `self.method()` — keep the `self.` prefix so the
+                            // resolver can read the owner off the calling
+                            // method's qualified name and resolve the method on
+                            // THAT type, instead of matching a bare name by file
+                            // proximity (#1861). Mirrors the wasm extractor.
+                            "self" => {
+                                callee_name = format!("self.{method_name}");
+                            }
                             _ => {
-                                // parenthesized, await_expression, `self` —
-                                // bare method name.
+                                // parenthesized, await_expression — bare method
+                                // name.
                                 callee_name = method_name.to_string();
                             }
                         }

+ 11 - 0
src/extraction/tree-sitter.ts

@@ -4709,6 +4709,17 @@ export class TreeSitterExtractor {
               } else {
                 calleeName = methodName;
               }
+            } else if (this.language === 'rust' && receiver && receiver.type === 'self') {
+              // Rust `self.method()`. Keep the `self.` prefix, exactly as the
+              // field shape below does (#1585): the resolver reads the owner
+              // off the CALLING method's qualified name and resolves the
+              // method on that type. Collapsing to the bare method name handed
+              // the resolver a name with no owner, which it then matched among
+              // all same-named methods by file proximity — so `self.reset()`
+              // inside `impl Target` landed on a `Decoy::reset` that happened
+              // to sit nearer, with nothing in the edge to show it was a guess
+              // (#1861). Mirrored in the kernel's extract_call (rustlang.rs).
+              calleeName = `self.${methodName}`;
             } else if (
               this.language === 'rust' &&
               receiver &&

+ 61 - 0
src/resolution/name-matcher.ts

@@ -2250,6 +2250,17 @@ export function matchMethodCall(
     return matchRustSelfFieldCall(objectOrClass!.slice('self.'.length), methodName!, ref, context);
   }
 
+  // Rust call on the enclosing type itself — `self.reset()`, emitted as
+  // `self.reset` (#1861). Same discipline as the field branch above, and
+  // EXCLUSIVE for the same reason: the owner is written on the `impl` line and
+  // carried in the calling method's qualified name, so it is not a guess.
+  // Letting this shape reach the bare-name strategies below is how
+  // `self.reset()` resolved to a same-named method on an unrelated type
+  // whenever that type's method happened to sit nearer the call site.
+  if (ref.language === 'rust' && dotMatch && objectOrClass === 'self') {
+    return matchRustSelfCall(methodName!, ref, context);
+  }
+
   // TS/JS call through a field of the enclosing class — `this.mailer.send()`,
   // emitted as `this.mailer.send` (#1496). Same discipline as the Rust branch
   // above, and EXCLUSIVE for the same reason: the field's declared type off
@@ -2590,6 +2601,56 @@ export function rustFieldTypeName(raw: string): string | null {
   return seg;
 }
 
+/**
+ * `self.method()` in Rust — the method on the type the call sits inside.
+ *
+ * The owner is the calling method's qualified-name prefix (`Target::run` →
+ * `Target`), which is where the `impl` block's type ends up. A free function
+ * has no `self`, so a caller whose qualified name carries no owner declines.
+ * Exactly one candidate must belong to that owner: a project with two `impl`
+ * blocks for the same type is normal, two same-named methods on it is not, and
+ * guessing between them is the failure this replaces.
+ */
+function matchRustSelfCall(
+  methodName: string,
+  ref: UnresolvedRef,
+  context: ResolutionContext,
+): ResolvedRef | null {
+  const caller = context.getNodeById?.(ref.fromNodeId);
+  if (!caller?.qualifiedName) return null;
+  const sep = caller.qualifiedName.lastIndexOf('::');
+  if (sep <= 0) return null; // a free fn has no `self`
+  const owner = caller.qualifiedName.slice(0, sep);
+
+  let owned = context
+    .getNodesByQualifiedName(`${owner}::${methodName}`)
+    .filter(
+      (n) =>
+        n.kind === 'method' &&
+        n.language === 'rust' &&
+        n.qualifiedName === `${owner}::${methodName}`,
+    );
+  // Rust's extracted qualified names omit module paths. Two modules can
+  // each declare `Target`; matching just `Target::reset` does not establish
+  // ownership. In that case require a single owner declaration in the
+  // caller's file and a method in that file. Otherwise leave it unresolved.
+  // A unique owner still permits ordinary impl blocks split across files.
+  const owners = context.getNodesByQualifiedName(owner).filter((n) =>
+    n.language === 'rust' && ['struct', 'enum', 'union', 'trait', 'class'].includes(n.kind));
+  if (owners.length > 1) {
+    if (owners.filter((n) => n.filePath === caller.filePath).length !== 1) return null;
+    owned = owned.filter((n) => n.filePath === caller.filePath);
+  }
+  if (owned.length !== 1) return null;
+
+  return {
+    original: ref,
+    targetNodeId: owned[0]!.id,
+    confidence: 0.9,
+    resolvedBy: 'qualified-name',
+  };
+}
+
 /**
  * Resolve a Rust call through a field of the enclosing type —
  * `self.inner.run()`, emitted by the extractor as `self.inner.run` (#1585).