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
This commit is contained in:
@@ -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());
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<String> 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<MemberValuePair>) ((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;
|
||||
}
|
||||
|
||||
}
|
||||
@@ -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"
|
||||
}
|
||||
]
|
||||
},
|
||||
|
||||
@@ -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<ReconcileProblem> 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<ReconcileProblem> 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<ReconcileProblem> 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<ReconcileProblem> 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<ReconcileProblem> 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());
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
@@ -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"
|
||||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user