Problem
Tier-1 reference resolution matches candidates on a def's short name only. indexer::index_repo_incremental indexes every def under name.rsplit('.').next(), and ExtractedRef.qualifier is recorded but never consulted when choosing candidates. The existing comment states the tradeoff plainly:
Defs are indexed under their short name: a method def like Calc.push is indexed under push too, so a bare-identifier ref matches both plain functions and qualified methods whether or not ref.qualifier is set (the receiver variable rarely equals the type name, so this is a deliberate over-approximation).
That over-approximation is cheap when the resolution scope is a single file (TS, Rust) or a small package directory (Go). It is expensive in Java, where the scope is a whole package plus its imports and method names like next, get, size, write, read, close are everywhere.
Evidence
Measured on google/gson (1,534 tests) after #31. Editing JsonStreamParser.hasNext — a peripheral class — selects 1,420 of 1,534 tests. The impact path:
JsonStreamParser.hasNext --calls--> JsonStreamParser.next
JsonStreamParser.next --calls--> GsonBuilder.newImmutableList <-- false hop
GsonBuilder.newImmutableList --calls--> Gson.Gson
Gson.Gson --calls--> <every test that does `new Gson()`>
The second hop is spurious. newImmutableList iterates a collection and calls iterator.next(); short-name matching binds that to JsonStreamParser.next, because both live in package com.google.gson and are therefore in scope for each other. From Gson's constructor the blast radius is effectively the whole suite.
The selection is still a superset, so this is a precision bug, not a soundness bug. But it makes Java selection much less useful on repos with large flat packages. spring-boot, which has many small packages, is unaffected in practice (62 of 17,720 for an equivalent edit).
Proposed fix
Make resolution qualifier-aware, preferring a qualified match and falling back to the current short-name behaviour so nothing under-selects:
- Language plugins resolve a receiver to a declared type where they cheaply can — for Java, fields, formal parameters and local variable declarations all carry an explicit type in the AST, so
parser.peek() is known to target JsonReader.peek.
ExtractedRef carries that resolved type (a new field, or a convention on qualifier).
indexer keys defs by both Class.method and bare method. When a ref has a resolved type, match the qualified key; when it does not, fall back to the short name exactly as today.
Falling back rather than replacing is what keeps this sound: an unresolvable receiver (a generic, an interface obtained from a factory, a chained call) is no worse off than now.
Notes
- This is a core change, not a Java-only one. TS and Go would benefit too but are far less affected, so Java is the natural place to validate it.
- Interface dispatch must keep widening: if the receiver's declared type is an interface, every implementation of that method has to stay a candidate. That is the current behaviour and must not regress.
- Good regression corpus: gson
JsonStreamParser.hasNext should drop well below 1,420 while JavaVersion.extractBeginningInt must stay wide (~1,427), since JavaVersion genuinely feeds ReflectionAccessFilterHelper and therefore nearly all of Gson's reflective serialization.
Follow-up to #31.
Problem
Tier-1 reference resolution matches candidates on a def's short name only.
indexer::index_repo_incrementalindexes every def undername.rsplit('.').next(), andExtractedRef.qualifieris recorded but never consulted when choosing candidates. The existing comment states the tradeoff plainly:That over-approximation is cheap when the resolution scope is a single file (TS, Rust) or a small package directory (Go). It is expensive in Java, where the scope is a whole package plus its imports and method names like
next,get,size,write,read,closeare everywhere.Evidence
Measured on google/gson (1,534 tests) after #31. Editing
JsonStreamParser.hasNext— a peripheral class — selects 1,420 of 1,534 tests. The impact path:The second hop is spurious.
newImmutableListiterates a collection and callsiterator.next(); short-name matching binds that toJsonStreamParser.next, because both live in packagecom.google.gsonand are therefore in scope for each other. FromGson's constructor the blast radius is effectively the whole suite.The selection is still a superset, so this is a precision bug, not a soundness bug. But it makes Java selection much less useful on repos with large flat packages. spring-boot, which has many small packages, is unaffected in practice (62 of 17,720 for an equivalent edit).
Proposed fix
Make resolution qualifier-aware, preferring a qualified match and falling back to the current short-name behaviour so nothing under-selects:
parser.peek()is known to targetJsonReader.peek.ExtractedRefcarries that resolved type (a new field, or a convention onqualifier).indexerkeys defs by bothClass.methodand baremethod. When a ref has a resolved type, match the qualified key; when it does not, fall back to the short name exactly as today.Falling back rather than replacing is what keeps this sound: an unresolvable receiver (a generic, an interface obtained from a factory, a chained call) is no worse off than now.
Notes
JsonStreamParser.hasNextshould drop well below 1,420 whileJavaVersion.extractBeginningIntmust stay wide (~1,427), sinceJavaVersiongenuinely feedsReflectionAccessFilterHelperand therefore nearly all of Gson's reflective serialization.Follow-up to #31.