From 0e64d9683615b602c62f40ca7c6321e92194090d Mon Sep 17 00:00:00 2001 From: Udayani Vaka <79973862+vudayani@users.noreply.github.com> Date: Wed, 26 Jun 2024 20:32:54 +0530 Subject: [PATCH] boot2 best practice - Apply recipe to remove implicit web annotation names (#1278) * boot2 best practice - remove implicit web annotation names * add tests and generate problemTypes metadata * use OR recipe for Node scope --- .../ide/vscode/boot/app/JdtConfig.java | 5 + .../boot/java/Boot2JavaProblemType.java | 4 +- .../ImplicitWebAnnotationNamesReconciler.java | 142 ++++++++++++ .../src/main/resources/problem-types.json | 6 + ...licitWebAnnotationNamesReconcilerTest.java | 213 ++++++++++++++++++ .../vscode-spring-boot/package.json | 12 + 6 files changed, 381 insertions(+), 1 deletion(-) create mode 100644 headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/ImplicitWebAnnotationNamesReconciler.java create mode 100644 headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/ImplicitWebAnnotationNamesReconcilerTest.java diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/app/JdtConfig.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/app/JdtConfig.java index 0d4802bc0..9d50ac9bd 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/app/JdtConfig.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/app/JdtConfig.java @@ -29,6 +29,7 @@ import org.springframework.ide.vscode.boot.java.reconcilers.BeanPostProcessingIg import org.springframework.ide.vscode.boot.java.reconcilers.Boot3NotSupportedTypeReconciler; import org.springframework.ide.vscode.boot.java.reconcilers.EntityIdForRepoReconciler; import org.springframework.ide.vscode.boot.java.reconcilers.HttpSecurityLambdaDslReconciler; +import org.springframework.ide.vscode.boot.java.reconcilers.ImplicitWebAnnotationNamesReconciler; import org.springframework.ide.vscode.boot.java.reconcilers.ModulithTypeReferenceViolationReconciler; import org.springframework.ide.vscode.boot.java.reconcilers.NoAutowiredOnConstructorReconciler; import org.springframework.ide.vscode.boot.java.reconcilers.NoRepoAnnotationReconciler; @@ -140,5 +141,9 @@ public class JdtConfig { @Bean JdtSpelReconciler jdtSpelReconciler(SpelReconciler spelReconciler) { return new JdtSpelReconciler(spelReconciler); } + + @Bean ImplicitWebAnnotationNamesReconciler implicitWebAnnotationNamesReconciler(SimpleLanguageServer server) { + return new ImplicitWebAnnotationNamesReconciler(server.getQuickfixRegistry()); + } } diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Boot2JavaProblemType.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Boot2JavaProblemType.java index 467b849f8..2310bcd87 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Boot2JavaProblemType.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Boot2JavaProblemType.java @@ -39,7 +39,9 @@ public enum Boot2JavaProblemType implements ProblemType { WEB_SECURITY_CONFIGURER_ADAPTER(WARNING, "'WebSecurityConfigurerAdapter' is removed in Spring-Security 6.x. Refactor classes extending the 'WebSecurityConfigurerAdapter' into 'Configuration' beans and methods into 'Bean' definitions ", "Replace usage of 'WebSecurityConfigurerAdapter' as this class to be removed in Security 6.x"), - DOMAIN_ID_FOR_REPOSITORY(ERROR, "Invalid Domain ID type for Spring Data Repository", "Invalid Domain ID Type for Spring Data Repository"); + DOMAIN_ID_FOR_REPOSITORY(ERROR, "Invalid Domain ID type for Spring Data Repository", "Invalid Domain ID Type for Spring Data Repository"), + + WEB_ANNOTATION_NAMES(HINT, "Web annotation names are unnecessary when it is the same as method parameter name", "Implicit web annotations names"); private final ProblemSeverity defaultSeverity; private String description; diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/ImplicitWebAnnotationNamesReconciler.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/ImplicitWebAnnotationNamesReconciler.java new file mode 100644 index 000000000..73c152fe3 --- /dev/null +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/ImplicitWebAnnotationNamesReconciler.java @@ -0,0 +1,142 @@ +package org.springframework.ide.vscode.boot.java.reconcilers; + +import static org.springframework.ide.vscode.commons.java.SpringProjectUtil.springBootVersionGreaterOrEqual; + +import java.net.URI; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; + +import org.eclipse.jdt.core.dom.ASTNode; +import org.eclipse.jdt.core.dom.ASTVisitor; +import org.eclipse.jdt.core.dom.Annotation; +import org.eclipse.jdt.core.dom.CompilationUnit; +import org.eclipse.jdt.core.dom.Expression; +import org.eclipse.jdt.core.dom.MemberValuePair; +import org.eclipse.jdt.core.dom.NormalAnnotation; +import org.eclipse.jdt.core.dom.SingleMemberAnnotation; +import org.eclipse.jdt.core.dom.SingleVariableDeclaration; +import org.eclipse.jdt.core.dom.StringLiteral; +import org.openrewrite.marker.Range; +import org.springframework.ide.vscode.boot.java.Boot2JavaProblemType; +import org.springframework.ide.vscode.commons.java.IJavaProject; +import org.springframework.ide.vscode.commons.languageserver.quickfix.QuickfixRegistry; +import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector; +import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblemImpl; +import org.springframework.ide.vscode.commons.rewrite.config.RecipeScope; +import org.springframework.ide.vscode.commons.rewrite.java.FixDescriptor; + +public class ImplicitWebAnnotationNamesReconciler implements JdtAstReconciler { + + private static final String LABEL = "Remove Implicit Web Annotation Names"; + + private static final Set PARAM_ANNOTATIONS = new HashSet<>( + Arrays.asList( + "PathVariable", + "RequestParam", + "RequestHeader", + "RequestAttribute", + "CookieValue", + "ModelAttribute", + "SessionAttribute" + ) + ); + + private QuickfixRegistry registry; + + public ImplicitWebAnnotationNamesReconciler(QuickfixRegistry registry) { + this.registry = registry; + } + + @Override + public boolean isApplicable(IJavaProject project) { + return springBootVersionGreaterOrEqual(2, 0, 0).test(project); + } + + @Override + public Boot2JavaProblemType getProblemType() { + return Boot2JavaProblemType.WEB_ANNOTATION_NAMES; + } + + @Override + public ASTVisitor createVisitor(IJavaProject project, URI docUri, CompilationUnit cu, IProblemCollector problemCollector, boolean isCompleteAst) { + + return new ASTVisitor() { + + @Override + public boolean visit(NormalAnnotation node) { + processWebAnnotation(node); + return false; + } + + @Override + public boolean visit(SingleMemberAnnotation node) { + processWebAnnotation(node); + return false; + } + + private void processWebAnnotation(Annotation a) { + if (isApplicableWebAnnotation(a)) { + ReconcileProblemImpl problem = new ReconcileProblemImpl(getProblemType(), LABEL, a.getStartPosition(), a.getLength()); + String uri = docUri.toASCIIString(); + Range range = ReconcileUtils.createOpenRewriteRange(cu, a); + ReconcileUtils.setRewriteFixes(registry, problem, List.of( + new FixDescriptor(org.openrewrite.java.spring.ImplicitWebAnnotationNames.class.getName(), List.of(uri), "Remove Implicit Web Annotation Name") + .withRangeScope(range) + .withRecipeScope(RecipeScope.NODE), + new FixDescriptor(org.openrewrite.java.spring.ImplicitWebAnnotationNames.class.getName(), List.of(uri), + ReconcileUtils.buildLabel(LABEL, RecipeScope.FILE)) + .withRecipeScope(RecipeScope.FILE), + new FixDescriptor(org.openrewrite.java.spring.ImplicitWebAnnotationNames.class.getName(), List.of(uri), + ReconcileUtils.buildLabel(LABEL, RecipeScope.PROJECT)) + .withRecipeScope(RecipeScope.PROJECT) + )); + problemCollector.accept(problem); + } + } + }; + } + + private static boolean isApplicableWebAnnotation(Annotation a) { + if (a.isSingleMemberAnnotation() || a.isNormalAnnotation()) { + String typeName = a.getTypeName().getFullyQualifiedName(); + String annotationParam = getAnnotationParameter(a); + String variableName = getParameterName(a); + if (PARAM_ANNOTATIONS.contains(typeName) && annotationParam != null && variableName != null) { + if(Objects.equals(annotationParam, variableName)) + return true; + } + } + return false; + } + + @SuppressWarnings("unchecked") + private static String getAnnotationParameter(Annotation a) { + Expression value = null; + if (a.isSingleMemberAnnotation()) { + value = ((SingleMemberAnnotation) a).getValue(); + } else if (a.isNormalAnnotation()) { + for (MemberValuePair pair : (List) ((NormalAnnotation) a).values()) { + String identifier = pair.getName().toString(); + value = identifier.equals("value") || identifier.equals("name") ? pair.getValue() : value; + } + } + if (value instanceof StringLiteral) { + return ((StringLiteral) value).getLiteralValue(); + } + return null; + } + + private static String getParameterName(Annotation a) { + ASTNode parent = a.getParent(); + if (parent instanceof SingleVariableDeclaration) { + SingleVariableDeclaration svd = (SingleVariableDeclaration) parent; + return svd.getName().getIdentifier(); + + } + return null; + } + +} diff --git a/headless-services/spring-boot-language-server/src/main/resources/problem-types.json b/headless-services/spring-boot-language-server/src/main/resources/problem-types.json index 8ddebe9fd..6e0d042a8 100644 --- a/headless-services/spring-boot-language-server/src/main/resources/problem-types.json +++ b/headless-services/spring-boot-language-server/src/main/resources/problem-types.json @@ -79,6 +79,12 @@ "label": "Invalid Domain ID Type for Spring Data Repository", "description": "Invalid Domain ID type for Spring Data Repository", "defaultSeverity": "ERROR" + }, + { + "code": "WEB_ANNOTATION_NAMES", + "label": "Implicit web annotations names", + "description": "Web annotation names are unnecessary when it is the same as method parameter name", + "defaultSeverity": "HINT" } ] }, diff --git a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/ImplicitWebAnnotationNamesReconcilerTest.java b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/ImplicitWebAnnotationNamesReconcilerTest.java new file mode 100644 index 000000000..133ea5ace --- /dev/null +++ b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/reconcilers/test/ImplicitWebAnnotationNamesReconcilerTest.java @@ -0,0 +1,213 @@ +package org.springframework.ide.vscode.boot.java.reconcilers.test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.List; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.ide.vscode.boot.java.Boot2JavaProblemType; +import org.springframework.ide.vscode.boot.java.reconcilers.ImplicitWebAnnotationNamesReconciler; +import org.springframework.ide.vscode.boot.java.reconcilers.JdtAstReconciler; +import org.springframework.ide.vscode.commons.languageserver.quickfix.QuickfixRegistry; +import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblem; + +public class ImplicitWebAnnotationNamesReconcilerTest extends BaseReconcilerTest { + + @Override + protected String getFolder() { + return "implicitwebannotationnames"; + } + + @Override + protected String getProjectName() { + return "test-spring-validations"; + } + + @Override + protected JdtAstReconciler getReconciler() { + return new ImplicitWebAnnotationNamesReconciler(new QuickfixRegistry()); + } + + @BeforeEach + void setup() throws Exception { + super.setup(); + } + + @AfterEach + void tearDown() throws Exception { + super.tearDown(); + } + + @Test + void sanityTest() throws Exception { + String source = """ + package example.demo; + + import org.springframework.stereotype.Controller; + import org.springframework.web.bind.annotation.GetMapping; + import org.springframework.web.bind.annotation.PathVariable; + + @Controller + class A { + + @GetMapping("/hello/{message}") + public String hello(@PathVariable("message") String message) { + return "Hello "+ message; + } + } + """; + + List problems = reconcile("A.java", source, false); + + assertEquals(1, problems.size()); + + ReconcileProblem problem = problems.get(0); + + assertEquals(Boot2JavaProblemType.WEB_ANNOTATION_NAMES, problem.getType()); + + String markedStr = source.substring(problem.getOffset(), problem.getOffset() + problem.getLength()); + assertEquals("@PathVariable(\"message\")", markedStr); + + assertEquals(3, problem.getQuickfixes().size()); + assertEquals("Remove Implicit Web Annotation Name", problems.get(0).getQuickfixes().get(0).title); + assertEquals("Remove Implicit Web Annotation Names in file", problems.get(0).getQuickfixes().get(1).title); + assertEquals("Remove Implicit Web Annotation Names in project", problems.get(0).getQuickfixes().get(2).title); + + } + + @Test + void webAnnotationWithDifferentNames() throws Exception { + String source = """ + package example.demo; + + import org.springframework.stereotype.Controller; + import org.springframework.web.bind.annotation.GetMapping; + import org.springframework.web.bind.annotation.PathVariable; + + @Controller + class A { + + @GetMapping("/hello/{msg}") + public String hello(@PathVariable("msg") String message) { + return "Hello "+ message; + } + } + """; + + List problems = reconcile("A.java", source, false); + + assertEquals(0, problems.size()); + + } + + @Test + void webAnnotationWithAssignment() throws Exception { + String source = """ + package example.demo; + + import org.springframework.stereotype.Controller; + import org.springframework.web.bind.annotation.GetMapping; + import org.springframework.web.bind.annotation.PathVariable; + + @Controller + class A { + + @GetMapping("/hello/{message}") + public String hello(@PathVariable(value="message") String message) { + return "Hello "+ message; + } + } + """; + + List problems = reconcile("A.java", source, false); + + assertEquals(1, problems.size()); + + ReconcileProblem problem = problems.get(0); + + assertEquals(Boot2JavaProblemType.WEB_ANNOTATION_NAMES, problem.getType()); + + String markedStr = source.substring(problem.getOffset(), problem.getOffset() + problem.getLength()); + assertEquals("@PathVariable(value=\"message\")", markedStr); + + assertEquals(3, problem.getQuickfixes().size()); + + } + + @Test + void webAnnotationWithMultipleParams() throws Exception { + String source = """ + package example.demo; + + import org.springframework.stereotype.Controller; + import org.springframework.web.bind.annotation.GetMapping; + import org.springframework.web.bind.annotation.RequestParam; + + @Controller + class A { + + @GetMapping("/hello/{message}") + public String hello(@RequestParam(name="message", defaultValue = "world") String message) { + return "Hello "+ message; + } + } + """; + + List problems = reconcile("A.java", source, false); + + assertEquals(1, problems.size()); + + ReconcileProblem problem = problems.get(0); + + assertEquals(Boot2JavaProblemType.WEB_ANNOTATION_NAMES, problem.getType()); + + String markedStr = source.substring(problem.getOffset(), problem.getOffset() + problem.getLength()); + assertEquals("@RequestParam(name=\"message\", defaultValue = \"world\")", markedStr); + + assertEquals(3, problem.getQuickfixes().size()); + + } + + @Test + void multipleWebAnnotations() throws Exception { + String source = """ + package example.demo; + + import org.springframework.stereotype.Controller; + import org.springframework.web.bind.annotation.GetMapping; + import org.springframework.web.bind.annotation.PathVariable; + + @Controller + class A { + + @GetMapping("/hello/{message}/{name}/{address}") + public String hello(@PathVariable(value="message", required=false) String message, @PathVariable(value="name") String name, @PathVariable(value="address", required=false) String location) { + return "Hello "+ message; + } + } + """; + + List problems = reconcile("A.java", source, false); + + assertEquals(2, problems.size()); + + ReconcileProblem problem1 = problems.get(0); + ReconcileProblem problem2 = problems.get(1); + + assertEquals(Boot2JavaProblemType.WEB_ANNOTATION_NAMES, problem1.getType()); + assertEquals(Boot2JavaProblemType.WEB_ANNOTATION_NAMES, problem2.getType()); + + String markedStr1 = source.substring(problem1.getOffset(), problem1.getOffset() + problem1.getLength()); + assertEquals("@PathVariable(value=\"message\", required=false)", markedStr1); + + String markedStr2 = source.substring(problem2.getOffset(), problem2.getOffset() + problem2.getLength()); + assertEquals("@PathVariable(value=\"name\")", markedStr2); + + assertEquals(3, problem1.getQuickfixes().size()); + assertEquals(3, problem2.getQuickfixes().size()); + + } + +} diff --git a/vscode-extensions/vscode-spring-boot/package.json b/vscode-extensions/vscode-spring-boot/package.json index a6f1047f7..12fed5c4c 100644 --- a/vscode-extensions/vscode-spring-boot/package.json +++ b/vscode-extensions/vscode-spring-boot/package.json @@ -609,6 +609,18 @@ "HINT", "ERROR" ] + }, + "spring-boot.ls.problem.boot2.WEB_ANNOTATION_NAMES": { + "type": "string", + "default": "HINT", + "description": "Web annotation names are unnecessary when it is the same as method parameter name", + "enum": [ + "IGNORE", + "INFO", + "WARNING", + "HINT", + "ERROR" + ] } } },