From 07c929313e5dfaeefd6b2eb7b7033fb91bcd5334 Mon Sep 17 00:00:00 2001 From: sanagaraj-pivotal Date: Wed, 12 Oct 2022 15:33:33 +0100 Subject: [PATCH 1/3] Fixing in OpenRewriteMethod --- .../springframework/sbm/java/api/Method.java | 2 +- .../sbm/java/impl/OpenRewriteMethod.java | 12 ++++++--- .../sbm/java/impl/OpenRewriteMethodTest.java | 27 ++++++++++++++++++- ...SpringBeanMethodDeclarationFinderTest.java | 2 +- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/components/sbm-core/src/main/java/org/springframework/sbm/java/api/Method.java b/components/sbm-core/src/main/java/org/springframework/sbm/java/api/Method.java index 115c8e97f..7c867d91f 100644 --- a/components/sbm-core/src/main/java/org/springframework/sbm/java/api/Method.java +++ b/components/sbm-core/src/main/java/org/springframework/sbm/java/api/Method.java @@ -51,7 +51,7 @@ public interface Method { Visibility getVisibility(); - String getReturnValue(); + Optional getReturnValue(); void rename(String methodPattern, String newName); diff --git a/components/sbm-core/src/main/java/org/springframework/sbm/java/impl/OpenRewriteMethod.java b/components/sbm-core/src/main/java/org/springframework/sbm/java/impl/OpenRewriteMethod.java index b52b56915..abaa214d1 100644 --- a/components/sbm-core/src/main/java/org/springframework/sbm/java/impl/OpenRewriteMethod.java +++ b/components/sbm-core/src/main/java/org/springframework/sbm/java/impl/OpenRewriteMethod.java @@ -22,6 +22,7 @@ import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.Statement; +import org.openrewrite.java.tree.TypeTree; import org.openrewrite.java.tree.TypeUtils; import org.springframework.sbm.java.api.Annotation; import org.springframework.sbm.java.api.Method; @@ -29,7 +30,6 @@ import org.springframework.sbm.java.api.Visibility; import org.springframework.sbm.java.refactoring.JavaRefactoring; import org.springframework.sbm.project.resource.RewriteSourceFileHolder; -import org.springframework.sbm.project.resource.SbmApplicationProperties; import org.springframework.sbm.support.openrewrite.GenericOpenRewriteRecipe; import org.springframework.sbm.support.openrewrite.java.AddAnnotationVisitor; import org.springframework.sbm.support.openrewrite.java.RemoveAnnotationVisitor; @@ -158,9 +158,13 @@ public Visibility getVisibility() { } @Override - public String getReturnValue() { - JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(getMethodDecl().getReturnTypeExpression().getType()); - return fullyQualified.getFullyQualifiedName(); + public Optional getReturnValue() { + TypeTree returnTypeExpression = getMethodDecl().getReturnTypeExpression(); + if (returnTypeExpression == null || returnTypeExpression.getType() == JavaType.Primitive.Void) { + return Optional.empty(); + } + + return Optional.of(TypeUtils.asFullyQualified(returnTypeExpression.getType()).getFullyQualifiedName()); } // FIXME: renaming method should not require a methodPattern in this context diff --git a/components/sbm-core/src/test/java/org/springframework/sbm/java/impl/OpenRewriteMethodTest.java b/components/sbm-core/src/test/java/org/springframework/sbm/java/impl/OpenRewriteMethodTest.java index f11e9a87f..b739dbbda 100644 --- a/components/sbm-core/src/test/java/org/springframework/sbm/java/impl/OpenRewriteMethodTest.java +++ b/components/sbm-core/src/test/java/org/springframework/sbm/java/impl/OpenRewriteMethodTest.java @@ -16,7 +16,6 @@ package org.springframework.sbm.java.impl; import org.assertj.core.api.Assertions; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.sbm.java.api.*; import org.springframework.sbm.project.resource.TestProjectContext; @@ -24,6 +23,7 @@ import java.util.List; import java.util.regex.Pattern; +import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; @@ -72,6 +72,31 @@ void testAddAnnotation() { .isEqualTo(expected); } + @Test + void getReturnValueHandlesVoidMethods() { + String sourceCode = """ + public class Foo { + void bar() { + } + } + """; + + JavaSource javaSource = TestProjectContext.buildProjectContext() + .withJavaSources(sourceCode) + .build() + .getProjectJavaSources() + .list() + .get(0); + + List method = javaSource.getTypes().stream() + .map(Type::getMethods) + .flatMap(List::stream) + .collect(Collectors.toList()); + + assertThat(method).hasSize(1); + assertThat(method.get(0).getReturnValue()).isEmpty(); + } + @Test void testAddAnnotationWithParameter() { String sourceCode = diff --git a/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java b/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java index 387b3c7cf..3fe33fbd8 100644 --- a/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java +++ b/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java @@ -165,4 +165,4 @@ void shouldReturnTheMatchingBeanDeclarations() { -} \ No newline at end of file +} From 0335649838c2f9c54eb439e09327c688e7ae964a Mon Sep 17 00:00:00 2001 From: sanagaraj-pivotal Date: Wed, 12 Oct 2022 16:03:23 +0100 Subject: [PATCH 2/3] Fixing tests --- .../SpringBeanMethodDeclarationFinder.java | 8 +++-- ...SpringBeanMethodDeclarationFinderTest.java | 36 +++++++++++++++---- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/components/sbm-support-boot/src/main/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinder.java b/components/sbm-support-boot/src/main/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinder.java index 66b6731be..f8500a7b0 100644 --- a/components/sbm-support-boot/src/main/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinder.java +++ b/components/sbm-support-boot/src/main/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinder.java @@ -49,12 +49,14 @@ public List apply(ProjectResourceSet projectResourceSet) { .filter(t -> t .getMethods() .stream() - .anyMatch(m -> m.getReturnValue().equals(returnValueFqName) && m.hasAnnotation( - SPRING_BEAN_ANNOTATION))) + .anyMatch(m -> m.getReturnValue().isPresent() && + m.getReturnValue().get().equals(returnValueFqName) && + m.hasAnnotation(SPRING_BEAN_ANNOTATION) + )) .forEach(t -> t .getMethods() .stream() - .filter(m -> m.getReturnValue().equals(returnValueFqName)) + .filter(m -> m.getReturnValue().get().equals(returnValueFqName)) .forEach(m -> matches.add(new MatchingMethod(js, t, m)))); }); return matches; diff --git a/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java b/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java index 3fe33fbd8..c77d0599a 100644 --- a/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java +++ b/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java @@ -16,8 +16,6 @@ package org.springframework.sbm.boot.common.finder; -import org.assertj.core.api.Assertions; -import org.intellij.lang.annotations.Language; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -26,7 +24,6 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; class SpringBeanMethodDeclarationFinderTest { @@ -84,7 +81,7 @@ void shouldReturnTheMatchingBeanDeclaration() { assertThat(matches).hasSize(1); assertThat(matches.get(0).getJavaSource().getSourcePath().toString()).isEqualTo("src/main/java/MyConfiguration.java"); assertThat(matches.get(0).getType().getFullyQualifiedName()).isEqualTo("MyConfiguration"); - assertThat(matches.get(0).getMethod().getReturnValue()).isEqualTo("a.b.c.SomeBean"); + assertThat(matches.get(0).getMethod().getReturnValue().get()).isEqualTo("a.b.c.SomeBean"); assertThat(matches.get(0).getMethod().getName()).isEqualTo("someBean"); } } @@ -154,15 +151,42 @@ void shouldReturnTheMatchingBeanDeclarations() { assertThat(matches).hasSize(2); assertThat(matches.get(0).getJavaSource().getSourcePath().toString()).isEqualTo("src/main/java/MyConfiguration.java"); assertThat(matches.get(0).getType().getFullyQualifiedName()).isEqualTo("MyConfiguration"); - assertThat(matches.get(0).getMethod().getReturnValue()).isEqualTo("a.b.c.SomeBean"); + assertThat(matches.get(0).getMethod().getReturnValue()).isPresent(); + assertThat(matches.get(0).getMethod().getReturnValue().get()).isEqualTo("a.b.c.SomeBean"); assertThat(matches.get(0).getMethod().getName()).isEqualTo("someBean"); assertThat(matches.get(1).getJavaSource().getSourcePath().toString()).isEqualTo("src/main/java/MyConfiguration2.java"); assertThat(matches.get(1).getType().getFullyQualifiedName()).isEqualTo("MyConfiguration2"); assertThat(matches.get(1).getMethod().getName()).isEqualTo("someBean2"); - assertThat(matches.get(1).getMethod().getReturnValue()).isEqualTo("a.b.c.SomeBean"); + assertThat(matches.get(1).getMethod().getReturnValue()).isPresent(); + assertThat(matches.get(1).getMethod().getReturnValue().get()).isEqualTo("a.b.c.SomeBean"); } } + @Nested + class WithVoidBeans { + + @Test + void shouldRecogniseMethodWithVoidReturns() { + builder.addJavaSource("src/main/java", + """ + import org.springframework.context.annotation.Configuration; + import org.springframework.context.annotation.Bean; + + @Configuration + public class MyConfiguration { + @Bean + void someBean() { + return null; + } + } + """ + ) + .withBuildFileHavingDependencies("org.springframework:spring-context:5.3.22"); + + List matches = builder.build().search(sut); + assertThat(matches).hasSize(1); + } + } } From 4e65d3db94b780f6fdc4b0a8601676b2be0dd263 Mon Sep 17 00:00:00 2001 From: sanagaraj-pivotal Date: Wed, 12 Oct 2022 16:09:19 +0100 Subject: [PATCH 3/3] Fixing tests --- .../common/finder/SpringBeanMethodDeclarationFinder.java | 1 + .../finder/SpringBeanMethodDeclarationFinderTest.java | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/components/sbm-support-boot/src/main/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinder.java b/components/sbm-support-boot/src/main/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinder.java index f8500a7b0..30864d8a2 100644 --- a/components/sbm-support-boot/src/main/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinder.java +++ b/components/sbm-support-boot/src/main/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinder.java @@ -59,6 +59,7 @@ public List apply(ProjectResourceSet projectResourceSet) { .filter(m -> m.getReturnValue().get().equals(returnValueFqName)) .forEach(m -> matches.add(new MatchingMethod(js, t, m)))); }); + return matches; } } diff --git a/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java b/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java index c77d0599a..59ecb8c3f 100644 --- a/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java +++ b/components/sbm-support-boot/src/test/java/org/springframework/sbm/boot/common/finder/SpringBeanMethodDeclarationFinderTest.java @@ -167,7 +167,8 @@ void shouldReturnTheMatchingBeanDeclarations() { class WithVoidBeans { @Test - void shouldRecogniseMethodWithVoidReturns() { + void shouldReturnEmptyListWithVoidReturnTypeOnBean() { + builder.addJavaSource("src/main/java", """ import org.springframework.context.annotation.Configuration; @@ -184,8 +185,8 @@ void someBean() { ) .withBuildFileHavingDependencies("org.springframework:spring-context:5.3.22"); - List matches = builder.build().search(sut); - assertThat(matches).hasSize(1); + List output = builder.build().search(sut); + assertThat(output).isEmpty(); } }