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.

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

Polish LombokValueToRecord #334

Merged
merged 1 commit into from
Oct 31, 2023
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,15 @@
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;
import org.openrewrite.java.JavaTemplate;
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;
Expand All @@ -42,7 +40,7 @@
@EqualsAndHashCode(callSuper = false)
public class LombokValueToRecord extends ScanningRecipe<Map<String, Set<String>>> {

private static final AnnotationMatcher LOMBOK_VALUE_MATCHER = new AnnotationMatcher("@lombok.Value");
private static final AnnotationMatcher LOMBOK_VALUE_MATCHER = new AnnotationMatcher("@lombok.Value()");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an attempt to have the annotation matcher only match annotations without arguments; that didn't work, but I think it should.


@Option(displayName = "Add a `toString()` implementation matching Lombok",
description = "When set the `toString` format from Lombok is used in the migrated record.",
Expand Down Expand Up @@ -111,16 +109,31 @@ 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) {
List<J.TypeParameter> typeParameters = classDeclaration.getTypeParameters();
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;
Expand Down Expand Up @@ -266,9 +279,18 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration cd, Execution
List<Statement> 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)
Expand Down Expand Up @@ -299,4 +321,3 @@ private static Set<String> getMemberVariableNames(List<J.VariableDeclarations> m
.collect(LinkedHashSet::new, LinkedHashSet::add, LinkedHashSet::addAll);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -73,6 +74,7 @@ public String toString() {
}

@Test
@DocumentExample
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/141")
void convertOnlyValueAnnotatedClassWithoutDefaultValuesToRecord() {
//language=java
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
"""
)
);
}
}
}
Loading