Skip to content

Kotlin: fix type of nested class reached via imported outer type#8238

Open
vlsi wants to merge 2 commits into
openrewrite:mainfrom
vlsi:kotlin-shorten-fqn
Open

Kotlin: fix type of nested class reached via imported outer type#8238
vlsi wants to merge 2 commits into
openrewrite:mainfrom
vlsi:kotlin-shorten-fqn

Conversation

@vlsi

@vlsi vlsi commented Jul 10, 2026

Copy link
Copy Markdown

Why

On Kotlin sources, ShortenFullyQualifiedTypeReferences corrupts a nested type reached through an imported outer type. Given:

import org.apiguardian.api.API

@API(status = API.Status.EXPERIMENTAL, since = "5.6")
class Foo

it rewrites API.Status.EXPERIMENTAL to Status.EXPERIMENTAL without adding an import, so Status no longer resolves and the file fails to compile. The equivalent Java source is left untouched. This surfaced running CodeCleanup over Apache JMeter's mixed Java/Kotlin sources — #8237.

What

The root cause is in Kotlin type attribution, not the recipe. PsiElementAssociations.matchClassId0 matched a reference's source text only against the fully-qualified class name (org.apiguardian.api.API.Status). The import-shortened form API.Status never matched, so the lookup walked up the outerClassId chain and returned the outer class org.apiguardian.api.API. The nested reference therefore carried the outer type, whose owningClass is null — which is exactly the guard ShortenFullyQualifiedTypeReferences uses to decide a reference is top-level and safe to shorten.

matchClassId0 now resolves every way a nested type can be written to its own class id:

  • fully qualified (foo.bar.A.B) and relative to an imported outer (A.B) match by text;
  • an import alias (import ... API as GuardApi, then GuardApi.Status) matches by leaf name plus nesting depth, since the aliased root matches neither the fully-qualified nor the package-relative name;
  • an imported nested type (import foo.bar.A.B, then B.A.C) matches a trailing run of the nested names.

The two multi-segment fallbacks require at least two segments, so a single receiver name whose leaf repeats deeper (the outer A of A.B.A.C) is not matched too deep.

How to verify

./gradlew :rewrite-kotlin:test — the full module passes, including the new ShortenFullyQualifiedTypeReferencesKotlinTest cases and the KotlinTypeMappingTest nestedFieldAccess* / nestedTypesWithSameSimpleNameFromDifferentOuters cases.

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Jul 10, 2026
@vlsi
vlsi force-pushed the kotlin-shorten-fqn branch from cda1461 to 9b7f640 Compare July 10, 2026 22:32
@vlsi vlsi changed the title Add failing test for ShortenFullyQualifiedTypeReferences on nested Kotlin types Kotlin: fix type of nested class reached via imported outer type Jul 10, 2026
@vlsi
vlsi force-pushed the kotlin-shorten-fqn branch 2 times, most recently from 238cabb to 2b0c74b Compare July 11, 2026 06:47
When a nested type is referenced through an imported outer type (`import
org.apiguardian.api.API`, then `API.Status`), the parser gave the reference the
outer type `org.apiguardian.api.API` instead of the nested type
`org.apiguardian.api.API$Status`. `matchClassId0` compared the reference text
only against the fully-qualified class name, so the import-shortened form
`API.Status` never matched and the lookup walked up the `outerClassId` chain to
the top-level class, leaving `owningClass` null.

Match the package-relative class name too, and fall back to the leaf name plus
nesting depth for alias imports (`import ... API as GuardApi`, then
`GuardApi.Status`) and to a trailing run of the nested names for imported nested
types (`import foo.bar.A.B`, then `B.A.C`). A nested type reached any of these
ways now keeps its own class id and populates `owningClass`, so
`ShortenFullyQualifiedTypeReferences` leaves `API.Status.EXPERIMENTAL` alone
instead of shortening it to an unresolved `Status.EXPERIMENTAL`.

See openrewrite#8237

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vlsi
vlsi force-pushed the kotlin-shorten-fqn branch from 2b0c74b to a0bd56d Compare July 11, 2026 07:01
@timtebeek
timtebeek self-requested a review July 11, 2026 13:01
@vlsi

vlsi commented Jul 12, 2026

Copy link
Copy Markdown
Author

I just saw the build produced 27635 lines. That is huge. Have you considered reducing the output? I see you have --info which sounds strange to me.
At this scale I do not think build log is helpful. How do you identify the reasons for the build failure?

implementation(kotlin("stdlib", kotlinVersion))

testImplementation("org.junit-pioneer:junit-pioneer:latest.release")
testImplementation("org.apiguardian:apiguardian-api:1.1.2")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we rewrite the test cases to be generic, i.e. possibly use some standard library references or existing dependencies. This way we can refrain from adding this (test) dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants