From a0bd56d57bd2f31af2c403c945e60c6f5a9aa97f Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Sat, 11 Jul 2026 01:31:56 +0300 Subject: [PATCH] Kotlin: fix type of nested class reached via imported outer type 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 https://github.com/openrewrite/rewrite/issues/8237 Co-Authored-By: Claude Opus 4.8 --- rewrite-kotlin/build.gradle.kts | 1 + .../kotlin/internal/PsiElementAssociations.kt | 20 +- .../kotlin/KotlinTypeMappingTest.java | 198 ++++++++++++++++++ ...ullyQualifiedTypeReferencesKotlinTest.java | 67 ++++++ 4 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 rewrite-kotlin/src/test/java/org/openrewrite/kotlin/ShortenFullyQualifiedTypeReferencesKotlinTest.java diff --git a/rewrite-kotlin/build.gradle.kts b/rewrite-kotlin/build.gradle.kts index 5f084929196..feee357f7c3 100644 --- a/rewrite-kotlin/build.gradle.kts +++ b/rewrite-kotlin/build.gradle.kts @@ -54,6 +54,7 @@ dependencies { implementation(kotlin("stdlib", kotlinVersion)) testImplementation("org.junit-pioneer:junit-pioneer:latest.release") + testImplementation("org.apiguardian:apiguardian-api:1.1.2") testImplementation(project(":rewrite-test")) testRuntimeOnly(project(":rewrite-java-21")) testRuntimeOnly("org.antlr:antlr4-runtime:4.13.2") diff --git a/rewrite-kotlin/src/main/kotlin/org/openrewrite/kotlin/internal/PsiElementAssociations.kt b/rewrite-kotlin/src/main/kotlin/org/openrewrite/kotlin/internal/PsiElementAssociations.kt index 5cab4355bc7..a8b733aad05 100644 --- a/rewrite-kotlin/src/main/kotlin/org/openrewrite/kotlin/internal/PsiElementAssociations.kt +++ b/rewrite-kotlin/src/main/kotlin/org/openrewrite/kotlin/internal/PsiElementAssociations.kt @@ -179,7 +179,25 @@ class PsiElementAssociations(val typeMapping: KotlinTypeMapping, val file: FirFi } private fun matchClassId0(psi: PsiElement, classId: ClassId): ClassId { - if (psi.text == classId.asFqNameString()) { + // A reference to a (possibly nested) type can be written several ways, and each must resolve to + // its own class id rather than collapsing to the outer class id via the `outerClassId` walk below: + // - fully qualified: `foo.bar.A.B` -> matches the fully-qualified name + // - relative to an imported outer: `A.B` -> matches the package-relative name + // - through an import alias: `Alias.B` -> same nesting depth and same leaf name + // - relative to an imported nested: `B.A` for `A.B.A` -> a trailing run of the nested names + // The two multi-segment fallbacks require at least two segments so a single receiver name (e.g. + // the outer `A` of `A.B.A.C`, whose leaf coincidentally repeats deeper) is not matched too deep. + val text = psi.text + if (text == classId.asFqNameString() || text == classId.relativeClassName.asString()) { + return classId + } + val segments = text.split('.') + val relSegments = classId.relativeClassName.pathSegments().map { it.asString() } + if (segments.size >= 2 && segments.size == relSegments.size && segments.last() == relSegments.last()) { + return classId + } + if (segments.size in 2..relSegments.size && + relSegments.subList(relSegments.size - segments.size, relSegments.size) == segments) { return classId } diff --git a/rewrite-kotlin/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java b/rewrite-kotlin/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java index 2c9b118fd93..b3a22272a03 100644 --- a/rewrite-kotlin/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java +++ b/rewrite-kotlin/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java @@ -1862,6 +1862,204 @@ public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, Integer n) { ); } + @Issue("https://github.com/openrewrite/rewrite/issues/8237") + @Test + void nestedFieldAccessViaImportedOuter() { + rewriteRun( + spec -> spec.parser(KotlinParser.builder().classpath("apiguardian-api")), + kotlin( + """ + import org.apiguardian.api.API + + @API(status = API.Status.EXPERIMENTAL, since = "5.6") + class Foo + """, + spec -> spec.afterRecipe(cu -> { + var count = new AtomicInteger(0); + new KotlinIsoVisitor() { + @Override + public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, Integer n) { + // `API.Status` must keep its own nested type rather than collapsing to the imported outer `API`, + // otherwise ShortenFullyQualifiedTypeReferences would rewrite `API.Status` to an unresolved `Status`. + if ("Status".equals(fieldAccess.getSimpleName())) { + JavaType.Class type = (JavaType.Class) fieldAccess.getType(); + assertThat(type.getFullyQualifiedName()).isEqualTo("org.apiguardian.api.API$Status"); + assertThat(type.getOwningClass()).isNotNull(); + assertThat(type.getOwningClass().getFullyQualifiedName()).isEqualTo("org.apiguardian.api.API"); + assertThat(fieldAccess.getName().getType()).isEqualTo(type); + + J.Identifier target = (J.Identifier) fieldAccess.getTarget(); + assertThat(((JavaType.FullyQualified) target.getType()).getFullyQualifiedName()).isEqualTo("org.apiguardian.api.API"); + count.getAndIncrement(); + } + return super.visitFieldAccess(fieldAccess, n); + } + }.visit(cu, 0); + assertThat(count.get()).isEqualTo(1); + }) + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/8237") + @Test + void nestedFieldAccessViaImportedOuterMultiLevel() { + // Same shape as `nestedFieldAccessType`, but the outer type is imported, so the reference is + // package-relative (`A.B.A.C`) rather than root-package or fully qualified. Every nested level must + // keep its own class id instead of collapsing to the imported outer `foo.bar.A`. + rewriteRun( + kotlin( + """ + package foo.bar + class A { + class B { + class A { + class C + } + } + } + """ + ), + kotlin( + """ + import foo.bar.A + + val x = A.B.A.C() + """, + spec -> spec.afterRecipe(cu -> { + var count = new AtomicInteger(0); + new KotlinIsoVisitor() { + @Override + public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, Integer n) { + String expected = null; + switch (fieldAccess.toString()) { + case "A.B.A.C": + expected = "foo.bar.A$B$A$C"; + break; + case "A.B.A": + expected = "foo.bar.A$B$A"; + break; + case "A.B": + expected = "foo.bar.A$B"; + // The outermost `A` is the imported top-level type, not a nested one. + J.Identifier target = (J.Identifier) fieldAccess.getTarget(); + assertThat(((JavaType.FullyQualified) target.getType()).getFullyQualifiedName()).isEqualTo("foo.bar.A"); + break; + } + if (expected != null) { + assertThat(fieldAccess.getType()).isNotNull(); + assertThat(fieldAccess.getType().toString()).isEqualTo(expected); + assertThat(fieldAccess.getName().getType()).isEqualTo(fieldAccess.getType()); + count.getAndIncrement(); + } + return super.visitFieldAccess(fieldAccess, n); + } + }.visit(cu, 0); + assertThat(count.get()).isEqualTo(3); + }) + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/8237") + @Test + void nestedTypesWithSameSimpleNameFromDifferentOuters() { + // `Status` is nested in both `Alpha` and `Beta`. The package-relative match must select + // each reference's own outer, not confuse the two because the leaf name is identical. + rewriteRun( + kotlin( + """ + package p + + class Alpha { enum class Status { A } } + class Beta { enum class Status { B } } + + val a = Alpha.Status.A + val b = Beta.Status.B + """, + spec -> spec.afterRecipe(cu -> { + var count = new AtomicInteger(0); + new KotlinIsoVisitor() { + @Override + public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, Integer n) { + String expected = null; + switch (fieldAccess.toString()) { + case "Alpha.Status": + expected = "p.Alpha$Status"; + break; + case "Beta.Status": + expected = "p.Beta$Status"; + break; + } + if (expected != null) { + JavaType.Class type = (JavaType.Class) fieldAccess.getType(); + assertThat(type.getFullyQualifiedName()).isEqualTo(expected); + assertThat(type.getOwningClass().getFullyQualifiedName()).isEqualTo(expected.replace("$Status", "")); + count.getAndIncrement(); + } + return super.visitFieldAccess(fieldAccess, n); + } + }.visit(cu, 0); + assertThat(count.get()).isEqualTo(2); + }) + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/8237") + @Test + void nestedFieldAccessViaImportedNestedClass() { + // The imported type is itself nested (`foo.bar.A.B`), so `B.A.C` is only a trailing run of + // the nested names. Each level must still keep its own class id rather than collapsing to + // the outer `foo.bar.A`. + rewriteRun( + kotlin( + """ + package foo.bar + class A { + class B { + class A { + class C + } + } + } + """ + ), + kotlin( + """ + import foo.bar.A.B + + val y = B.A.C() + """, + spec -> spec.afterRecipe(cu -> { + var count = new AtomicInteger(0); + new KotlinIsoVisitor() { + @Override + public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, Integer n) { + String expected = null; + switch (fieldAccess.toString()) { + case "B.A.C": + expected = "foo.bar.A$B$A$C"; + break; + case "B.A": + expected = "foo.bar.A$B$A"; + break; + } + if (expected != null) { + assertThat(fieldAccess.getType()).isNotNull(); + assertThat(fieldAccess.getType().toString()).isEqualTo(expected); + assertThat(fieldAccess.getName().getType()).isEqualTo(fieldAccess.getType()); + count.getAndIncrement(); + } + return super.visitFieldAccess(fieldAccess, n); + } + }.visit(cu, 0); + assertThat(count.get()).isEqualTo(2); + }) + ) + ); + } + @Test void packageFieldAccess() { rewriteRun( diff --git a/rewrite-kotlin/src/test/java/org/openrewrite/kotlin/ShortenFullyQualifiedTypeReferencesKotlinTest.java b/rewrite-kotlin/src/test/java/org/openrewrite/kotlin/ShortenFullyQualifiedTypeReferencesKotlinTest.java new file mode 100644 index 00000000000..ca6ab936abf --- /dev/null +++ b/rewrite-kotlin/src/test/java/org/openrewrite/kotlin/ShortenFullyQualifiedTypeReferencesKotlinTest.java @@ -0,0 +1,67 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.kotlin; + +import org.junit.jupiter.api.Test; +import org.openrewrite.Issue; +import org.openrewrite.java.ShortenFullyQualifiedTypeReferences; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.kotlin.Assertions.kotlin; + +class ShortenFullyQualifiedTypeReferencesKotlinTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ShortenFullyQualifiedTypeReferences()); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/8237") + @Test + void doesNotShortenNestedTypeReachedViaImportedOuter() { + rewriteRun( + spec -> spec.parser(KotlinParser.builder().classpath("apiguardian-api")), + kotlin( + """ + import org.apiguardian.api.API + + @API(status = API.Status.EXPERIMENTAL, since = "5.6") + class Foo + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/8237") + @Test + void doesNotShortenNestedTypeReachedViaAliasedOuter() { + // The outer type is imported under an alias, so `GuardApi.Status` matches neither the + // fully-qualified nor the package-relative name of the nested `Status`. It must still + // keep its own nested type and be left unchanged. + rewriteRun( + spec -> spec.parser(KotlinParser.builder().classpath("apiguardian-api")), + kotlin( + """ + import org.apiguardian.api.API as GuardApi + + @GuardApi(status = GuardApi.Status.EXPERIMENTAL, since = "5.6") + class Foo + """ + ) + ); + } +}