diff --git a/src/main/java/org/openrewrite/java/spring/NoAutowiredOnConstructor.java b/src/main/java/org/openrewrite/java/spring/NoAutowiredOnConstructor.java new file mode 100644 index 000000000..3c65593ba --- /dev/null +++ b/src/main/java/org/openrewrite/java/spring/NoAutowiredOnConstructor.java @@ -0,0 +1,157 @@ +/* + * Copyright 2020 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; + +import org.openrewrite.Cursor; +import org.openrewrite.ExecutionContext; +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.search.UsesType; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.Space; +import org.openrewrite.java.tree.Statement; +import org.openrewrite.java.tree.TypeUtils; + +import java.util.ArrayList; +import java.util.List; + +/** + * Removes the Autowired annotation from a bean's constructor when there is only one constructor. + */ +public class NoAutowiredOnConstructor extends Recipe { + + @Override + public String getDisplayName() { + return "Remove the `@Autowired` annotation on inferred constructor"; + } + + @Override + public String getDescription() { + return "Spring can infer an autowired constructor when there is a single constructor on the bean."; + } + + @Nullable + @Override + protected TreeVisitor getSingleSourceApplicableTest() { + return new UsesType<>("org.springframework.beans.factory.annotation.Autowired"); + } + + @Override + protected TreeVisitor getVisitor() { + return new NoAutowiredOnConstructor.NoAutowiredOnConstructorVisitor(); + } + + private static class NoAutowiredOnConstructorVisitor extends JavaIsoVisitor { + private static final AnnotationMatcher REQUIRED_AUTOWIRED_ANNOTATION_MATCHER = + new AnnotationMatcher("@org.springframework.beans.factory.annotation.Autowired(true)"); + + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext executionContext) { + int constructorCount = 0; + + for (Statement s : classDecl.getBody().getStatements()) { + if (s instanceof J.MethodDeclaration) { + if (((J.MethodDeclaration) s).isConstructor()) { + constructorCount++; + getCursor().putMessage("METHOD_DECLARATION_KEY", s); + } + } + } + + if (constructorCount == 1) { + return super.visitClassDeclaration(classDecl, executionContext); + } + + return classDecl; + } + + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { + J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx); + J.Annotation annotationRemoved = getCursor().pollMessage("annotationRemoved"); + + List leadingAnnotations = method.getLeadingAnnotations(); + if (annotationRemoved != null && !leadingAnnotations.isEmpty()) { + if (leadingAnnotations.size() == 1 && leadingAnnotations.get(0) == annotationRemoved) { + if (!m.getModifiers().isEmpty()) { + m = m.withModifiers(Space.formatFirstPrefix(m.getModifiers(), Space.firstPrefix(m.getModifiers()).withWhitespace(""))); + } else { + m = m.withName(m.getName().withPrefix(m.getName().getPrefix().withWhitespace(""))); + } + } else { + int index = leadingAnnotations.indexOf(annotationRemoved); + if (index == 0) { + List newLeadingAnnotations = new ArrayList<>(); + J.Annotation nextAnnotation = leadingAnnotations.get(1); + if (!nextAnnotation.getPrefix().equals(annotationRemoved.getPrefix())) { + newLeadingAnnotations.add(nextAnnotation.withPrefix(annotationRemoved.getPrefix())); + for (int i = 2; i < leadingAnnotations.size(); ++i) { + newLeadingAnnotations.add(leadingAnnotations.get(i)); + } + m = m.withLeadingAnnotations(newLeadingAnnotations); + } + } + } + } + return m; + } + + @Override + public J.Annotation visitAnnotation(J.Annotation annotation, ExecutionContext ctx) { + J.Annotation a = super.visitAnnotation(annotation, ctx); + + Cursor cursor = getCursorToParentScope(getCursor()); + if (cursor.getValue() instanceof J.MethodDeclaration) { + Cursor classDeclarationCursor = cursor.dropParentUntil(J.ClassDeclaration.class::isInstance); + J.MethodDeclaration m = classDeclarationCursor.getMessage("METHOD_DECLARATION_KEY"); + if (cursor.getValue().equals(m)) { + if (REQUIRED_AUTOWIRED_ANNOTATION_MATCHER.matches(annotation)) { + cursor.putMessage("annotationRemoved", annotation); + maybeRemoveImport(TypeUtils.asFullyQualified(annotation.getType())); + //noinspection ConstantConditions + return null; + } + } + } + return a; + } + + /** + * Returns either the current block or a J.Type that may create a reference to a variable. + * I.E. for(int target = 0; target < N; target++) creates a new name scope for `target`. + * The name scope in the next J.Block `{}` cannot create new variables with the name `target`. + *

+ * J.* types that may only reference an existing name and do not create a new name scope are excluded. + */ + private static Cursor getCursorToParentScope(Cursor cursor) { + return cursor.dropParentUntil(is -> + is instanceof J.Block || + is instanceof J.MethodDeclaration || + is instanceof J.ForLoop || + is instanceof J.ForEachLoop || + is instanceof J.ForLoop.Control || + is instanceof J.Case || + is instanceof J.Try || + is instanceof J.Try.Catch || + is instanceof J.MultiCatch || + is instanceof J.Lambda + ); + } + } +} diff --git a/src/main/resources/META-INF/rewrite/spring-boot2.yml b/src/main/resources/META-INF/rewrite/spring-boot2.yml index d67e522b1..1d064ceb9 100644 --- a/src/main/resources/META-INF/rewrite/spring-boot2.yml +++ b/src/main/resources/META-INF/rewrite/spring-boot2.yml @@ -118,7 +118,7 @@ recipeList: onlyIfUsing: - javax.validation.constraints.NotBlank - javax.validation.constraints.NotEmpty - - org.openrewrite.java.spring.NoAutowired + - org.openrewrite.java.spring.NoAutowiredOnConstructor - org.openrewrite.java.spring.boot2.ConditionalOnBeanAnyNestedCondition - org.openrewrite.java.spring.boot2.RestTemplateBuilderRequestFactory - org.openrewrite.java.spring.boot2.ReplaceDeprecatedEnvironmentTestUtils diff --git a/src/main/resources/META-INF/rewrite/spring.yml b/src/main/resources/META-INF/rewrite/spring.yml deleted file mode 100644 index f16f56885..000000000 --- a/src/main/resources/META-INF/rewrite/spring.yml +++ /dev/null @@ -1,25 +0,0 @@ -# -# Copyright 2021 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. -# ---- -type: specs.openrewrite.org/v1beta/recipe -name: org.openrewrite.java.spring.NoAutowired -displayName: Remove the Spring `@Autowired` annotation -description: Spring can dependency inject `@Autowire` annotated collaborators without the annotation. -tags: - - spring -recipeList: - - org.openrewrite.java.RemoveAnnotation: - annotationPattern: '@org.springframework.beans.factory.annotation.Autowired(true)' diff --git a/src/test/kotlin/org/openrewrite/java/spring/NoAutowiredTest.kt b/src/test/kotlin/org/openrewrite/java/spring/NoAutowiredTest.kt index 03138cb32..d52726c6a 100644 --- a/src/test/kotlin/org/openrewrite/java/spring/NoAutowiredTest.kt +++ b/src/test/kotlin/org/openrewrite/java/spring/NoAutowiredTest.kt @@ -19,7 +19,6 @@ import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test import org.openrewrite.Issue import org.openrewrite.Recipe -import org.openrewrite.config.Environment import org.openrewrite.java.JavaParser import org.openrewrite.java.JavaRecipeTest @@ -30,13 +29,324 @@ class NoAutowiredTest : JavaRecipeTest { .build() override val recipe: Recipe - get() = Environment.builder() - .scanRuntimeClasspath() - .build() - .activateRecipes("org.openrewrite.java.spring.NoAutowired") + get() = NoAutowiredOnConstructor() + @Issue("https://github.com/openrewrite/rewrite-spring/issues/78") @Test - fun removeAutowiredAnnotations() = assertChanged( + fun removeLeadingAutowiredAnnotation() = assertChanged( + dependsOn = arrayOf( + """ + package org.C; + import org.springframework.stereotype.Component; + @Component + public class TestSourceA { + } + """, + """ + package org.B; + import org.springframework.stereotype.Component; + @Component + public class TestSourceB { + } + """, + """ + package org.C; + import org.springframework.stereotype.Component; + @Component + public class TestSourceC { + } + """), + before = """ + import org.A.TestSourceA; + import org.B.TestSourceB; + import org.C.TestSourceC; + import org.springframework.beans.factory.annotation.Autowired; + + public class TestConfiguration { + private final TestSourceA testSourceA; + private TestSourceB testSourceB; + + @Autowired + private TestSourceC testSourceC; + + @Autowired + public TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + + @Autowired + public void setTestSourceB(TestSourceB testSourceB) { + this.testSourceB = testSourceB; + } + } + """, + after = """ + import org.A.TestSourceA; + import org.B.TestSourceB; + import org.C.TestSourceC; + import org.springframework.beans.factory.annotation.Autowired; + + public class TestConfiguration { + private final TestSourceA testSourceA; + private TestSourceB testSourceB; + + @Autowired + private TestSourceC testSourceC; + + public TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + + @Autowired + public void setTestSourceB(TestSourceB testSourceB) { + this.testSourceB = testSourceB; + } + } + """ + ) + + @Issue("https://github.com/openrewrite/rewrite-spring/issues/78") + @Test + fun removeLeadingAutowiredAnnotationNoModifiers() = assertChanged( + dependsOn = arrayOf( + """ + package org.C; + import org.springframework.stereotype.Component; + @Component + public class TestSourceA { + } + """, + """ + package org.B; + import org.springframework.stereotype.Component; + @Component + public class TestSourceB { + } + """, + """ + package org.C; + import org.springframework.stereotype.Component; + @Component + public class TestSourceC { + } + """), + before = """ + import org.A.TestSourceA; + import org.B.TestSourceB; + import org.C.TestSourceC; + import org.springframework.beans.factory.annotation.Autowired; + + public class TestConfiguration { + private final TestSourceA testSourceA; + private TestSourceB testSourceB; + + @Autowired + private TestSourceC testSourceC; + + @Autowired + TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + + @Autowired + public void setTestSourceB(TestSourceB testSourceB) { + this.testSourceB = testSourceB; + } + } + """, + after = """ + import org.A.TestSourceA; + import org.B.TestSourceB; + import org.C.TestSourceC; + import org.springframework.beans.factory.annotation.Autowired; + + public class TestConfiguration { + private final TestSourceA testSourceA; + private TestSourceB testSourceB; + + @Autowired + private TestSourceC testSourceC; + + TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + + @Autowired + public void setTestSourceB(TestSourceB testSourceB) { + this.testSourceB = testSourceB; + } + } + """ + ) + + @Issue("https://github.com/openrewrite/rewrite-spring/issues/78") + @Test + fun removeAutowiredWithMultipleAnnotation() = assertChanged( + dependsOn = arrayOf( + """ + package org.C; + import org.springframework.stereotype.Component; + @Component + public class TestSourceA { + } + """), + before = """ + import org.A.TestSourceA; + import org.springframework.beans.factory.annotation.Autowired; + import org.springframework.beans.factory.annotation.Qualifier; + import org.springframework.beans.factory.annotation.Required; + + public class AnnotationPos1 { + private final TestSourceA testSourceA; + + @Autowired + @Required + @Qualifier + public TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + + public class AnnotationPos2 { + private final TestSourceA testSourceA; + + @Required + @Autowired + @Qualifier + public TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + + public class AnnotationPos3 { + private final TestSourceA testSourceA; + + @Required + @Qualifier + @Autowired + public TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + """, + after = """ + import org.A.TestSourceA; + import org.springframework.beans.factory.annotation.Qualifier; + import org.springframework.beans.factory.annotation.Required; + + public class AnnotationPos1 { + private final TestSourceA testSourceA; + + @Required + @Qualifier + publicAnnotationPos1 TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + + public class AnnotationPos2 { + private final TestSourceA testSourceA; + + @Required + @Qualifier + publicAnnotationPos2 TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + + public class AnnotationPos3 { + private final TestSourceA testSourceA; + + @Required + @Qualifier + publicAnnotationPos3 TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + """ + ) + + @Issue("https://github.com/openrewrite/rewrite-spring/issues/78") + @Test + fun removeAutowiredWithMultipleInLineAnnotation() = assertChanged( + dependsOn = arrayOf( + """ + package org.C; + import org.springframework.stereotype.Component; + @Component + public class TestSourceA { + } + """), + before = """ + import org.A.TestSourceA; + import org.springframework.beans.factory.annotation.Autowired; + import org.springframework.beans.factory.annotation.Qualifier; + import org.springframework.beans.factory.annotation.Required; + + public class AnnotationPos1 { + private final TestSourceA testSourceA; + + @Autowired @Required @Qualifier + public TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + + public class AnnotationPos2 { + private final TestSourceA testSourceA; + + @Required @Autowired @Qualifier + public TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + + public class AnnotationPos3 { + private final TestSourceA testSourceA; + + @Required @Qualifier @Autowired + public TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + """, + after = """ + import org.A.TestSourceA; + import org.springframework.beans.factory.annotation.Qualifier; + import org.springframework.beans.factory.annotation.Required; + + public class AnnotationPos1 { + private final TestSourceA testSourceA; + + @Required @Qualifier + publicAnnotationPos1 TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + + public class AnnotationPos2 { + private final TestSourceA testSourceA; + + @Required @Qualifier + publicAnnotationPos2 TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + + public class AnnotationPos3 { + private final TestSourceA testSourceA; + + @Required @Qualifier + publicAnnotationPos3 TestConfiguration(TestSourceA testSourceA) { + this.testSourceA = testSourceA; + } + } + """ + ) + @Disabled + @Issue("https://github.com/openrewrite/rewrite/issues/726") + @Test + fun removeNamePrefixAutowiredAnnotation() = assertChanged( before = """ import javax.sql.DataSource; import org.springframework.beans.factory.annotation.Autowired; @@ -44,8 +354,7 @@ class NoAutowiredTest : JavaRecipeTest { public class DatabaseConfiguration { private final DataSource dataSource; - @Autowired - public DatabaseConfiguration(DataSource dataSource) { + public @Autowired DatabaseConfiguration(DataSource dataSource) { } } """, @@ -61,7 +370,6 @@ class NoAutowiredTest : JavaRecipeTest { """ ) - @Disabled @Issue("https://github.com/openrewrite/rewrite-spring/issues/78") @Test fun keepAutowiredAnnotationsWhenMultipleConstructorsExist() = assertUnchanged(