From 20ef4ccd05a4f7ddc39a6758c50900a54c6b32aa Mon Sep 17 00:00:00 2001 From: nbruno Date: Mon, 27 Feb 2023 09:33:04 -0500 Subject: [PATCH] Add recipe to consolidate @ExtendWith and @ContextConfiguration (#297) * Add recipe to replace @ExtendWith and @ContextConfiguration with @SpringJUnitConfig (#296) * Override getSingleSourceApplicableTest for UsesType checks Also apply default formatter for consistency * Adopt String.formatted instead of String concatenation * Consistently use replace with instead of into * Adopt Applicability.and(..) to require both annotations * Move and temporarily disable inclusion in update chain --------- Co-authored-by: Tim te Beek --- ...laceExtendWithAndContextConfiguration.java | 145 ++++++++++ .../boot2/UnnecessarySpringExtension.java | 3 +- .../META-INF/rewrite/spring-boot-24.yml | 2 + .../SpringBoot2JUnit4to5MigrationTest.java | 63 +++- ...ExtendWithAndContextConfigurationTest.java | 272 ++++++++++++++++++ 5 files changed, 477 insertions(+), 8 deletions(-) create mode 100644 src/main/java/org/openrewrite/java/spring/boot2/ReplaceExtendWithAndContextConfiguration.java create mode 100644 src/testWithSpringBoot_2_4/java/org/openrewrite/java/spring/boot2/ReplaceExtendWithAndContextConfigurationTest.java diff --git a/src/main/java/org/openrewrite/java/spring/boot2/ReplaceExtendWithAndContextConfiguration.java b/src/main/java/org/openrewrite/java/spring/boot2/ReplaceExtendWithAndContextConfiguration.java new file mode 100644 index 000000000..b5bfe52c1 --- /dev/null +++ b/src/main/java/org/openrewrite/java/spring/boot2/ReplaceExtendWithAndContextConfiguration.java @@ -0,0 +1,145 @@ +/* + * Copyright 2023 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.spring.boot2; + +import org.openrewrite.Applicability; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.AnnotationMatcher; +import org.openrewrite.java.ChangeType; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; + +import java.time.Duration; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CopyOnWriteArrayList; + +public class ReplaceExtendWithAndContextConfiguration extends Recipe { + private static final String FQN_EXTEND_WITH = "org.junit.jupiter.api.extension.ExtendWith"; + private static final String FQN_CONTEXT_CONFIGURATION = "org.springframework.test.context.ContextConfiguration"; + private static final String FQN_SPRING_JUNIT_CONFIG = "org.springframework.test.context.junit.jupiter.SpringJUnitConfig"; + + public ReplaceExtendWithAndContextConfiguration() { + doNext(new UnnecessarySpringExtension()); + } + + @Override + public String getDisplayName() { + return "Replace `@ExtendWith` and `@ContextConfiguration` with `@SpringJunitConfig`"; + } + + @Override + public String getDescription() { + return "Replaces `@ExtendWith(SpringRunner.class)` and `@ContextConfiguration` with `@SpringJunitConfig`, " + + "preserving attributes on `@ContextConfiguration`, unless `@ContextConfiguration(loader = ...)` is used."; + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofMinutes(2); + } + + @Override + protected TreeVisitor getSingleSourceApplicableTest() { + return Applicability.and(new UsesType<>(FQN_EXTEND_WITH), new UsesType<>(FQN_CONTEXT_CONFIGURATION)); + } + + @Override + protected TreeVisitor getVisitor() { + return new JavaIsoVisitor() { + private final AnnotationMatcher CONTEXT_CONFIGURATION_ANNOTATION_MATCHER = new AnnotationMatcher("@" + FQN_CONTEXT_CONFIGURATION, true); + + @Override + public J.Annotation visitAnnotation(J.Annotation annotation, ExecutionContext context) { + J.Annotation a = super.visitAnnotation(annotation, context); + + if (CONTEXT_CONFIGURATION_ANNOTATION_MATCHER.matches(a) && getCursor().getParentOrThrow().getValue() instanceof J.ClassDeclaration) { + // @SpringJUnitConfig supports every attribute on @ContextConfiguration except loader() + // If it's present, skip the transformation since removing @ContextConfiguration will do harm + Optional loaderArg = findLoaderArgument(a); + if (loaderArg.isPresent()) { + return a; + } + + // @ContextConfiguration value() is an alias for locations() + // @SpringJUnitConfig value() is an alias for classes() + // Since these are incompatible, we need to map value() to locations() + // If value() is present (either explicitly or implicitly), replace it with locations() + // Note that it's invalid to specify both value() and locations() on @ContextConfiguration + if (a.getArguments() != null) { + List newArgs = new CopyOnWriteArrayList<>(a.getArguments()); + replaceValueArgumentWithLocations(a, newArgs); + a = a.withArguments(newArgs); + } + + // Change the @ContextConfiguration annotation to @SpringJUnitConfig + maybeRemoveImport(FQN_CONTEXT_CONFIGURATION); + maybeAddImport(FQN_SPRING_JUNIT_CONFIG); + a = (J.Annotation) new ChangeType(FQN_CONTEXT_CONFIGURATION, FQN_SPRING_JUNIT_CONFIG, false) + .getVisitor().visit(a, context, getCursor()); + } + + return a != null ? autoFormat(a, context) : annotation; + } + + private void replaceValueArgumentWithLocations(J.Annotation a, List newArgs) { + for (int i = 0; i < newArgs.size(); i++) { + Expression expression = newArgs.get(i); + if (expression instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) expression; + String name = ((J.Identifier) assignment.getVariable()).getSimpleName(); + if (name.equals("value")) { + J.Assignment as = createLocationsAssignment(a, assignment.getAssignment()) + .withPrefix(expression.getPrefix()); + newArgs.set(i, as); + break; + } + } else { + // The implicit assignment to "value" + J.Assignment as = createLocationsAssignment(a, expression).withPrefix(expression.getPrefix()); + newArgs.set(i, as); + break; + } + } + } + + private J.Assignment createLocationsAssignment(J.Annotation annotation, Expression value) { + return (J.Assignment) ((J.Annotation) annotation.withTemplate( + JavaTemplate.builder(this::getCursor, "locations = #{any(String)}").build(), + annotation.getCoordinates().replaceArguments(), + value + )).getArguments().get(0); + } + }; + } + + private static Optional findLoaderArgument(J.Annotation annotation) { + if (annotation.getArguments() == null) { + return Optional.empty(); + } + return annotation.getArguments().stream() + .filter(arg -> arg instanceof J.Assignment + && ((J.Assignment) arg).getVariable() instanceof J.Identifier + && "loader".equals(((J.Identifier) ((J.Assignment) arg).getVariable()).getSimpleName())) + .map(J.Assignment.class::cast) + .findFirst(); + } +} diff --git a/src/main/java/org/openrewrite/java/spring/boot2/UnnecessarySpringExtension.java b/src/main/java/org/openrewrite/java/spring/boot2/UnnecessarySpringExtension.java index 66fc36399..4a7548d06 100644 --- a/src/main/java/org/openrewrite/java/spring/boot2/UnnecessarySpringExtension.java +++ b/src/main/java/org/openrewrite/java/spring/boot2/UnnecessarySpringExtension.java @@ -51,7 +51,8 @@ public class UnnecessarySpringExtension extends Recipe { "org.springframework.boot.test.autoconfigure.data.neo4j.DataNeo4jTest", "org.springframework.boot.test.autoconfigure.data.r2dbc.DataR2dbcTest", "org.springframework.boot.test.autoconfigure.data.redis.DataRedisTest", - "org.springframework.batch.test.context.SpringBatchTest" + "org.springframework.batch.test.context.SpringBatchTest", + "org.springframework.test.context.junit.jupiter.SpringJUnitConfig" ); private static final String EXTEND_WITH_SPRING_EXTENSION_ANNOTATION_PATTERN = "@org.junit.jupiter.api.extension.ExtendWith(org.springframework.test.context.junit.jupiter.SpringExtension.class)"; diff --git a/src/main/resources/META-INF/rewrite/spring-boot-24.yml b/src/main/resources/META-INF/rewrite/spring-boot-24.yml index 1e6c6dd22..9d6000861 100644 --- a/src/main/resources/META-INF/rewrite/spring-boot-24.yml +++ b/src/main/resources/META-INF/rewrite/spring-boot-24.yml @@ -141,6 +141,8 @@ recipeList: - org.openrewrite.java.spring.boot2.OutputCaptureExtension - org.openrewrite.java.spring.boot2.UnnecessarySpringRunWith - org.openrewrite.java.spring.boot2.UnnecessarySpringExtension +# TODO Enable once tested on more projects through public.moderne.io +# - org.openrewrite.java.spring.boot2.ReplaceExtendWithAndContextConfiguration - org.openrewrite.java.spring.boot2.RemoveObsoleteSpringRunners - org.openrewrite.maven.AddDependency: groupId: org.springframework.boot diff --git a/src/testWithSpringBoot_2_3/java/org/openrewrite/java/spring/boot2/SpringBoot2JUnit4to5MigrationTest.java b/src/testWithSpringBoot_2_3/java/org/openrewrite/java/spring/boot2/SpringBoot2JUnit4to5MigrationTest.java index 31946be24..90c3748a6 100644 --- a/src/testWithSpringBoot_2_3/java/org/openrewrite/java/spring/boot2/SpringBoot2JUnit4to5MigrationTest.java +++ b/src/testWithSpringBoot_2_3/java/org/openrewrite/java/spring/boot2/SpringBoot2JUnit4to5MigrationTest.java @@ -15,6 +15,7 @@ */ package org.openrewrite.java.spring.boot2; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.openrewrite.Issue; import org.openrewrite.config.Environment; @@ -32,14 +33,10 @@ public void defaults(RecipeSpec spec) { .recipe(Environment.builder() .scanRuntimeClasspath() .build() - .activateRecipes( - "org.openrewrite.java.spring.boot2.UnnecessarySpringRunWith", - "org.openrewrite.java.spring.boot2.UnnecessarySpringExtension", - "org.openrewrite.java.testing.junit5.JUnit4to5Migration" - ) + .activateRecipes("org.openrewrite.java.spring.boot2.SpringBoot2JUnit4to5Migration") ) .parser(JavaParser.fromJavaVersion() - .classpath("spring-boot-test", "junit", "spring-test")); + .classpath("spring-boot-test", "junit", "spring-test", "spring-context")); } @Issue("https://github.com/openrewrite/rewrite-spring/issues/43") @@ -72,7 +69,7 @@ public void testFindAll() { import org.springframework.boot.test.context.SpringBootTest; @SpringBootTest - public class ProductionConfigurationTests { + class ProductionConfigurationTests { @Test void testFindAll() { @@ -82,4 +79,56 @@ void testFindAll() { ) ); } + + @Issue("https://github.com/openrewrite/rewrite-spring/issues/296") + @Test + @Disabled("Requires inclusion of ReplaceExtendWithAndContextConfiguration after that's verified on more projects") + void springBootRunWithContextConfigurationReplacedWithSpringJUnitConfig() { + //language=java + rewriteRun( + java( + """ + package org.springframework.samples.petclinic.system; + + import org.junit.Test; + import org.junit.runner.RunWith; + import org.springframework.context.annotation.Configuration; + import org.springframework.test.context.ContextConfiguration; + import org.springframework.test.context.junit4.SpringRunner; + + @RunWith(SpringRunner.class) + @ContextConfiguration(classes = ProductionConfigurationTests.CustomConfiguration.class) + public class ProductionConfigurationTests { + + @Test + public void testFindAll() { + } + + @Configuration + static class CustomConfiguration { + } + } + """, + """ + package org.springframework.samples.petclinic.system; + + import org.junit.jupiter.api.Test; + import org.springframework.context.annotation.Configuration; + import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + + @SpringJUnitConfig(classes = ProductionConfigurationTests.CustomConfiguration.class) + class ProductionConfigurationTests { + + @Test + void testFindAll() { + } + + @Configuration + static class CustomConfiguration { + } + } + """ + ) + ); + } } diff --git a/src/testWithSpringBoot_2_4/java/org/openrewrite/java/spring/boot2/ReplaceExtendWithAndContextConfigurationTest.java b/src/testWithSpringBoot_2_4/java/org/openrewrite/java/spring/boot2/ReplaceExtendWithAndContextConfigurationTest.java new file mode 100644 index 000000000..f803491c4 --- /dev/null +++ b/src/testWithSpringBoot_2_4/java/org/openrewrite/java/spring/boot2/ReplaceExtendWithAndContextConfigurationTest.java @@ -0,0 +1,272 @@ +/* + * Copyright 2023 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.spring.boot2; + +import org.junit.jupiter.api.Test; +import org.openrewrite.Issue; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +@Issue("https://github.com/openrewrite/rewrite-spring/issues/296") +class ReplaceExtendWithAndContextConfigurationTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ReplaceExtendWithAndContextConfiguration()) + .parser(JavaParser.fromJavaVersion() + .classpath("spring-boot-test", "spring-test", "junit-jupiter-api", "spring-context")); + } + + @Test + void extendWithContextConfigurationRemovedWithConfigurationClass() { + //language=java + rewriteRun( + java( + """ + package org.example; + + import org.junit.jupiter.api.extension.ExtendWith; + import org.springframework.context.annotation.Configuration; + import org.springframework.test.context.ContextConfiguration; + import org.springframework.test.context.junit.jupiter.SpringExtension; + + @ExtendWith(SpringExtension.class) + @ContextConfiguration(classes = ExampleClass.ExampleConfiguration.class) + public class ExampleClass { + @Configuration + static class ExampleConfiguration { + } + } + """, + """ + package org.example; + + import org.springframework.context.annotation.Configuration; + import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + + @SpringJUnitConfig(classes = ExampleClass.ExampleConfiguration.class) + public class ExampleClass { + @Configuration + static class ExampleConfiguration { + } + } + """ + ) + ); + } + + @Test + void extendWithContextConfigurationKeptWhenUsingLoaderArgument() { + //language=java + rewriteRun( + java( + """ + package org.example; + + import org.junit.jupiter.api.extension.ExtendWith; + import org.springframework.boot.test.context.SpringBootContextLoader; + import org.springframework.test.context.ContextConfiguration; + import org.springframework.test.context.junit.jupiter.SpringExtension; + + @ExtendWith(SpringExtension.class) + @ContextConfiguration(loader = SpringBootContextLoader.class) + public class ExampleClass { + } + """ + ) + ); + } + + @Test + void extendWithContextConfigurationUsesExplicitValueExplicitArray() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration(value = {"classpath:beans.xml"}) + """, + """ + @SpringJUnitConfig(locations = {"classpath:beans.xml"}) + """ + ); + } + + @Test + void extendWithContextConfigurationUsesExplicitValueImplicitArray() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration(value = "classpath:beans.xml") + """, + """ + @SpringJUnitConfig(locations = "classpath:beans.xml") + """ + ); + } + + @Test + void extendWithContextConfigurationUsesImplicitValueExplicitArray() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration({"classpath:beans.xml"}) + """, + """ + @SpringJUnitConfig(locations = {"classpath:beans.xml"}) + """ + ); + } + + @Test + void extendWithContextConfigurationUsesImplicitValueImplicitArray() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration("classpath:beans.xml") + """, + """ + @SpringJUnitConfig(locations = "classpath:beans.xml") + """ + ); + } + + @Test + void extendWithContextConfigurationUsesImplicitValueExplicitArrayMultipleValues() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration({"classpath:beans.xml", "classpath:more-beans.xml"}) + """, + """ + @SpringJUnitConfig(locations = {"classpath:beans.xml", "classpath:more-beans.xml"}) + """ + ); + } + + @Test + void extendWithContextConfigurationUsesExplicitValueExplicitArrayMultipleValues() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration(value = {"classpath:beans.xml", "classpath:more-beans.xml"}) + """, + """ + @SpringJUnitConfig(locations = {"classpath:beans.xml", "classpath:more-beans.xml"}) + """ + ); + } + + @Test + void extendWithContextConfigurationUsesExplicitValueExplicitArrayAndAdditionalArgumentsAfter() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration(value = {"classpath:beans.xml"}, inheritLocations = false) + """, + """ + @SpringJUnitConfig(locations = {"classpath:beans.xml"}, inheritLocations = false) + """ + ); + } + + @Test + void extendWithContextConfigurationUsesExplicitValueExplicitArrayAndAdditionalArgumentsBeforeAndAfter() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration(inheritInitializers = true, value = {"classpath:beans.xml"}, inheritLocations = false) + """, + """ + @SpringJUnitConfig(inheritInitializers = true, locations = {"classpath:beans.xml"}, inheritLocations = false) + """ + ); + } + + @Test + void extendWithContextConfigurationUsesAllSupportedContextConfigurationAttributesWithValueAttribute() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration( + value = {"classpath:beans.xml"}, + classes = {}, + initializers = {}, + inheritLocations = true, + inheritInitializers = false, + name = "name" + ) + """, + """ + @SpringJUnitConfig( + locations = {"classpath:beans.xml"}, + classes = {}, + initializers = {}, + inheritLocations = true, + inheritInitializers = false, + name = "name" + ) + """ + ); + } + + @Test + void extendWithContextConfigurationUsesAllSupportedContextConfigurationAttributesWithLocationsAttribute() { + doExtendWithContextConfigurationTest( + """ + @ContextConfiguration( + classes = {}, + initializers = {}, + inheritLocations = true, + inheritInitializers = false, + name = "name", + locations = {"classpath:beans.xml"} + ) + """, + """ + @SpringJUnitConfig( + classes = {}, + initializers = {}, + inheritLocations = true, + inheritInitializers = false, + name = "name", + locations = {"classpath:beans.xml"} + ) + """ + ); + } + + private void doExtendWithContextConfigurationTest(String contextConfigurationAnnotation, String springJunitConfigAnnotation) { + //language=java + rewriteRun( + java( + """ + package org.example; + + import org.junit.jupiter.api.extension.ExtendWith; + import org.springframework.test.context.ContextConfiguration; + import org.springframework.test.context.junit.jupiter.SpringExtension; + + @ExtendWith(SpringExtension.class) + %s + public class ExampleClass { + } + """.formatted(contextConfigurationAnnotation), + """ + package org.example; + + import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + + %s + public class ExampleClass { + } + """.formatted(springJunitConfigAnnotation) + ) + ); + } +}