From f60de55b8857b073d7f389fca23b3ade04e2f1c7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 31 Oct 2023 16:17:24 +0100 Subject: [PATCH] Polish LombokValueToRecord --- .../migrate/lombok/LombokValueToRecord.java | 39 ++++-- .../lombok/LombokValueToRecordTest.java | 118 ++++++++++++++++++ 2 files changed, 148 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lombok/LombokValueToRecord.java b/src/main/java/org/openrewrite/java/migrate/lombok/LombokValueToRecord.java index 1bb06a72bd..3530ad4e1f 100644 --- a/src/main/java/org/openrewrite/java/migrate/lombok/LombokValueToRecord.java +++ b/src/main/java/org/openrewrite/java/migrate/lombok/LombokValueToRecord.java @@ -19,6 +19,7 @@ import lombok.RequiredArgsConstructor; import lombok.Value; import org.openrewrite.*; +import org.openrewrite.internal.ListUtils; import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.AnnotationMatcher; import org.openrewrite.java.JavaIsoVisitor; @@ -26,10 +27,7 @@ import org.openrewrite.java.RemoveAnnotationVisitor; import org.openrewrite.java.search.UsesJavaVersion; import org.openrewrite.java.search.UsesType; -import org.openrewrite.java.tree.Expression; -import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.JavaType; -import org.openrewrite.java.tree.Statement; +import org.openrewrite.java.tree.*; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -42,7 +40,7 @@ @EqualsAndHashCode(callSuper = false) public class LombokValueToRecord extends ScanningRecipe>> { - private static final AnnotationMatcher LOMBOK_VALUE_MATCHER = new AnnotationMatcher("@lombok.Value"); + private static final AnnotationMatcher LOMBOK_VALUE_MATCHER = new AnnotationMatcher("@lombok.Value()"); @Option(displayName = "Add a `toString()` implementation matching Lombok", description = "When set the `toString` format from Lombok is used in the migrated record.", @@ -111,9 +109,11 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex private boolean isRelevantClass(J.ClassDeclaration classDeclaration) { return classDeclaration.getType() != null && !J.ClassDeclaration.Kind.Type.Record.equals(classDeclaration.getKind()) - && classDeclaration.getAllAnnotations().stream().allMatch(LOMBOK_VALUE_MATCHER::matches) + && classDeclaration.getAllAnnotations().stream() + .allMatch(ann -> LOMBOK_VALUE_MATCHER.matches(ann) && (ann.getArguments() == null || ann.getArguments().isEmpty())) && !hasGenericTypeParameter(classDeclaration) - && classDeclaration.getBody().getStatements().stream().allMatch(this::isRecordCompatibleField); + && classDeclaration.getBody().getStatements().stream().allMatch(this::isRecordCompatibleField) + && !hasIncompatibleModifier(classDeclaration); } private boolean hasGenericTypeParameter(J.ClassDeclaration classDeclaration) { @@ -121,6 +121,19 @@ private boolean hasGenericTypeParameter(J.ClassDeclaration classDeclaration) { return typeParameters != null && !typeParameters.isEmpty(); } + private boolean hasIncompatibleModifier(J.ClassDeclaration classDeclaration) { + // Inner classes need to be static + if (getCursor().getParent() != null) { + Object parentValue = getCursor().getParent().getValue(); + if (parentValue instanceof J.ClassDeclaration || (parentValue instanceof JRightPadded && ((JRightPadded) parentValue).getElement() instanceof J.ClassDeclaration)) { + if (classDeclaration.getModifiers().stream().noneMatch(mod -> mod.getType() == J.Modifier.Type.Static)) { + return true; + } + } + } + return false; + } + private boolean isRecordCompatibleField(Statement statement) { if (!(statement instanceof J.VariableDeclarations)) { return false; @@ -266,9 +279,18 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration cd, Execution List bodyStatements = new ArrayList<>(classDeclaration.getBody().getStatements()); bodyStatements.removeAll(memberVariables); - doAfterVisit(new RemoveAnnotationVisitor(LOMBOK_VALUE_MATCHER)); + classDeclaration = new RemoveAnnotationVisitor(LOMBOK_VALUE_MATCHER).visitClassDeclaration(classDeclaration, ctx); + maybeRemoveImport("lombok.Value"); + classDeclaration = classDeclaration .withKind(J.ClassDeclaration.Kind.Type.Record) + .withModifiers(ListUtils.map(classDeclaration.getModifiers(), modifier -> { + J.Modifier.Type type = modifier.getType(); + if (type == J.Modifier.Type.Static || type == J.Modifier.Type.Final) { + return null; + } + return modifier; + })) .withType(buildRecordType(classDeclaration)) .withBody(classDeclaration.getBody() .withStatements(bodyStatements) @@ -299,4 +321,3 @@ private static Set getMemberVariableNames(List m .collect(LinkedHashSet::new, LinkedHashSet::add, LinkedHashSet::addAll); } } - diff --git a/src/test/java/org/openrewrite/java/migrate/lombok/LombokValueToRecordTest.java b/src/test/java/org/openrewrite/java/migrate/lombok/LombokValueToRecordTest.java index 4306bc86cb..fe8e3f6405 100644 --- a/src/test/java/org/openrewrite/java/migrate/lombok/LombokValueToRecordTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lombok/LombokValueToRecordTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; import org.openrewrite.Issue; import org.openrewrite.java.JavaParser; import org.openrewrite.test.RecipeSpec; @@ -73,6 +74,7 @@ public String toString() { } @Test + @DocumentExample @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/141") void convertOnlyValueAnnotatedClassWithoutDefaultValuesToRecord() { //language=java @@ -135,6 +137,80 @@ public String getRecordValue() { ); } + @Test + void onlyRemoveAnnotationFromRecords() { + //language=java + rewriteRun( + s -> s.typeValidationOptions(TypeValidation.none()), + java( + """ + package example; + + import lombok.ToString; + import lombok.Value; + + @Value + public class A { + String test; + } + + @Value + @ToString + public class B { + Strign test; + } + """, + """ + package example; + + import lombok.ToString; + import lombok.Value; + + public record A( + String test) { + } + + @Value + @ToString + public class B { + Strign test; + } + """ + ) + ); + } + + @Test + void innerRecordsNotStatic() { + //language=java + rewriteRun( + s -> s.typeValidationOptions(TypeValidation.none()), + java( + """ + package example; + + import lombok.Value; + + public class A { + @Value + static class B { + String test; + } + } + """, + """ + package example; + + public class A { + record B( + String test) { + } + } + """ + ) + ); + } + @Nested class Unchanged { @Test @@ -286,5 +362,47 @@ public class A { ) ); } + + @Test + void nonStaticInnerClass() { + //language=java + rewriteRun( + s -> s.typeValidationOptions(TypeValidation.none()), + java( + """ + package example; + + import lombok.Value; + + public class A { + @Value + class B { + String test; + } + } + """ + ) + ); + } + + @Test + void staticConstructor() { + //language=java + rewriteRun( + s -> s.typeValidationOptions(TypeValidation.none()), + java( + """ + package example; + + import lombok.Value; + + @Value(staticConstructor = "of") + public class A { + String test; + } + """ + ) + ); + } } }