From de845672258a2e08e4c01ab7d53caccbf3488dcf Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Tue, 27 Oct 2026 10:29:02 -0500 Subject: [PATCH] fix(resolution): a call a method hands on to another object no longer links back to it matchMethodCall's strategy 3 guesses a method by its name when the receiver's type is unknown. With one method of that name in the project it was taken outright, so a wrapper handing its call on, like Play's `def acceptsURL(url) = proxied.acceptsURL(url)` or vben's `form.validate()` inside `FormApi.validate()`, linked to the method making the call, which then looked recursive in callers and impact. The single-candidate branch now gives no edge when that method is the caller itself and the receiver is another object (not this, self, Self, super or cls), as the word-overlap branch already did. On top of the PR: Go is left out. A Go method's receiver and struct fields are typed before this point, so a Go call that still reaches the guess is on a local the resolver could not type, mostly a range variable over the type's own children, which is real recursion: cobra's `sub.checkCommandGroups()` over `c.commands`, etcd's `child.Repr(...)`, prometheus's `subParameter.toLabels()`. Also a TypeScript wrapper test, a Go range-variable recursion test, and a CHANGELOG bullet. Validation, main (3a93f57a) vs this change, 24 repos in 11 languages: no edge added; 63 self-loop edges removed, 56 of them wrong (delegations to a library or another object: playframework 19, vben 9, http4s 8, mihon 8, okhttp 5, ripgrep 3, flask 2, spring-modulith 2) and 7 real recursion through another instance of the caller's type, which now gets no edge (okhttp's Kotlin extension functions on File and Throwable 3, fastapi's _IncludedRouter 2, a serilog C# extension method 1, spring-modulith 1). As written the PR also removed the 8 Go recursion edges. Re-run on playframework after #2875 changed Play's resolver: 17 removed, the same less Java's two Multipart.* calls, which main now links to the Scala object itself. Index CPU on okhttp, median of 8 interleaved runs: 32.8 s on main, 32.3 s with this change. Tests: 2 of 4 fail on main; the PR as written fails the Go range-variable case. Original PR #1939 by @danusha2345. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/resolution.test.ts | 76 ++++++++++++++++++++++++++++++++++ src/resolution/name-matcher.ts | 25 +++++++++++ 3 files changed, 102 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c40c77df2..93523c1374 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In CFML, `createObject("component", "contenscms.core.queue.batchcontext")`, and the one-argument `createObject("contenscms.core.queue.batchcontext")`, now link to the component they create, found by its path the way `extends` finds one, as `new` already did. A call chained onto the new object, like `createObject(…).init()` or `new contenscms.core.queue.batchcontext().init()`, now links to that component's `init`, or to the one it inherits. In a framework's own source, a path like `wheels.JobWorker`, whose first part is the framework itself, now also finds the component at the project root, for `extends` as well. Before, `createObject` showed up as a call to a function that doesn't exist, and the chained `init` was linked to whichever component's `init` came first, or to none. Thanks @HarryMuc for the report. (#2652) - In VB.NET, a call on a variable, parameter, field or property now reaches a method of the type it is declared with — one that type inherits, or an extension method written for it — and nothing when that type comes from outside your project, instead of any project method that merely shares the name. - In VB.NET, a call to one of .NET's own methods such as `Add`, `Contains`, `Clear` or `Dispose` on a value whose type isn't known is no longer linked to a project method of that name, unless the value is named after that method's class. +- A method that hands its call on to another object's method of the same name, like a wrapper's `def unbind(value) = wrapped.unbind(value)` or `form.validate()` inside its own `validate()`, no longer links that call back to itself, so the wrapper no longer looks recursive in callers and impact. Calls through `this` or `self`, bare calls, and calls in Go keep their link when they really are recursive. Thanks @danusha2345. - In VB.NET, a type name is looked up the way VB.NET does it — through the namespaces around it, the file's and project's `Imports` (aliases included), and the caller's own project — so a class declared in several namespaces or projects no longer draws every call to whichever copy was indexed first, and two candidates nothing tells apart get no link at all. - In VB.NET, a call with no receiver, or on `Me`, now reaches only a member of the class it is written in, of a class it inherits, or of a Module, and no longer a nearby class's member of the same name. - VB.NET designer code such as `New System.Drawing.Point(…)` or `Me.Size = New System.Drawing.Size(…)` is no longer linked to a project type or member that only shares the name, and a class nested inside another is matched by its bare name only from inside that class or one that inherits it. diff --git a/__tests__/resolution.test.ts b/__tests__/resolution.test.ts index 631132d56c..197ebd4810 100644 --- a/__tests__/resolution.test.ts +++ b/__tests__/resolution.test.ts @@ -6470,4 +6470,80 @@ bracketed() expect(reachedFrom('consumer.ts', 'HiddenFace')).toBe(false); }, 30000); }); + + describe('Name-only method match never binds a delegating call to its own caller', () => { + const selfLoops = (g: CodeGraph, file: string): string[] => + g.getNodesInFile(file) + .filter((n) => n.kind === 'method' || n.kind === 'function') + .flatMap((n) => g.getOutgoingEdges(n.id) + .filter((e) => e.kind === 'calls' && e.target === n.id) + .map(() => n.name)); + + it('drops the self-loop for a Scala delegating wrapper', async () => { + fs.writeFileSync(path.join(tempDir, 'Form.scala'), ` +package play.api.data +class WrappedMapping(wrapped: Mapping) { + def unbind(value: String): String = wrapped.unbind(value) +} +`); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + expect(selfLoops(cg, 'Form.scala')).toEqual([]); + }, 30000); + + it('drops the self-loop for a TypeScript wrapper handing its call to a parameter', async () => { + fs.writeFileSync(path.join(tempDir, 'wrapped.ts'), ` +export class WrappedMapping { + unbind(wrapped, value: string): string { + return wrapped.unbind(value); + } +} +`); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + expect(selfLoops(cg, 'wrapped.ts')).toEqual([]); + }, 30000); + + it('keeps real recursion through this./bare calls and a Go named receiver', async () => { + fs.writeFileSync(path.join(tempDir, 'Tree.java'), ` +public class Tree { + public int depth(int n) { return n == 0 ? 0 : this.depth(n - 1); } + public int size(int n) { return n == 0 ? 0 : size(n - 1); } +} +`); + fs.writeFileSync(path.join(tempDir, 'relay.go'), ` +package relay +type relayClient struct{} +func (r *relayClient) sendBatchAttempt(retry bool) error { + if retry { + return r.sendBatchAttempt(false) + } + return nil +} +`); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + expect(selfLoops(cg, 'Tree.java').sort()).toEqual(['depth', 'size']); + expect(selfLoops(cg, 'relay.go')).toEqual(['sendBatchAttempt']); + }, 30000); + + it('keeps Go recursion through a range variable over the type\'s own children', async () => { + // cobra's `sub.checkCommandGroups()` over `c.commands`, etcd's + // `child.Repr(…)`: the loop variable is untyped, and it is the caller's type. + fs.writeFileSync(path.join(tempDir, 'node.go'), ` +package tree +type Node struct{ children []*Node } +func (n *Node) Size() int { + total := 1 + for _, child := range n.children { + total += child.Size() + } + return total +} +`); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + expect(selfLoops(cg, 'node.go')).toEqual(['Size']); + }, 30000); + }); }); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 837eacd06a..e5b0d970c4 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -20604,6 +20604,7 @@ export function matchMethodCall( !(ref.language === 'objc' && objcReceiverReaches(objectOrClass!, targetMethods[0]!, context)) && !(ref.language === 'php' && phpReceiverReaches(objectOrClass!, targetMethods[0]!, context)))) { if (throughCppNamespace() && !isCppConstructorThrough(objectOrClass!, targetMethods[0]!, context)) return null; + if (isForeignReceiverSelfLoop(targetMethods[0]!, objectOrClass!, ref)) return null; return { original: ref, targetNodeId: targetMethods[0]!.id, @@ -20918,6 +20919,30 @@ function importBindsOther(candidate: Node, ref: UnresolvedRef, context: Resoluti return !/\bimport\s*\(/.test(lines.slice(node.startLine - 1, Math.min(node.endLine, node.startLine + 15)).join('\n')); } +/** Receivers that name the current object (or its own class/base) — a call + * through one of these may genuinely recurse into the enclosing method. */ +const SELF_RECEIVERS = new Set(['this', 'self', 'Self', 'super', 'cls']); + +/** + * Strategy 3's single-candidate branch matches by method NAME alone, so a + * delegating wrapper whose method is the only one of that name — + * `wrapped.unbind(v)` inside `unbind(v)` — name-matches the method it is + * written in and the resolver emitted a self-loop. (The word-overlap branch + * already refuses the caller itself.) A receiver that is an explicit, other + * object is by construction not the enclosing method's own instance, so that + * candidate is the one guess we know is wrong: no edge beats a wrong edge. + * Real recursion (`this.foo()`, `self.foo()`, bare `foo()`) is kept; Go's + * named method receiver (`r.foo()`) is typed and never gets here. Go is left + * out altogether: a method's receiver and a struct's fields are typed before + * this point, so a Go call that still arrives here is on a local the + * resolver could not type, mostly a range variable over the type's own + * children — cobra's `sub.checkCommandGroups()` over `c.commands`, etcd's + * `child.Repr(…)` — which is real recursion. + */ +function isForeignReceiverSelfLoop(candidate: Node, receiver: string, ref: UnresolvedRef): boolean { + return candidate.id === ref.fromNodeId && !SELF_RECEIVERS.has(receiver) && ref.language !== 'go'; +} + /** The directory of a Go file, which is its package: Go keeps one package per directory. */ function goPackageDir(filePath: string): string { return path.posix.dirname(filePath.replace(/\\/g, '/'));