Multiple quick fixes for @RequestMapping

This commit is contained in:
aboyko
2023-09-15 15:02:58 -04:00
parent 6de7307c84
commit 78cd6ace6d
6 changed files with 370 additions and 48 deletions

View File

@@ -0,0 +1,149 @@
/*******************************************************************************
* Copyright (c) 2023 VMware, Inc.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* VMware, Inc. - initial API and implementation
*******************************************************************************/
package org.springframework.ide.vscode.commons.rewrite.java;
import java.util.Optional;
import org.openrewrite.ExecutionContext;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.AnnotationMatcher;
import org.openrewrite.java.ChangeType;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.Space;
import org.openrewrite.marker.Range;
public class NoRequestMappingAnnotation extends org.openrewrite.java.spring.NoRequestMappingAnnotation implements RangeScopedRecipe {
private static final AnnotationMatcher REQUEST_MAPPING_ANNOTATION_MATCHER = new AnnotationMatcher(
"@org.springframework.web.bind.annotation.RequestMapping");
private String preferredMapping;
private Range range;
public String getPreferredMapping() {
return preferredMapping;
}
public void setPreferredMapping(String preferredMapping) {
this.preferredMapping = preferredMapping;
}
@Override
public void setRange(Range range) {
this.range = range;
}
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new RangeScopedJavaIsoVisitor<ExecutionContext>(range) {
@Override
public J.Annotation visitAnnotation(J.Annotation annotation, ExecutionContext ctx) {
J.Annotation a = super.visitAnnotation(annotation, ctx);
if (REQUEST_MAPPING_ANNOTATION_MATCHER.matches(a)
&& getCursor().getParentOrThrow().getValue() instanceof J.MethodDeclaration) {
Optional<J.Assignment> requestMethodArg = requestMethodArgument(a);
Optional<String> requestType = requestMethodArg.map(this::requestMethodType).or(() -> Optional.ofNullable(preferredMapping));
String resolvedRequestMappingAnnotationClassName = requestType.map(NoRequestMappingAnnotation::associatedRequestMapping)
.orElse(null);
if (resolvedRequestMappingAnnotationClassName == null) {
// Without a method argument @RequestMapping matches all request methods, so we
// can't safely convert
return a;
}
maybeRemoveImport("org.springframework.web.bind.annotation.RequestMapping");
maybeRemoveImport("org.springframework.web.bind.annotation.RequestMethod");
requestType.ifPresent(requestMethod -> maybeRemoveImport(
"org.springframework.web.bind.annotation.RequestMethod." + requestMethod));
// Remove the argument
if (requestMethodArg.isPresent() && resolvedRequestMappingAnnotationClassName != null) {
if (a.getArguments() != null) {
a = a.withArguments(ListUtils.map(a.getArguments(),
arg -> requestMethodArg.get().equals(arg) ? null : arg));
}
}
// Change the Annotation Type
if (resolvedRequestMappingAnnotationClassName != null) {
maybeAddImport(
"org.springframework.web.bind.annotation." + resolvedRequestMappingAnnotationClassName);
a = (J.Annotation) new ChangeType("org.springframework.web.bind.annotation.RequestMapping",
"org.springframework.web.bind.annotation." + resolvedRequestMappingAnnotationClassName,
false).getVisitor().visit(a, ctx, getCursor());
}
// if there is only one remaining argument now, and it is "path" or "value",
// then we can drop the key name
if (a != null && a.getArguments() != null && a.getArguments().size() == 1) {
a = a.withArguments(ListUtils.map(a.getArguments(), arg -> {
if (arg instanceof J.Assignment && ((J.Assignment) arg).getVariable() instanceof J.Identifier) {
J.Identifier ident = (J.Identifier) ((J.Assignment) arg).getVariable();
if ("path".equals(ident.getSimpleName()) || "value".equals(ident.getSimpleName())) {
return ((J.Assignment) arg).getAssignment().withPrefix(Space.EMPTY);
}
}
return arg;
}));
}
}
return a != null ? a : annotation;
}
private Optional<J.Assignment> requestMethodArgument(J.Annotation annotation) {
return annotation.getArguments().stream()
.filter(arg -> arg instanceof J.Assignment
&& ((J.Assignment) arg).getVariable() instanceof J.Identifier
&& "method".equals(((J.Identifier) ((J.Assignment) arg).getVariable()).getSimpleName()))
.map(J.Assignment.class::cast).findFirst();
}
private String requestMethodType(@Nullable J.Assignment assignment) {
if (assignment.getAssignment() instanceof J.Identifier) {
return ((J.Identifier) assignment.getAssignment()).getSimpleName();
} else if (assignment.getAssignment() instanceof J.FieldAccess) {
return ((J.FieldAccess) assignment.getAssignment()).getSimpleName();
} else if (assignment.getAssignment() instanceof J.NewArray) {
J.NewArray newArray = (J.NewArray) assignment.getAssignment();
if (newArray.getInitializer() != null && newArray.getInitializer().size() > 0) {
if (preferredMapping != null && !preferredMapping.isBlank()) {
return newArray.getInitializer().stream().filter(J.FieldAccess.class::isInstance)
.map(J.FieldAccess.class::cast).map(fa -> fa.getSimpleName())
.filter(n -> preferredMapping.equals(n)).findFirst().orElse(null);
} else if (newArray.getInitializer().size() == 1) {
return ((J.FieldAccess) newArray.getInitializer().get(0)).getSimpleName();
}
}
}
return null;
}
};
}
@Nullable
public static String associatedRequestMapping(String method) {
switch (method) {
case "POST":
case "PUT":
case "DELETE":
case "PATCH":
case "GET":
return method.charAt(0) + method.toLowerCase().substring(1) + "Mapping";
}
// HEAD, OPTIONS, TRACE do not have associated RequestMapping variant
return null;
}
}

View File

@@ -0,0 +1,41 @@
/*******************************************************************************
* Copyright (c) 2023 VMware, Inc.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* VMware, Inc. - initial API and implementation
*******************************************************************************/
package org.springframework.ide.vscode.commons.rewrite.java;
import org.openrewrite.Tree;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.tree.J;
import org.openrewrite.marker.Range;
public class RangeScopedJavaIsoVisitor<P> extends JavaIsoVisitor<P> {
private Range range;
public RangeScopedJavaIsoVisitor(Range range) {
this.range = range;
}
@Override
public @Nullable J visit(@Nullable Tree tree, P ctx) {
if (tree instanceof J) {
J t = (J) tree;
Range r = t.getMarkers().findFirst(Range.class).orElse(null);
if (range == null || r == null || r.getStart().getOffset() <= range.getStart().getOffset() && range.getEnd().getOffset() - 1 <= r.getEnd().getOffset()) {
return (J) super.visit(t, ctx);
} else {
return t;
}
}
return super.visit(tree, ctx);
}
}

View File

@@ -0,0 +1,19 @@
/*******************************************************************************
* Copyright (c) 2023 VMware, Inc.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* VMware, Inc. - initial API and implementation
*******************************************************************************/
package org.springframework.ide.vscode.commons.rewrite.java;
import org.openrewrite.marker.Range;
public interface RangeScopedRecipe {
void setRange(Range range);
}

View File

@@ -13,19 +13,25 @@ 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.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
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.Expression;
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.QualifiedName;
import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.core.dom.SingleMemberAnnotation;
import org.openrewrite.java.spring.NoRequestMappingAnnotation;
import org.openrewrite.marker.Range;
import org.springframework.ide.vscode.boot.java.Annotations;
import org.springframework.ide.vscode.boot.java.Boot2JavaProblemType;
import org.springframework.ide.vscode.commons.java.IJavaProject;
@@ -35,11 +41,15 @@ import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemTy
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;
import org.springframework.ide.vscode.commons.rewrite.java.NoRequestMappingAnnotation;
public class NoRequestMappingAnnotationReconciler implements JdtAstReconciler {
private static final String UNSUPPORTED_REQUEST_METHOD = "UNSUPPORTED";
private static final List<String> SUPPORTED_REQUEST_METHODS = List.of("GET", "POST", "PUT", "DELETE", "PATCH");
private static final String LABEL = "Replace @RequestMapping with specific @GetMapping, @PostMapping etc.";
private static final String ID = NoRequestMappingAnnotation.class.getName();
private QuickfixRegistry registry;
@@ -71,58 +81,83 @@ public class NoRequestMappingAnnotationReconciler implements JdtAstReconciler {
}
private void processAnnotation(Annotation a) {
if (a.getParent() instanceof MethodDeclaration && isRequestMappingAnnotation(cu, a)) {
String uri = docUri.toASCIIString();
ReconcileProblemImpl problem = new ReconcileProblemImpl(getProblemType(), LABEL, a.getStartPosition(), a.getLength());
ReconcileUtils.setRewriteFixes(registry, problem, List.of(
// new FixDescriptor(ID, List.of(uri), RecipeCodeActionDescriptor.buildLabel(LABEL, RecipeScope.NODE))
// .withRangeScope(RewriteQuickFixUtils.createOpenRewriteRange(cu, a))
// .withRecipeScope(RecipeScope.NODE),
new FixDescriptor(ID, List.of(uri), ReconcileUtils.buildLabel(LABEL, RecipeScope.FILE))
.withRecipeScope(RecipeScope.FILE),
new FixDescriptor(ID, List.of(uri), ReconcileUtils.buildLabel(LABEL, RecipeScope.PROJECT))
.withRecipeScope(RecipeScope.PROJECT)
));
problemCollector.accept(problem);
if (a.getParent() instanceof MethodDeclaration) {
List<String> requestMethods = getRequestMethods(cu, a);
if (!requestMethods.isEmpty() && !requestMethods.contains(UNSUPPORTED_REQUEST_METHOD)) {
String uri = docUri.toASCIIString();
Range range = ReconcileUtils.createOpenRewriteRange(cu, a);
if (requestMethods.size() == 1 && SUPPORTED_REQUEST_METHODS.contains(requestMethods.get(0))) {
ReconcileProblemImpl problem = new ReconcileProblemImpl(getProblemType(), LABEL, a.getStartPosition(), a.getLength());
ReconcileUtils.setRewriteFixes(registry, problem, List.of(
createFixDescriptor(uri, range, requestMethods.get(0)),
new FixDescriptor(org.openrewrite.java.spring.NoRequestMappingAnnotation.class.getName(), List.of(uri), ReconcileUtils.buildLabel(LABEL, RecipeScope.FILE))
.withRecipeScope(RecipeScope.FILE),
new FixDescriptor(org.openrewrite.java.spring.NoRequestMappingAnnotation.class.getName(), List.of(uri), ReconcileUtils.buildLabel(LABEL, RecipeScope.PROJECT))
.withRecipeScope(RecipeScope.PROJECT)
));
problemCollector.accept(problem);
} else if (SUPPORTED_REQUEST_METHODS == requestMethods) { // the case of no request methods specified
ReconcileProblemImpl problem = new ReconcileProblemImpl(getProblemType(), "Consider replacing with precise mapping annotation", a.getStartPosition(), a.getLength());
ReconcileUtils.setRewriteFixes(registry, problem,
requestMethods.stream().map(m -> createFixDescriptor(uri, range, m)).collect(Collectors.toList()));
problemCollector.accept(problem);
}
}
}
}
});
}
private static boolean isRequestMappingAnnotation(CompilationUnit cu, Annotation a) {
// 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 FixDescriptor createFixDescriptor(String uri, Range range, String requestMethod) {
return new FixDescriptor(NoRequestMappingAnnotation.class.getName(), List.of(uri), "Replace with '@%s'".formatted(NoRequestMappingAnnotation.associatedRequestMapping(requestMethod)))
.withRangeScope(range)
.withParameters(Map.of("preferredMapping", requestMethod))
.withRecipeScope(RecipeScope.NODE);
}
private static boolean hasApplicableMethodParameter(Annotation a) {
private static List<String> getRequestMethods(CompilationUnit cu, Annotation a) {
String typeName = a.getTypeName().getFullyQualifiedName();
if (Annotations.SPRING_REQUEST_MAPPING.equals(typeName)) {
return getRequestMethods(a);
} else if (typeName.endsWith("RequestMapping")) {
ITypeBinding type = a.resolveTypeBinding();
if (type != null && Annotations.SPRING_REQUEST_MAPPING.equals(type.getQualifiedName())) {
return getRequestMethods(a);
}
}
return Collections.emptyList();
}
@SuppressWarnings("unchecked")
private static List<String> getRequestMethods(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;
List<Expression> expressions = ((ArrayInitializer) pair.getValue()).expressions();
return expressions.stream().map(NoRequestMappingAnnotationReconciler::convertRequestMethodToString).collect(Collectors.toList());
} else {
return List.of(convertRequestMethodToString(pair.getValue()));
}
return true;
}
}
}
}
return false;
return SUPPORTED_REQUEST_METHODS;
}
private static String convertRequestMethodToString(Expression e) {
String requestMethod = UNSUPPORTED_REQUEST_METHOD;
if (e instanceof QualifiedName) {
requestMethod = ((QualifiedName) e).getName().getIdentifier();
} else if (e instanceof SimpleName) {
requestMethod = ((SimpleName) e).getIdentifier();
}
return SUPPORTED_REQUEST_METHODS.contains(requestMethod) ? requestMethod : UNSUPPORTED_REQUEST_METHOD;
}
@Override
public boolean isApplicable(IJavaProject project) {

View File

@@ -51,6 +51,7 @@ import org.springframework.ide.vscode.commons.rewrite.ORDocUtils;
import org.springframework.ide.vscode.commons.rewrite.config.RecipeScope;
import org.springframework.ide.vscode.commons.rewrite.java.FixDescriptor;
import org.springframework.ide.vscode.commons.rewrite.java.ORAstUtils;
import org.springframework.ide.vscode.commons.rewrite.java.RangeScopedRecipe;
import org.springframework.ide.vscode.commons.util.text.LanguageId;
import org.springframework.ide.vscode.commons.util.text.TextDocument;
@@ -204,16 +205,20 @@ public class RewriteRefactorings implements CodeActionResolver, QuickfixHandler
if (d.getRangeScope() == null) {
throw new IllegalArgumentException("Missing scope AST node!");
} else {
r = ORAstUtils.nodeRecipe(r, j -> {
if (j != null) {
Range range = j.getMarkers().findFirst(Range.class).orElse(null);
if (range != null) {
// Rewrite range end offset is up to not including hence -1
return d.getRangeScope().getStart().getOffset() <= range.getStart().getOffset() && range.getEnd().getOffset() - 1 <= d.getRangeScope().getEnd().getOffset();
}
}
return false;
});
if (r instanceof RangeScopedRecipe) {
((RangeScopedRecipe) r).setRange(d.getRangeScope());
} else {
r = ORAstUtils.nodeRecipe(r, j -> {
if (j != null) {
Range range = j.getMarkers().findFirst(Range.class).orElse(null);
if (range != null) {
// Rewrite range end offset is up to not including hence -1
return d.getRangeScope().getStart().getOffset() <= range.getStart().getOffset() && range.getEnd().getOffset() - 1 <= d.getRangeScope().getEnd().getOffset();
}
}
return false;
});
}
}
}
return r;

View File

@@ -80,10 +80,46 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest
String markedStr = source.substring(problem.getOffset(), problem.getOffset() + problem.getLength());
assertEquals("@RequestMapping(value = \"/1\", method = RequestMethod.GET)", markedStr);
assertEquals(2, problem.getQuickfixes().size());
assertEquals(3, problem.getQuickfixes().size());
assertEquals("Replace with '@GetMapping'", problems.get(0).getQuickfixes().get(0).title);
}
@Test
void staticImport() throws Exception {
String source = """
package example.demo;
import org.springframework.web.bind.annotation.RequestMapping;
import static org.springframework.web.bind.annotation.RequestMethod.*;
@RequestMapping("/hello")
class A {
@RequestMapping(value = "/1", method = GET)
String hello1() {
return "1";
};
}
""";
List<ReconcileProblem> 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 = GET)", markedStr);
assertEquals(3, problem.getQuickfixes().size());
assertEquals("Replace with '@GetMapping'", problems.get(0).getQuickfixes().get(0).title);
}
@Test
void arrayMethodTest() throws Exception {
String source = """
@@ -114,7 +150,8 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest
String markedStr = source.substring(problem.getOffset(), problem.getOffset() + problem.getLength());
assertEquals("@RequestMapping(value = \"/1\", method = { RequestMethod.GET })", markedStr);
assertEquals(2, problem.getQuickfixes().size());
assertEquals(3, problem.getQuickfixes().size());
assertEquals("Replace with '@GetMapping'", problems.get(0).getQuickfixes().get(0).title);
}
@@ -143,7 +180,36 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest
}
@Test
void noMethodsNoProblems() throws Exception {
void multiMethodTest_2() 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.PUT })
String hello1() {
return "1";
};
}
""";
List<ReconcileProblem> problems = reconcile("A.java", source, false);
assertEquals(0, problems.size());
// assertEquals(1, problems.size());
//
// assertEquals(2, problems.get(0).getQuickfixes().size());
// assertEquals("Replace with '@GetMapping'", problems.get(0).getQuickfixes().get(0).title);
// assertEquals("Replace with '@PutMapping'", problems.get(0).getQuickfixes().get(1).title);
}
@Test
void noMethods() throws Exception {
String source = """
package example.demo;
@@ -162,7 +228,14 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest
""";
List<ReconcileProblem> problems = reconcile("A.java", source, false);
assertEquals(0, problems.size());
assertEquals(1, problems.size());
assertEquals(5, problems.get(0).getQuickfixes().size());
assertEquals("Replace with '@GetMapping'", problems.get(0).getQuickfixes().get(0).title);
assertEquals("Replace with '@PostMapping'", problems.get(0).getQuickfixes().get(1).title);
assertEquals("Replace with '@PutMapping'", problems.get(0).getQuickfixes().get(2).title);
assertEquals("Replace with '@DeleteMapping'", problems.get(0).getQuickfixes().get(3).title);
assertEquals("Replace with '@PatchMapping'", problems.get(0).getQuickfixes().get(4).title);
}
@Test