From 5b49715dd77af81f41f77a269937c9e2ea0c57d5 Mon Sep 17 00:00:00 2001 From: aboyko Date: Thu, 31 Aug 2023 13:37:31 -0400 Subject: [PATCH] Only flag @RequestMapping with single request method --- .../NoRequestMappingAnnotationReconciler.java | 36 ++++++-- ...equestMappingAnnotationReconcilerTest.java | 86 ++++++++++++++++++- 2 files changed, 113 insertions(+), 9 deletions(-) diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/NoRequestMappingAnnotationReconciler.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/NoRequestMappingAnnotationReconciler.java index e2d2ba25c..23274686a 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/NoRequestMappingAnnotationReconciler.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/NoRequestMappingAnnotationReconciler.java @@ -17,9 +17,11 @@ import java.util.List; import org.eclipse.jdt.core.dom.ASTVisitor; import org.eclipse.jdt.core.dom.Annotation; +import org.eclipse.jdt.core.dom.ArrayInitializer; import org.eclipse.jdt.core.dom.CompilationUnit; import org.eclipse.jdt.core.dom.ITypeBinding; import org.eclipse.jdt.core.dom.MarkerAnnotation; +import org.eclipse.jdt.core.dom.MemberValuePair; import org.eclipse.jdt.core.dom.MethodDeclaration; import org.eclipse.jdt.core.dom.NormalAnnotation; import org.eclipse.jdt.core.dom.SingleMemberAnnotation; @@ -90,13 +92,33 @@ public class NoRequestMappingAnnotationReconciler implements JdtAstReconciler { } private static boolean isRequestMappingAnnotation(CompilationUnit cu, Annotation a) { - String typeName = a.getTypeName().getFullyQualifiedName(); - if (Annotations.SPRING_REQUEST_MAPPING.equals(typeName)) { - return true; - } else if (typeName.endsWith("RequestMapping")) { - ITypeBinding type = a.resolveTypeBinding(); - if (type != null && Annotations.SPRING_REQUEST_MAPPING.equals(type.getQualifiedName())) { - return true; + // Consider only NormalAnnotation as we need to flag @RequestMapping with single method parameter value + if (a.isNormalAnnotation()) { + String typeName = a.getTypeName().getFullyQualifiedName(); + if (Annotations.SPRING_REQUEST_MAPPING.equals(typeName)) { + return hasApplicableMethodParameter(a); + } else if (typeName.endsWith("RequestMapping")) { + ITypeBinding type = a.resolveTypeBinding(); + if (type != null && Annotations.SPRING_REQUEST_MAPPING.equals(type.getQualifiedName())) { + return hasApplicableMethodParameter(a); + } + } + } + return false; + } + + private static boolean hasApplicableMethodParameter(Annotation a) { + if (a.isNormalAnnotation()) { + for (Object o : ((NormalAnnotation) a).values()) { + if (o instanceof MemberValuePair) { + MemberValuePair pair = (MemberValuePair) o; + if ("method".equals(pair.getName().getIdentifier())) { + if (pair.getValue() instanceof ArrayInitializer) { + return ((ArrayInitializer) pair.getValue()).expressions().size() == 1; + } + return true; + } + } } } return false; diff --git a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/NoRequestMappingAnnotationReconcilerTest.java b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/NoRequestMappingAnnotationReconcilerTest.java index 0faa89db7..c88ebf83b 100644 --- a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/NoRequestMappingAnnotationReconcilerTest.java +++ b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/NoRequestMappingAnnotationReconcilerTest.java @@ -56,12 +56,13 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest package example.demo; import org.springframework.web.bind.annotation.RequestMapping; + import org.springframework.web.bind.annotation.RequestMethod; @RequestMapping("/hello") class A { - @RequestMapping("/1") + @RequestMapping(value = "/1", method = RequestMethod.GET) String hello1() { return "1"; }; @@ -77,12 +78,93 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest assertEquals(Boot2JavaProblemType.JAVA_PRECISE_REQUEST_MAPPING, problem.getType()); String markedStr = source.substring(problem.getOffset(), problem.getOffset() + problem.getLength()); - assertEquals("@RequestMapping(\"/1\")", markedStr); + assertEquals("@RequestMapping(value = \"/1\", method = RequestMethod.GET)", markedStr); assertEquals(2, problem.getQuickfixes().size()); } + @Test + void arrayMethodTest() throws Exception { + String source = """ + package example.demo; + + import org.springframework.web.bind.annotation.RequestMapping; + import org.springframework.web.bind.annotation.RequestMethod; + + + @RequestMapping("/hello") + class A { + + @RequestMapping(value = "/1", method = { RequestMethod.GET }) + String hello1() { + return "1"; + }; + + } + """; + List problems = reconcile("A.java", source, false); + + assertEquals(1, problems.size()); + + ReconcileProblem problem = problems.get(0); + + assertEquals(Boot2JavaProblemType.JAVA_PRECISE_REQUEST_MAPPING, problem.getType()); + + String markedStr = source.substring(problem.getOffset(), problem.getOffset() + problem.getLength()); + assertEquals("@RequestMapping(value = \"/1\", method = { RequestMethod.GET })", markedStr); + + assertEquals(2, problem.getQuickfixes().size()); + + } + + @Test + void multiMethodTest() throws Exception { + String source = """ + package example.demo; + + import org.springframework.web.bind.annotation.RequestMapping; + import org.springframework.web.bind.annotation.RequestMethod; + + + @RequestMapping("/hello") + class A { + + @RequestMapping(value = "/1", method = { RequestMethod.GET, RequestMethod.HEAD }) + String hello1() { + return "1"; + }; + + } + """; + List problems = reconcile("A.java", source, false); + + assertEquals(0, problems.size()); + } + + @Test + void noMethodsNoProblems() throws Exception { + String source = """ + package example.demo; + + import org.springframework.web.bind.annotation.RequestMapping; + + + @RequestMapping("/hello") + class A { + + @RequestMapping("/1") + String hello1() { + return "1"; + }; + + } + """; + List problems = reconcile("A.java", source, false); + + assertEquals(0, problems.size()); + } + @Test void noProblems() throws Exception { String source = """