Repository navigation
fix(kotlin): a call on an imported type reaches that package's class when two share a name - #3013
Merged
Conversation
…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>
4 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In Kotlin, when two packages each declare a class with the same name, a call on a value of that type now links to the class the file imports, even when that class lives in a file named for something else (
FooConverterinConverters.kt). Before, it went to whichever class came first.The Java calls this PR fixed when @arttttt opened it, a Java file calling a Kotlin class it imports (okhttp's Java tests and
mockwebserver3.MockWebServer, SystemUI'sBlurUtils), are now resolved on main by #1942, which types a Java call by its receiver's declared type. So on today's main the change shows in Kotlin, as the test does.Root cause
Among same-named candidates,
resolveMethodOnTypekeeps the one the call file imports by matching the import's path against each candidate's file path (com/example/b/FooConverter.kt). That holds for Java, where a public class names its file, but a Kotlin file is often named for something else, so no candidate matched and the first one won:import com.example.b.FooConverterfollowed byfooConverter.convert()reachedcom.example.a::FooConverter::convert.From the contributor, and what we changed
From @arttttt (#539), unchanged: the qualified-name match in
resolveMethodOnType(import com.example.b.FooConverterexpectscom.example.b::FooConverter::convert, which the package directive gives every JVM declaration), ahead of the file-path match, which stays as the fallback, and theresolution.test.tscase for Kotlin same-name classes inConverters.ktfiles.Left out: the PR's other two parts, indexing Kotlin class properties in
extractFieldand reading Kotlin imports without a trailing;in the import resolver. Both have since landed on main another way (Kotlin properties are indexed asfield/constantnodes and Kotlin import mappings exist), so those hunks no longer apply.Compared with the other Kotlin PRs: #1933 (Kotlin receiver typing, now on main) types the receiver but does not choose between same-named classes, so the test is still red on main. #1948 (open) covers a static
Type.method()call and extensions on imported types (overlapping #2258) and re-reads the source for each reference; it does not touch this lookup either.Our changes: a CHANGELOG bullet in the house format, which says that Java calls on such a class now link through #1942, and the commit scope
kotlin. Rebased onto main40f58517.Validation
Tests. The Kotlin same-name case in
resolution.test.tsis red on main (com.example.a::FooConverter::convert) and green here.A/B against main
40f58517(with #1942's Java receiver typing and the Kotlin changes of #1933, #1761 and #2258), fresh indexes with main's kernel, edges compared by site and target identity. The lookup serves every JVM language, so the Java repos ran too:No edge changes on today's main: none of the twelve repos has the test's shape (a typed call on a class whose name another package also declares, the imported one in a file named otherwise) on a resolved call. Against main
4eb8c2a5, before #1942, the same change moved 8 Java calls onto the Kotlin class their file imports, all right: okhttp'smaven-testsAppTestandmodule-testsJavaModuleTest(import mockwebserver3.MockWebServer) had reached the deprecatedokhttp3.mockwebserver.MockWebServer'sstart,enqueueandurl, and SystemUI'sGlobalActionsImplandScrimController(import com.android.systemui.statusbar.BlurUtils) had reachedshared.system's JavaBlurUtils. Main now types those receivers by their declared Java type and reaches the right class at 0.9 without this change. http4s, scalachess (Scala) and serilog (C#) were identical in that run as well.Cost (mihon, median of 3 interleaved fresh indexes, on main
4eb8c2a5): process CPU 25.7 s on main vs 25.2 s, wall 8.2 s vs 8.3 s.Suite:
CODEGRAPH_KERNEL_EXPECT=1 npx vitest run --maxWorkers=4 --minWorkers=1on09a7e237(Windows 11, kernel built): 775 files passed, 6 skipped; 9,248 tests passed, 85 skipped, 0 failed.Lands #539 by @arttttt.
🤖 Generated with Claude Code