call targets are invented when they cannot be worked out, and the invented ones look exactly like real ones
Found in a read-only audit of v2.3.8. Three languages, three different pieces of code, the same mistake.
What happens
Several pieces of resolution added since 2.3.6 will rewrite a target to a name inside the repository that the source never actually referred to. Leaving the target as a plain name, as before, at least said honestly that it could not be worked out. An invented name says the opposite, and nothing later can tell the two apart.
In Java and Kotlin, _resolve_typed_method_target (code_review_graph/parser.py:5470) builds <file>::<Type>.<method> at :5507 whenever it knows the type of the thing the method was called on, without checking that the type actually has that method.
In Rust, a call to something from the standard library is rewritten as if it lived in the local crate.
In Python, this is the second half of #981.
Reproduction
Java, one file, no package, no imports:
public class One {
public int run() { Helper h = new Helper(); return h.hashCode(); }
}
class Helper { public int build() { return 1; } }
CALLS -> One.java::Helper.hashCode (matches no node)
extra = {"receiver_resolution": "typed_receiver"}hashCode comes from Object; Helper does not define it. The trigger is that Helper is declared at the top level of the same file, so the code knows the type -- with Helper in a separate file and no import, the target correctly stays a plain name.
Rust, a Cargo crate whose src/lib.rs contains let v = Vec::new();:
CALLS -> src/lib.rs::new (matches no node)Why it matters
callers_of, get_impact_radius, tests_for and find_dead_code all trust a target that has a file path in it. A plain name is something every one of them already treats as coming from outside the repository. A name with a file path in front of it passes every check they make, while pointing at nothing. In the Rust case the invented name is worse than what it replaced, because a target containing :: is skipped by the later pass that repairs plain names.
Suggested fix
When a target cannot be worked out with confidence, leave it as a plain name rather than building a name that looks right. confidence_tier already exists and would say this well, but only one of the four resolvers currently sets it. For the Java and Kotlin case specifically, checking that the type really does have the method -- or inherits it from something the graph knows about -- would be enough.
C# already avoids this: it writes Class::method for scoped_resolver to pick up, and that only rewrites when it finds a real node. That is the pattern the others want.
A fourth case was reported to me but I have not reproduced it, so treat it as unconfirmed: in JS/TS, import * as NS turning NS.thing() into an invented ns.ts::NS.thing.
Source: tirth8205/code-review-graph