Skip to content

Commit

Permalink
refactor: OpenRewrite best practices (#4243)
Browse files Browse the repository at this point in the history 
Use this link to re-run the recipe: https://app.moderne.io/recipes/org.openrewrite.recipes.OpenRewriteBestPractices?organizationId=T3BlblJld3JpdGU%3D

Co-authored-by: Moderne <[email protected]>
  • Loading branch information
@timtebeek @TeamModerne
timtebeek and TeamModerne authored Jun 10, 2024
1 parent 9bcc84c commit ddc560f
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 71 deletions.
8 changes: 4 additions & 4 deletions rewrite-gradle/src/main/java/org/openrewrite/gradle/UpgradeTransitiveDependencyVersion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
private static class RemoveVersionVisitor extends GroovyIsoVisitor<ExecutionContext> {

@Override
public J.Return visitReturn(J.Return _return, ExecutionContext executionContext) {
J.Return r = super.visitReturn(_return, executionContext);
public J.Return visitReturn(J.Return _return, ExecutionContext ctx) {
J.Return r = super.visitReturn(_return, ctx);
if(r.getExpression() == null) {
//noinspection DataFlowIssue
return null;
Expand All @@ -493,8 +493,8 @@ public J.Return visitReturn(J.Return _return, ExecutionContext executionContext)
}

@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
J.MethodInvocation m = super.visitMethodInvocation(method, executionContext);
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation m = super.visitMethodInvocation(method, ctx);
if("version".equals(m.getSimpleName()) && m.getArguments().size() == 1 && m.getArguments().get(0) instanceof J.Lambda) {
//noinspection DataFlowIssue
return null;
Expand Down
32 changes: 17 additions & 15 deletions rewrite-gradle/src/test/java/org/openrewrite/gradle/RemoveEnableFeaturePreviewTest.java
View file
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.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

Expand All @@ -29,29 +30,30 @@ public void defaults(RecipeSpec spec) {
spec.recipe(new RemoveEnableFeaturePreview("ONE_LOCKFILE_PER_PROJECT"));
}

@DocumentExample
@Test
void singleQuotes() {
//language=gradle
rewriteRun(
settingsGradle(
"""
pluginManagement {
repositories {
gradlePluginPortal()
}
}
rootProject.name = 'merge-service'
enableFeaturePreview('ONE_LOCKFILE_PER_PROJECT')
pluginManagement {
repositories {
gradlePluginPortal()
}
}
rootProject.name = 'merge-service'
enableFeaturePreview('ONE_LOCKFILE_PER_PROJECT')
""",
"""
pluginManagement {
repositories {
gradlePluginPortal()
}
}
rootProject.name = 'merge-service'
pluginManagement {
repositories {
gradlePluginPortal()
}
}
rootProject.name = 'merge-service'
"""
)
);
Expand Down
24 changes: 12 additions & 12 deletions rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,10 +444,10 @@ class A {
}
""",
"""
import org.openrewrite.test.other.Test;
class A {
Test test = null;
}
import org.openrewrite.test.other.Test;
class A {
Test test = null;
}
""",
spec -> spec.afterRecipe(cu -> {
assertThat(cu.findType("org.openrewrite.other.Test")).isEmpty();
Expand Down Expand Up @@ -502,10 +502,10 @@ class A {
}
""",
"""
import org.openrewrite.test.other.Test;
class A {
Test test = null;
}
import org.openrewrite.test.other.Test;
class A {
Test test = null;
}
""",
spec -> spec.afterRecipe(cu -> {
assertThat(cu.findType("org.openrewrite.other.Test")).isEmpty();
Expand Down Expand Up @@ -560,10 +560,10 @@ class A {
}
""",
"""
import org.openrewrite.test.other.Test;
class A {
Test test = null;
}
import org.openrewrite.test.other.Test;
class A {
Test test = null;
}
""",
spec -> spec.afterRecipe(cu -> {
assertThat(cu.findType("org.openrewrite.other.Test")).isEmpty();
Expand Down
55 changes: 28 additions & 27 deletions rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ void assignmentWithinIfPredicate() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() {
@Override
public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext p) {
public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) {
if ((assignment.getAssignment() instanceof J.Literal) &&
((J.Literal) assignment.getAssignment()).getValue().equals(1)) {
return JavaTemplate.builder("value = 0")
Expand Down Expand Up @@ -77,7 +77,7 @@ void lambdaIsNewClass() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() {
@Override
public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext p) {
public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) {
var a = assignment;
if (a.getAssignment() instanceof J.MethodInvocation) {
J.MethodInvocation mi = (J.MethodInvocation) a.getAssignment();
Expand Down Expand Up @@ -120,8 +120,8 @@ void replaceForEachControlVariableType() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() {
@Override
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext p) {
var mv = super.visitVariableDeclarations(multiVariable, p);
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) {
var mv = super.visitVariableDeclarations(multiVariable, ctx);
if (mv.getVariables().get(0).getInitializer() == null && TypeUtils.isOfType(mv.getTypeExpression()
.getType(), JavaType.Primitive.String)) {
mv = JavaTemplate.apply("Object #{}", getCursor(),
Expand Down Expand Up @@ -158,8 +158,8 @@ void replaceForEachControlIterable() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() {
@Override
public J.ForEachLoop.Control visitForEachControl(J.ForEachLoop.Control control, ExecutionContext executionContext) {
control = super.visitForEachControl(control, executionContext);
public J.ForEachLoop.Control visitForEachControl(J.ForEachLoop.Control control, ExecutionContext ctx) {
control = super.visitForEachControl(control, ctx);
Expression iterable = control.getIterable();
if (!TypeUtils.isOfClassType(iterable.getType(), "java.lang.String")) {
return control;
Expand Down Expand Up @@ -192,8 +192,8 @@ void replaceBinaryExpression() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() {
@Override
public J.Binary visitBinary(J.Binary binary, ExecutionContext executionContext) {
binary = super.visitBinary(binary, executionContext);
public J.Binary visitBinary(J.Binary binary, ExecutionContext ctx) {
binary = super.visitBinary(binary, ctx);
if (binary.getLeft() instanceof J.Literal lit && lit.getValue().equals(42)) {
return JavaTemplate.apply("43", getCursor(), lit.getCoordinates().replace());
}
Expand Down Expand Up @@ -225,7 +225,7 @@ void replaceForEachControlIterator() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitNewClass(J.NewClass newClass, ExecutionContext p) {
public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) {
if (TypeUtils.isOfClassType(newClass.getType(), "java.util.ArrayList")) {
return JavaTemplate.builder("Collections.emptyList()")
.imports("java.util.Collections").build()
Expand Down Expand Up @@ -313,7 +313,7 @@ void replaceAnonymousClassObject() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
if (method.getSimpleName().equals("asList")) {
maybeAddImport("java.util.Collections");
maybeRemoveImport("java.util.Arrays");
Expand Down Expand Up @@ -361,7 +361,7 @@ void replaceGenericTypedObject() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
if (method.getSimpleName().equals("asList")) {
maybeAddImport("java.util.Collections");
maybeRemoveImport("java.util.Arrays");
Expand Down Expand Up @@ -411,7 +411,7 @@ void replaceParameterizedTypeObject() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
if (method.getSimpleName().equals("asList")) {
maybeAddImport("java.util.Collections");
maybeRemoveImport("java.util.Arrays");
Expand Down Expand Up @@ -465,7 +465,7 @@ void templatingWhileLoopCondition() {
rewriteRun(
spec -> spec.expectedCyclesThatMakeChanges(2).recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitBinary(J.Binary binary, ExecutionContext p) {
public J visitBinary(J.Binary binary, ExecutionContext ctx) {
if (binary.getLeft() instanceof J.MethodInvocation) {
J.MethodInvocation mi = (J.MethodInvocation) binary.getLeft();
return JavaTemplate.builder("!#{any(java.util.List)}.isEmpty()")
Expand Down Expand Up @@ -508,8 +508,8 @@ void javaTemplateControlsSemiColons() {
.imports("java.math.RoundingMode").build();

@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext p) {
J.MethodInvocation mi = (J.MethodInvocation) super.visitMethodInvocation(method, p);
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation mi = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
if (bigDecimalSetScale.matches(mi)) {
mi = twoArgScale.apply(updateCursor(mi), mi.getCoordinates().replaceArguments(),
mi.getArguments().get(0), "RoundingMode.HALF_UP");
Expand Down Expand Up @@ -550,7 +550,7 @@ void replaceExpressionWithAnotherExpression() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitUnary(J.Unary unary, ExecutionContext p) {
public J visitUnary(J.Unary unary, ExecutionContext ctx) {
return JavaTemplate.builder("#{any()}++")
.contextSensitive()
.build().apply(
Expand Down Expand Up @@ -585,7 +585,7 @@ void replaceMemberReference() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitMemberReference(J.MemberReference memberRef, ExecutionContext executionContext) {
public J visitMemberReference(J.MemberReference memberRef, ExecutionContext ctx) {
return JavaTemplate.builder("() -> new ArrayList<>(1)")
.contextSensitive()
.imports("java.util.ArrayList")
Expand Down Expand Up @@ -624,12 +624,12 @@ void replaceFieldAccessWithMethodInvocation() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext p) {
return method.withBody((J.Block) visit(method.getBody(), p));
public J visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
return method.withBody((J.Block) visit(method.getBody(), ctx));
}

@Override
public J visitFieldAccess(J.FieldAccess fa, ExecutionContext p) {
public J visitFieldAccess(J.FieldAccess fa, ExecutionContext ctx) {
if (fa.getSimpleName().equals("f")) {
return JavaTemplate.apply("#{any(java.io.File)}.getCanonicalFile().toPath()",
getCursor(), fa.getCoordinates().replace(), fa);
Expand Down Expand Up @@ -705,7 +705,7 @@ void innerEnumWithStaticMethod() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitNewClass(J.NewClass newClass, ExecutionContext p) {
public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) {
if (newClass.getArguments().get(0) instanceof J.Empty) {
return newClass;
}
Expand Down Expand Up @@ -774,7 +774,7 @@ void arrayInitializer() {
final MethodMatcher mm = new MethodMatcher("abc.ArrayHelper of(..)");

@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext p) {
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
if (mm.matches(method)) {
return JavaTemplate.builder("Arrays.asList(#{any(java.lang.Integer)}, #{any(java.lang.Integer)}, #{any(java.lang.Integer)})")
.imports("java.util.Arrays")
Expand Down Expand Up @@ -828,8 +828,8 @@ void multiDimensionalArrayInitializer() {
final MethodMatcher mm = new MethodMatcher("java.util.stream.IntStream sum()");

@Override
public J visitNewClass(J.NewClass newClass, ExecutionContext p) {
J.NewClass nc = (J.NewClass) super.visitNewClass(newClass, p);
public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) {
J.NewClass nc = (J.NewClass) super.visitNewClass(newClass, ctx);
return JavaTemplate.apply("Integer.valueOf(#{any(java.lang.Integer)}",
getCursor(), nc.getCoordinates().replace(), nc.getArguments().get(0));
}
Expand All @@ -855,7 +855,7 @@ void dontDropTheAssert() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitBinary(J.Binary binary, ExecutionContext p) {
public J visitBinary(J.Binary binary, ExecutionContext ctx) {
J isEmpty = JavaTemplate.apply("!#{any(java.util.Collection)}.isEmpty()", getCursor(),
binary.getCoordinates().replace(), ((J.MethodInvocation) binary.getLeft()).getSelect());
return isEmpty.withPrefix(binary.getPrefix());
Expand Down Expand Up @@ -889,7 +889,7 @@ void nestedEnums() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitBinary(J.Binary binary, ExecutionContext p) {
public J visitBinary(J.Binary binary, ExecutionContext ctx) {
return JavaTemplate.builder("\"ab\"").contextSensitive().build()
.apply(getCursor(), binary.getCoordinates().replace());
}
Expand Down Expand Up @@ -1199,7 +1199,8 @@ interface Test {
void finalMethodParameter() {
rewriteRun(
spec -> spec.recipe(new ReplaceAnnotation("@org.jetbrains.annotations.NotNull", "@lombok.NonNull", null)),
java("""
java(
"""
import org.jetbrains.annotations.NotNull;
class A {
Expand Down
4 changes: 2 additions & 2 deletions ...te-java-test/src/test/java/org/openrewrite/java/ReplaceStringLiteralWithConstantTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,10 @@ void missingFieldNoError() {
rewriteRun(
spec -> spec.recipe(RewriteTest.toRecipe(() -> new JavaVisitor<>(){
@Override
public @Nullable J visit(@Nullable Tree tree, ExecutionContext executionContext) {
public @Nullable J visit(@Nullable Tree tree, ExecutionContext ctx) {
// Circumvent validation to match use in rewrite-spring's ReplaceStringLiteralsWithMediaTypeConstants
doAfterVisit(new ReplaceStringLiteralWithConstant(EXAMPLE_STRING_FQN + "_xyz").getVisitor());
return super.visit(tree, executionContext);
return super.visit(tree, ctx);
}
})),
java(
Expand Down
7 changes: 4 additions & 3 deletions rewrite-java/src/main/java/org/openrewrite/java/ReplaceAnnotation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ public class ReplaceAnnotation extends Recipe {

@Option(displayName = "Classpath resource",
description = "If the annotation's type is defined by a jar within the META-INF/rewrite/classpath directory provide its name here " +
"so that it can be loaded. " +
"When this parameter is not passed the runtime classpath of the recipe is provided to the parser producing the new annotation. " +
"This is necessary when the annotation is not on the runtime classpath of the recipe and isn't in the Java standard library.",
"so that it can be loaded. " +
"When this parameter is not passed the runtime classpath of the recipe is provided to the parser producing the new annotation. " +
"This is necessary when the annotation is not on the runtime classpath of the recipe and isn't in the Java standard library.",
example = "annotations",
required = false)
@Nullable
String classpathResourceName;
Expand Down
9 changes: 6 additions & 3 deletions rewrite-maven/src/test/java/org/openrewrite/maven/AddPropertyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ void ifRemoteParentIsDefined() {
<artifactId>my-parent</artifactId>
<version>1</version>
</project>
""","""
""",
"""
<project>
<parent>
<groupId>org.springframework.boot</groupId>
Expand Down Expand Up @@ -214,7 +215,8 @@ void ifRemoteParentIsDefined_2() {
<artifactId>my-parent</artifactId>
<version>1</version>
</project>
""","""
""",
"""
<project>
<parent>
<groupId>org.springframework.boot</groupId>
Expand Down Expand Up @@ -251,7 +253,8 @@ void ifRemoteParentIsDefined_3() {
<artifactId>my-parent</artifactId>
<version>1</version>
</project>
""","""
""",
"""
<project>
<parent>
<groupId>org.springframework.boot</groupId>
Expand Down
Loading

0 comments on commit ddc560f

Please sign in to comment.