Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rewrite-kotlin/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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")

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.

testImplementation(project(":rewrite-test"))
testRuntimeOnly(project(":rewrite-java-21"))
testRuntimeOnly("org.antlr:antlr4-runtime:4.13.2")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Integer>() {
@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<Integer>() {
@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<Integer>() {
@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<Integer>() {
@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(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright 2024 the original author or authors.
* <p>
* 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
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* 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
"""
)
);
}
}
Loading