From c27510f273c12bc778fb056d1b0b86a6bc9686ac Mon Sep 17 00:00:00 2001 From: k3v1n Date: Sun, 15 Oct 2023 20:30:58 +0200 Subject: [PATCH] #141 extend implementation with further acceptance criterias --- .../migrate/lombok/LombokValueToRecord.java | 119 +++++++++++++++++- .../lombok/LombokValueToRecordTest.java | 96 +++++++++++++- 2 files changed, 209 insertions(+), 6 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 04536aa90f..3c3e398a14 100644 --- a/src/main/java/org/openrewrite/java/migrate/lombok/LombokValueToRecord.java +++ b/src/main/java/org/openrewrite/java/migrate/lombok/LombokValueToRecord.java @@ -15,14 +15,18 @@ */ package org.openrewrite.java.migrate.lombok; +import lombok.EqualsAndHashCode; +import lombok.Value; import org.openrewrite.ExecutionContext; import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.AnnotationMatcher; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.RemoveAnnotationVisitor; import org.openrewrite.java.search.UsesJavaVersion; +import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.Statement; @@ -31,7 +35,9 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.regex.Pattern; @@ -39,6 +45,8 @@ import static java.util.Objects.requireNonNull; +@Value +@EqualsAndHashCode(callSuper = false) public class LombokValueToRecord extends Recipe { @Override public String getDisplayName() { @@ -47,7 +55,7 @@ public String getDisplayName() { @Override public String getDescription() { - return "Convert Lombok Value annotated classes to standard java record classes in java 11 or higher."; + return "Convert Lombok Value annotated classes to standard Java Record classes in Java 17 or higher."; } @Override @@ -60,6 +68,16 @@ public Duration getEstimatedEffortPerOccurrence() { return Duration.ofMinutes(5); } + @Override + public boolean causesAnotherCycle() { + return true; + } + + @Override + public int maxCycles() { + return 2; + } + @Override public TreeVisitor getVisitor() { final TreeVisitor check = Preconditions.and( @@ -71,13 +89,74 @@ public TreeVisitor getVisitor() { private static class LombokValueToRecordVisitor extends JavaIsoVisitor { + private static final Map> RECORD_TYPE_TO_MEMBERS = new HashMap<>(); + + private static final String STANDARD_GETTER_START = "get"; + + @Override + public J.MethodInvocation visitMethodInvocation( + final J.MethodInvocation method, + final ExecutionContext executionContext + ) { + final J.MethodInvocation methodInvocation = super.visitMethodInvocation(method, executionContext); + + if (executionContext.getCycle() <= 1 || !isMethodInvocationOnRecordTypeClassMember(methodInvocation)) { + return methodInvocation; + } + + final J.Identifier methodName = methodInvocation.getName(); + return methodInvocation + .withName(methodName + .withSimpleName(getterMethodNameToFluentMethodName(methodName.getSimpleName())) + ); + } + + private static boolean isMethodInvocationOnRecordTypeClassMember(final J.MethodInvocation methodInvocation) { + final Expression expression = methodInvocation.getSelect(); + if (!isClassExpression(expression)) { + return false; + } + + final JavaType.Class classType = requireNonNull((JavaType.Class) expression.getType(), + "Class type must not be null"); + + final String methodName = methodInvocation.getName().getSimpleName(); + + return RECORD_TYPE_TO_MEMBERS.containsKey(classType.getFullyQualifiedName()) + && methodName.startsWith(STANDARD_GETTER_START) + && RECORD_TYPE_TO_MEMBERS + .get(classType.getFullyQualifiedName()) + .contains(getterMethodNameToFluentMethodName(methodName)); + } + + private static boolean isClassExpression(final @Nullable Expression expression) { + return expression != null && (expression.getType() instanceof JavaType.Class); + } + + private static String getterMethodNameToFluentMethodName(final String methodName) { + final StringBuilder fluentMethodName = new StringBuilder( + methodName.replace(STANDARD_GETTER_START, "")); + + if (fluentMethodName.length() == 0) { + return ""; + } + + final char firstMemberChar = fluentMethodName.charAt(0); + fluentMethodName.setCharAt(0, Character.toLowerCase(firstMemberChar)); + + return fluentMethodName.toString(); + } + @Override public J.ClassDeclaration visitClassDeclaration(final J.ClassDeclaration cd, final ExecutionContext ctx) { J.ClassDeclaration classDeclaration = super.visitClassDeclaration(cd, ctx); + if (isRecord(classDeclaration) - || hasExplicitConstructor(classDeclaration) || !hasOnlyLombokValueAnnotation(classDeclaration) + || hasGenericTypeParameter(classDeclaration) + || hasExplicitMethods(classDeclaration) + || hasExplicitConstructor(classDeclaration) ) { return classDeclaration; } @@ -99,9 +178,45 @@ public J.ClassDeclaration visitClassDeclaration(final J.ClassDeclaration cd, fin ) .withPrimaryConstructor(mapToConstructorArguments(memberVariables)); + addToRecordTypeState(classDeclaration, memberVariables); + return maybeAutoFormat(cd, classDeclaration, ctx); } + private static void addToRecordTypeState(final J.ClassDeclaration classDeclaration, + final List memberVariables + ) { + final JavaType.FullyQualified classType = requireNonNull(classDeclaration.getType(), + "Class type must not be null"); + + RECORD_TYPE_TO_MEMBERS.putIfAbsent( + classType.getFullyQualifiedName(), + getMemberVariableNames(memberVariables)); + } + + private static boolean hasExplicitMethods(final J.ClassDeclaration classDeclaration) { + return classDeclaration + .getBody() + .getStatements() + .stream() + .anyMatch(J.MethodDeclaration.class::isInstance); + } + + private static Set getMemberVariableNames(final List memberVariables) { + return memberVariables + .stream() + .map(J.VariableDeclarations::getVariables) + .flatMap(List::stream) + .map(J.VariableDeclarations.NamedVariable::getSimpleName) + .collect(Collectors.toSet()); + } + + private static boolean hasGenericTypeParameter(final J.ClassDeclaration classDeclaration) { + final List typeParameters = classDeclaration.getTypeParameters(); + + return typeParameters != null && !typeParameters.isEmpty(); + } + private static JavaType.Class buildRecordType(final J.ClassDeclaration cd) { requireNonNull(cd.getType(), "Class type must not be null"); final String className = requireNonNull(cd.getType().getFullyQualifiedName(), 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 9fa66931f1..37bec76f88 100644 --- a/src/test/java/org/openrewrite/java/migrate/lombok/LombokValueToRecordTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lombok/LombokValueToRecordTest.java @@ -19,6 +19,7 @@ import org.openrewrite.java.JavaParser; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import org.openrewrite.test.TypeValidation; import static org.openrewrite.java.Assertions.java; import static org.openrewrite.java.Assertions.version; @@ -35,23 +36,66 @@ public void defaults(RecipeSpec spec) { void convertOnlyValueAnnotatedClassWithoutDefaultValuesToRecord() { //language=java rewriteRun( + // TODO: find a way to please type validation so this workaround is not required anymore + s -> s.typeValidationOptions(TypeValidation.none()), version( java( """ + package example; + import lombok.Value; - + @Value public class A { String test; } """, """ + package example; + public record A( String test) { } """ ), 17 + ), + version( + java( + """ + package example; + + public class UserOfA { + + private final A record; + + public UserOfA() { + this.record = new A("some value"); + } + + public String getRecordValue() { + return record.getTest(); + } + } + """, + """ + package example; + + public class UserOfA { + + private final A record; + + public UserOfA() { + this.record = new A("some value"); + } + + public String getRecordValue() { + return record.test(); + } + } + """ + ), + 17 ) ); } @@ -64,7 +108,7 @@ void classWithExplicitConstructorIsUnchanged() { java( """ import lombok.Value; - + @Value public class A { String test; @@ -80,6 +124,50 @@ public A() { ); } + @Test + void classWithExplicitMethodsIsUnchanged() { + //language=java + rewriteRun( + version( + java( + """ + import lombok.Value; + + @Value + public class A { + String test; + + public String getTest() { + return test; + } + } + """ + ), + 17 + ) + ); + } + + @Test + void genericClassIsUnchanged() { + //language=java + rewriteRun( + version( + java( + """ + import lombok.Value; + + @Value + public class A { + T test; + } + """ + ), + 17 + ) + ); + } + @Test void nonJava17ClassIsUnchanged() { //language=java @@ -88,7 +176,7 @@ void nonJava17ClassIsUnchanged() { java( """ import lombok.Value; - + @Value public class A { String test; @@ -109,7 +197,7 @@ void classWithMultipleLombokAnnotationsIsUnchanged() { """ import lombok.Value; import lombok.experimental.Accessors; - + @Value @Accessors(fluent = true) public class A {