mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-11-14 18:23:25 +08:00
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 <noreply@anthropic.com>
This commit is contained in:
committed by
Colby Mchenry
co-authored by
Claude Opus 5.5
parent
b6379e6d5e
commit
0b194448dc
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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, '/'));
|
||||
|
||||
Reference in New Issue
Block a user