Repository navigation
fix(java): resolve receivers on their declared type, leave library types unguessed - #1942
danusha2345 wants to merge 1 commit into
Conversation
35a787b to
0b4bd03
Compare
|
Rebased onto main after the resolution wave. 7 of this PR's 12 cases now pass on main (#2126, #2166, #2169, #2171, #2177), so their code and tests are dropped: the local-variable regex (main's version with type arguments and for-each is kept) and
🤖 Generated with Claude Code |
5a22e2a to
80b7d4e
Compare
80b7d4e to
be018ef
Compare
|
@danusha2345, one more receiver form this PR doesn't cover yet, in case it fits its scope: a Java // src/com/app/Repo.java
package com.app;
public class Repo { public void query(String sql) {} }// src/com/app/Svc.java
package com.app;
import org.springframework.jdbc.core.JdbcTemplate;
public class Svc {
private JdbcTemplate jdbc;
JdbcTemplate getJdbc() { return jdbc; }
public void explicitType() { JdbcTemplate t = jdbc; t.query("x"); }
public void varFromField() { var t = jdbc; t.query("x"); }
public void varFromParam(JdbcTemplate p) { var t = p; t.query("x"); }
public void varFromCall() { var t = getJdbc(); t.query("x"); }
public void varFromNew() { var t = new JdbcTemplate(); t.query("x"); }
}
The Java patterns in If you'd rather keep this PR's scope as it is, I'm happy to send it as a follow-up once this lands. |
97d91cf to
9aec542
Compare
…Go field/interface types, workspace globs, baseUrl imports) Conflicts: name-matcher's receiver typing takes main's TS array/indexed-property block, then our Java branch (colbymchenry#1942) and Go outside-type check (colbymchenry#1954); `tsFieldCall` (main's new home of the this-field tie-break) keeps colbymchenry#1983's code-unit ordering; types.ts / index.ts keep colbymchenry#1921's `resolveModuleFile` beside main's `isDeclaredPackage` / `isIndexedFile`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9aec542 to
aa200df
Compare
…Rust Self:: and std impls, colbymchenry#2688 synthesis inputs, first foreign merges) Conflicts: name-matcher takes main's `isGoOutsideImport` family in place of colbymchenry#1954's `isGoExternalImport`; keeps colbymchenry#1942's Java same-language preference and colbymchenry#1947's `cls` beside main's `withinCppTranslationUnit`; colbymchenry#1949's Java static field block and main's Rust `Self::` block both stay; colbymchenry#1935's `findOwnMethod` gets main's Go standard-method-name condition. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
aa200df to
5a1bfcf
Compare
…ymchenry#1921, colbymchenry#1935, colbymchenry#1955, colbymchenry#1957, colbymchenry#1965, colbymchenry#2575, colbymchenry#2595, colbymchenry#2597, colbymchenry#2603–colbymchenry#2608, colbymchenry#2611 landed upstream) Conflicts: files owned by the foreign PRs now in main (colbymchenry#1921's Python import resolver and tests, colbymchenry#1935's class-strategy method) take main's version; our colbymchenry#1954/colbymchenry#1942/colbymchenry#1945/colbymchenry#1939/colbymchenry#1715/colbymchenry#1951/colbymchenry#2308 hunks sit beside main's new Rust and Python blocks as in the rebased PR branches; java.rs keeps the integration's `constant_declaration` fields under main's initializer walk; three CHANGELOG lines the merged PRs had already contributed are deduplicated, and the method blocks the merge duplicated in queries.ts and extraction/index.ts (the resolution-edge queries and the sync resurrection helpers, present once on main) are kept once. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
b1c361d to
2151f25
Compare
…pes unguessed
Receivers the field lookup missed are typed: `Outer.this.f`, and a field
of an outer class read inside an inner or anonymous class (the lookup
only read the tightest enclosing class, so `mStore.save()` inside a
`new Runnable() {…}` went to a name-only guess). A chain of fields
(`owner.ctx.getResources()`) is read hop by hop, and a hop whose type is
a library class gets no edge instead of a same-named project method.
A type is read with the package or outer class written in source and
with the file's imports, so `import java.util.Iterator` is not taken for
a same-named type nested in another package of the project, and an
imported nested type (`import p.Http.Cookie`) is the one the import
names. Among same-named candidates no import pins, a Java call prefers
the Java declaration over a Scala or Kotlin one.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2151f25 to
32eee85
Compare
…rary type gets no guess
Java typed a call receiver from a local declaration or a plain field of
the enclosing class only. A field of an outer class read inside an inner
or anonymous class (mHandler.post(...) inside new Runnable() {...}),
Outer.this.f, a chain of fields (this.mOwner.mRepo.save()) and a local
whose type has type arguments fell through to the name-only strategies,
and so did every receiver whose declared type is a library class:
parcel.readInt() on an android.os.Parcel went to a project
VersionedParcel.readInt, list.size() to any project size().
The declared type is now read for each of those shapes, a chain one hop
at a time with each hop's own imports, keeping a written package or
outer class (Map.Entry, play.api.mvc.BodyParser). A receiver whose
declared type the project does not declare, or that an import or a
written package pins outside the project, gets no edge. When no import
decides between same-named types, a Java call prefers the Java
declaration over a Scala or Kotlin one.
Also, from review: a field the class around the call declares is read
as written before the member reader, which keeps only the last name
(PauseTest.Suite, a field written with its package); a local or loop
variable keeps the package its declaration writes; a field typed by a
type parameter goes to its bound, and a class the class head names as
a type argument (Comparable<ApplicationModule>) is not taken for one; a
nested type a superclass declares, or one an on-demand import of its
class names (import play.mvc.Http.*), counts as named; and a typed miss
on a type no project class or interface is named (a URI, an enum's
name()) gets no guess, as on main.
Validation:
- java-receiver-type.test.ts: 19 tests, the PR's 5 red on main; the
other 14 pass on main, and our 8 each fail without their change.
- A/B against main 2577601 (with #2942 and #2950; the same sites
change against 7cceba5), sites added/removed/retargeted: jextract
+2/-1/106, spring-modulith +1/-3/2, Minestom +52/-91/71, SystemUI
+53/-319/96, spring-ai +69/-659/532, kestra-java +5/-504/162,
playframework +0/-196/599; click and gin identical. Of the removed
edges main had at 0.9, 1,004 were wrong (an import oracle plus 16
read by hand) and 4 right (a lambda parameter, two nested types of a
Scala object, a field read under a static import of a same-named
member). Retargets land on the declared type or the supertype
declaring the method, except about 16 build()/clone() calls on
spring-ai's nested option builders, which move from one wrong target
to another.
- Cost (3 interleaved rounds): spring-ai wall 27.9 s on main vs
25.9 s, CPU 90.2 s vs 88.1 s. SystemUI read 71.3 vs 79.4 s wall
under other jobs' load; CPU profiles of both arms put the new code at
about 0.6 s of 115 s.
Original PR #1942 by @danusha2345.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @danusha2345! This landed in #2965, with you as the commit author. Your declared-type reader is the heart of it: on our A/B repos it removed about a thousand wrong name-guessed links and moved over a thousand calls onto their declared types. We added seven small fixes that the A/B turned up, each with its own test. Among them: a local keeps the package written in its declaration, a type-parameter field goes to its bound, a nested type can be reached through a superclass or an on-demand import, and main's library-type guard applies after a typed miss. We also rewrote the CHANGELOG entry. |
…n its declared type Kotlin indexes a class property without a signature, and a primary-constructor property (class A(private val repo: Repo)) is no node at all, so a call through either (probe.close() on a lateinit var probe: HttpProbe) found no receiver type and fell to the name-only strategies: the interface's method, or any same-named method elsewhere. A call through a chain (engine.pump.drain()) was emitted as its bare method name, and a local bound to a qualified call (val lease = HardwareLock.tryBegin()) was untyped. tree-sitter-kotlin also predates fun interface: such a declaration parsed as a broken function and, with a doc comment after it, its error recovery swallowed the next declaration too. The declared type is now read from the declaration itself: a property's own lines, the class header for a constructor property, a callee's declared return type for a local bound to a call, an enum entry's enum, each segment of a receiver chain in turn (the chain is kept in the reference name, by the wasm extractor and the native kernel alike), a superclass's property, and an outer function's local inside an anonymous object. A receiver whose type the project does not declare (Regex, Properties, a JDK class) gets no edge instead of a name-only guess, unless the project extends that type. The extractor's preParse blanks the fun of a fun interface, offset-preserving, so the file parses cleanly in both paths. Validation: - kotlin-property-receiver.test.ts: 17 tests, 12 red on main, all green here; kernel-kotlin-parity.test.ts pins the fun interface fixture. - Kernel parity (kotlin), 0 diffs: okhttp 588/619 files byte-identical (main's kernel 578, the rest deferred to wasm on parse errors), mihon 891/940 (887), nowinandroid 340/350 (339): the fun interface files no longer defer. - A/B against main with #1942 (positions only: the extractor now keeps a chain in the reference name): okhttp +299/-257, 2,554 retargeted, 2,158 of them test calls moving from the deprecated okhttp3.mockwebserver.MockWebServer (a 0.65 name guess) to the mockwebserver3.MockWebServer each file imports; mihon +82/-118, 337 retargeted (Preference get/changes/set/register onto the declared Preference); nowinandroid -9, 23 retargeted (properties onto their concrete test doubles); SystemUI's Kotlin +1/-104, 7 retargeted; jextract, click and gin identical. The removed are name guesses on library types (okio sinks, collections' filter and clear, OkHttp bodies, MutableStateFlow.update, Kotlin's apply). 7 right edges are lost where a chain passes through a library wrapper (Lazy.value, StateFlow.value, Pair.first, a ViewBinding's view). - Cost (mihon, median of 3 interleaved indexes): CPU 25.8 s on main with #1942 vs 26.7 s, wall 8.3 vs 8.4 s. Original PR #1933 by @danusha2345. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mchenry#2972; 10 landed PRs yield to main; EXTRACTION_VERSION 56) Main landed ten more of our PRs as our own commits with the maintainer's additions (colbymchenry#2923 ← colbymchenry#1936, colbymchenry#2924 ← colbymchenry#1951, colbymchenry#2925 ← colbymchenry#1939, colbymchenry#2930 ← colbymchenry#2921, colbymchenry#2940 ← colbymchenry#1736, colbymchenry#2942 ← colbymchenry#1947, colbymchenry#2950 ← colbymchenry#1952, colbymchenry#2961 ← colbymchenry#1954, colbymchenry#2965 ← colbymchenry#1942, colbymchenry#2972 ← colbymchenry#1933) and the module-path half of colbymchenry#2308 (colbymchenry#2955). Where the integration carried our version, main's wins: the enclosing-scope member pick and its tests, the Go and Java receiver typers, the delegating-call self-loop rule, the duplicated-module copy confidence, the C #if collapse, the module-path resolution of Rust paths, the trail/path hardening in utils.ts. Kept beside main: colbymchenry#1875's Verilog hooks and status fields, colbymchenry#2839's implementations command, colbymchenry#1923, colbymchenry#2922, colbymchenry#2258, colbymchenry#1761, colbymchenry#1583, the bare-call half of colbymchenry#2308 (rust-use-bindings.ts). CHANGELOG is main's plus the open PRs' bullets. EXTRACTION_VERSION 56: the C #if group handling (colbymchenry#2940) and the kernel change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…when two share a name When several classes share a name, resolveMethodOnType picks the one the call file imports by matching the import's path against the candidate's file path (com/example/b/FooConverter.kt). That holds for Java, where a public class names its file, but a Kotlin class often lives in a file named otherwise (FooConverter in Converters.kt), so no candidate matched and the call went to whichever class came first. The imported name is now matched against each candidate's qualified name first: import com.example.b.FooConverter expects the method com.example.b::FooConverter::convert, which the package directive gives every JVM declaration whatever its file is called. The file-path match stays as the fallback. The Java calls this once fixed, a Java file calling a Kotlin class it imports (okhttp's Java tests and mockwebserver3.MockWebServer, SystemUI's BlurUtils), are now resolved on main by the receiver's declared type (#1942), so on today's main the change shows in Kotlin. From #539 we take only this change and its test. Its other two parts, indexing Kotlin class properties and reading Kotlin imports without a trailing semicolon, have since landed on main another way. Validation: - resolution.test.ts: the Kotlin same-name case (#314's shape in Converters.kt files) is red on main, green here. - A/B against main 40f5851 (with the Java receiver typing of #1942 and the Kotlin changes of #1933, #1761 and #2258): no edge changes on okhttp, SystemUI, nowinandroid, mihon, jextract, spring-modulith, Minestom, spring-ai, kestra-java, playframework, click or gin; none of them has the test's shape on a resolved call. Against main 4eb8c2a, before #1942, it moved the 8 Java calls above onto the Kotlin class their file imports; main now reaches them at 0.9. - Cost (mihon, median of 3 interleaved indexes, on main 4eb8c2a): CPU 25.7 s on main vs 25.2 s, wall 8.2 s vs 8.3 s. Original PR #539 by @arttttt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
Java already types a call receiver from a local declaration or a plain field. Two gaps remain.
parcel.readInt()on anandroid.os.Parcel→ a projectVersionedParcel.readInt;mHandler.post(...)orlist.size()→ any projectpost/size;Log.e(...)→ a projectFalsingLog.e.this.fandOuter.this.f;Map<String, Foo> byName);this.mOwner.mRepo.save()).On decompiled Android SystemUI (3,421 Java files),
mainhas about 21.8k call edges whose receiver's declared type is a library class but that land on a project method. Another ~2.9k have a project-class receiver but go to an unrelated class. In playframework's Java sources (790 files) the numbers are ~1.7k and 125.Fix
All changes are in
src/resolution/name-matcher.ts. The extractor already emits the full receiver text, so the kernel and extraction version are unchanged.A declared type is read for every receiver shape above. A chain is typed one hop at a time, and each hop's field type is read with its own file's imports.
A written qualifier is kept:
Map.Entryandplay.api.mvc.BodyParser<B>stay as written, andimport p.Http.Cookiepins that exact nested class.When the declared type is a library class, the call gets no edge. A type counts as a library class when:
import java.util.Iteratornext to a projectUByteArray.Iterator);A short all-caps name (
T,VH) is taken for a type parameter and left alone.A static call on an imported or
java.langlibrary class (Log.d,TextUtils.isEmpty) is left unresolved the same way.When no import decides between same-named types, a Java call prefers the Java declaration over a Scala or Kotlin one (
play.data.Formvsplay.api.data.Form).This is the Java counterpart of the Kotlin receiver typing in #1933. The two share an idea but not code yet:
JAVA_EXTERNAL_TYPEhere,KOTLIN_EXTERNAL_TYPEthere. Whichever lands second will need a small textual merge ininferJavaFieldReceiverType, whose Java/Kotlin field block this PR narrows to Kotlin.Measured
Each corpus was indexed with
main(ba3c21e) and with this branch. Node counts are identical.Every gained edge is a typed resolution at 0.9. I hand-checked samples against source; every sampled edge was correct:
Indexing time on SystemUI isn't higher.
Known limits
PARSER.parseFrom→Parserrather thanAbstractParser. This is the same choice as for Kotlin.records aren't indexed as types, an existing extractor gap. A call on a record-typed receiver now gets no edge instead of a guess.Tests
The new
__tests__/java-receiver-type.test.tshas 12 cases: 8 fail onmain, and 4 are regression guards. The Java-over-Scala guard also fails if the tie-break is removed. The full suite passes: 275 files, 4,730 tests.🤖 Generated with Claude Code