Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add explicit imports to avoid conflicts with classes added to java.lang, like record #4785

Merged
merged 4 commits into from
Dec 15, 2024
Merged

Add explicit imports to avoid conflicts with classes added to java.lang, like record #4785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce021da
rewrite-migrate-java issue 540 : Add explicit imports to avoid confli…
BramliAK Dec 15, 2024
8972196
change test name
BramliAK Dec 15, 2024
de84ed1
Split conditional and add clarifying comments on Record handling
timtebeek Dec 15, 2024
4cb70f5
Add issue links to clarify the reason these tests were added
timtebeek Dec 15, 2024
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
45 changes: 45 additions & 0 deletions rewrite-java-test/src/test/java/org/openrewrite/java/AddImportTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,51 @@ class A {
);
}

@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/540")
@Test
void forceImportNoJavaRecord() {
// Add import for a class named `Record`, even within the same package, to avoid conflicts with java.lang.Record
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new AddImport<>("com.acme.bank.Record", null, false))),
//language=java
java(
"""
package com.acme.bank;

class Foo {
}
""",
"""
package com.acme.bank;

import com.acme.bank.Record;

class Foo {
}
""",
spec -> spec.markers(javaVersion(11))
)
);
}

@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/540")
@Test
void notForceImportJavaRecord() {
// Do not add import for java.lang.Record by default
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new AddImport<>("java.lang.Record", null, false))),
//language=java
java(
"""
package com.acme.bank;

class Foo {
}
""",
spec -> spec.markers(javaVersion(11))
)
);
}
@Test
void dontImportJavaLang() {
rewriteRun(
Expand Down
11 changes: 8 additions & 3 deletions rewrite-java/src/main/java/org/openrewrite/java/AddImport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,14 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String
return cu;
}

// No need to add imports if the class to import is in java.lang, or if the classes are within the same package
if (("java.lang".equals(packageName) && StringUtils.isBlank(member)) || (cu.getPackageDeclaration() != null &&
packageName.equals(cu.getPackageDeclaration().getExpression().printTrimmed(getCursor())))) {
// No need to add imports if the class to import is in java.lang
if ("java.lang".equals(packageName) && StringUtils.isBlank(member)) {
return cu;
}
// Nor if the classes are within the same package
if (!"Record".equals(typeName) && // Record's late addition to `java.lang` might conflict with user class
cu.getPackageDeclaration() != null &&
packageName.equals(cu.getPackageDeclaration().getExpression().printTrimmed(getCursor()))) {
return cu;
}

Expand Down