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

Always add import for Record classes not from java.lang package, to avoid conflicts on Java 14+ #4787

Merged
merged 3 commits into from
Dec 15, 2024
Merged

Always add import for Record classes not from java.lang package, to avoid conflicts on Java 14+ #4787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
602e0fa
force Import No Java Record
BramliAK Dec 15, 2024
1911398
Apply formatter
timtebeek Dec 15, 2024
223f622
Minor polish
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
37 changes: 35 additions & 2 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 @@ -186,10 +186,11 @@ class A {

@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/540")
@Test
void forceImportNoJavaRecord() {
void forceImportNonJavaLangRecord() {
// 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))),
spec -> spec.recipe(toRecipe(() -> new AddImport<>("com.acme.bank.Record", null, false)))
.parser(JavaParser.fromJavaVersion().dependsOn("package com.acme.bank; public class Record {}")),
//language=java
java(
"""
Expand All @@ -211,6 +212,38 @@ class Foo {
);
}

@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/540")
@Test
void forceImportNonJavaLangRecordFromWildcardImport() {
// 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)))
.parser(JavaParser.fromJavaVersion().dependsOn("package com.acme.bank; public class Record {}")),
//language=java
java(
"""
package com.acme.bank;

import com.acme.bank.*;

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

import 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() {
Expand Down
29 changes: 13 additions & 16 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 @@ -110,8 +110,7 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String
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 &&
if (!"Record".equals(typeName) && cu.getPackageDeclaration() != null &&
packageName.equals(cu.getPackageDeclaration().getExpression().printTrimmed(getCursor()))) {
return cu;
}
Expand All @@ -120,7 +119,7 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String
return cu;
}

if (cu.getImports().stream().anyMatch(i -> {
if (!"Record".equals(typeName) && cu.getImports().stream().anyMatch(i -> {
String ending = i.getQualid().getSimpleName();
if (member == null) {
return !i.isStatic() && i.getPackageName().equals(packageName) &&
Expand All @@ -143,19 +142,17 @@ public AddImport(@Nullable String packageName, String typeName, @Nullable String

List<JRightPadded<J.Import>> imports = new ArrayList<>(cu.getPadding().getImports());

if (imports.isEmpty() && !cu.getClasses().isEmpty()) {
if (cu.getPackageDeclaration() == null) {
// leave javadocs on the class and move other comments up to the import
// (which could include license headers and the like)
Space firstClassPrefix = cu.getClasses().get(0).getPrefix();
importToAdd = importToAdd.withPrefix(firstClassPrefix
.withComments(ListUtils.map(firstClassPrefix.getComments(), comment -> comment instanceof Javadoc ? null : comment))
.withWhitespace(""));

cu = cu.withClasses(ListUtils.mapFirst(cu.getClasses(), clazz ->
clazz.withComments(ListUtils.map(clazz.getComments(), comment -> comment instanceof Javadoc ? comment : null))
));
}
if (imports.isEmpty() && !cu.getClasses().isEmpty() && cu.getPackageDeclaration() == null) {
// leave javadocs on the class and move other comments up to the import
// (which could include license headers and the like)
Space firstClassPrefix = cu.getClasses().get(0).getPrefix();
importToAdd = importToAdd.withPrefix(firstClassPrefix
.withComments(ListUtils.map(firstClassPrefix.getComments(), comment -> comment instanceof Javadoc ? null : comment))
.withWhitespace(""));

cu = cu.withClasses(ListUtils.mapFirst(cu.getClasses(), clazz ->
clazz.withComments(ListUtils.map(clazz.getComments(), comment -> comment instanceof Javadoc ? comment : null))
));
}

ImportLayoutStyle layoutStyle = Optional.ofNullable(((SourceFile) cu).getStyle(ImportLayoutStyle.class))
Expand Down
Loading