Skip to content

Commit

Permalink
Quickfix sorting. Preferred fix. Wording for RequestMapping quickfixes
Browse files Browse the repository at this point in the history
  • Loading branch information
BoykoAlex committed Sep 27, 2023
1 parent e41f45d commit d1e9ed5
Show file tree
Hide file tree
Showing 10 changed files with 183 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
id="org.eclipse.languageserver.languages.springboot"
label="Spring Boot Language Server"
lastDocumentDisconnectedTimeout="2147483647"
markerType="org.springframework.tooling.boot.ls.diagnostic"
serverInterface="org.springframework.ide.vscode.commons.protocol.spring.SpringIndexLanguageServer"
singleton="true">
</server>
Expand Down Expand Up @@ -752,6 +753,21 @@
class="org.springframework.tooling.boot.ls.Startup">
</startup>
</extension>
<extension
point="org.eclipse.ui.ide.markerResolution">
<markerResolutionGenerator
class="org.springframework.tooling.boot.ls.markers.LSPBootCodeActionMarkerResolution"
markerType="org.springframework.tooling.boot.ls.diagnostic">
</markerResolutionGenerator>
</extension>
<extension
id="diagnostic"
name="Spring Boot"
point="org.eclipse.core.resources.markers">
<super
type="org.eclipse.core.resources.problemmarker">
</super>
</extension>
<!--
<extension
point="org.eclipse.wildwebdeveloper.xml.lemminxExtension">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.eclipse.core.runtime.preferences.IEclipsePreferences;
import org.eclipse.core.runtime.preferences.InstanceScope;
import org.eclipse.jface.bindings.Binding;
import org.eclipse.jface.resource.ImageRegistry;
import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.keys.IBindingService;
import org.eclipse.ui.plugin.AbstractUIPlugin;
Expand All @@ -32,6 +33,8 @@
*/
public class BootLanguageServerPlugin extends AbstractUIPlugin {

public static final String SPRING_ICON = "SPRING_ICON";

public static String PLUGIN_ID = "org.springframework.tooling.boot.ls";

private static final Object LSP4E_COMMAND_SYMBOL_IN_WORKSPACE = "org.eclipse.lsp4e.symbolinworkspace";
Expand Down Expand Up @@ -108,4 +111,12 @@ public void run() {
});
}
}

@Override
protected void initializeImageRegistry(ImageRegistry reg) {
super.initializeImageRegistry(reg);
reg.put(SPRING_ICON, imageDescriptorFromPlugin(PLUGIN_ID, "icons/spring_obj.gif"));
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*******************************************************************************
* 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.tooling.boot.ls.markers;

import org.eclipse.core.resources.IMarker;
import org.eclipse.jdt.ui.text.java.IJavaCompletionProposal;
import org.eclipse.jface.text.IDocument;
import org.eclipse.jface.text.contentassist.IContextInformation;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.Point;
import org.eclipse.ui.IMarkerResolution;
import org.eclipse.ui.IMarkerResolutionRelevance;
import org.springframework.tooling.boot.ls.BootLanguageServerPlugin;

public class DelegateMarkerResolution implements IMarkerResolution, IMarkerResolutionRelevance, IJavaCompletionProposal {

final private IMarkerResolution delegate;

final private int relevance;

public DelegateMarkerResolution(IMarkerResolution res, int relevance) {
this.delegate = res;
this.relevance = relevance;
}

@Override
public String getLabel() {
return delegate.getLabel();
}

@Override
public void run(IMarker marker) {
delegate.run(marker);
}

@Override
public void apply(IDocument document) {
throw new UnsupportedOperationException();
}

@Override
public Point getSelection(IDocument document) {
throw new UnsupportedOperationException();
}

@Override
public String getAdditionalProposalInfo() {
throw new UnsupportedOperationException();
}

@Override
public String getDisplayString() {
return getLabel();
}

@Override
public Image getImage() {
return BootLanguageServerPlugin.getDefault().getImageRegistry().get(BootLanguageServerPlugin.SPRING_ICON);
}

@Override
public IContextInformation getContextInformation() {
throw new UnsupportedOperationException();
}

@Override
public int getRelevance() {
return relevance;
}

@Override
public int getRelevanceForResolution() {
return relevance;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*******************************************************************************
* 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.tooling.boot.ls.markers;

import java.util.Arrays;
import java.util.concurrent.atomic.AtomicInteger;

import org.eclipse.core.resources.IMarker;
import org.eclipse.lsp4e.operations.codeactions.LSPCodeActionMarkerResolution;
import org.eclipse.ui.IMarkerResolution;

public class LSPBootCodeActionMarkerResolution extends LSPCodeActionMarkerResolution {

@Override
public IMarkerResolution[] getResolutions(IMarker marker) {
IMarkerResolution[] res = super.getResolutions(marker);
AtomicInteger relevance = new AtomicInteger(res.length);
return Arrays.stream(res).map(r -> new DelegateMarkerResolution(r, relevance.getAndDecrement()))
.toArray(IMarkerResolution[]::new);
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2017 Pivotal, Inc.
* Copyright (c) 2017, 2023 Pivotal, 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
Expand Down Expand Up @@ -30,13 +30,20 @@ public static class QuickfixData<T> {
public final QuickfixType type;
public final T params;
public final String title;
public final boolean preferred;

public QuickfixData(QuickfixType type, T params, String title) {
public QuickfixData(QuickfixType type, T params, String title, boolean preferred) {
super();
this.type = type;
this.params = params;
this.title = title;
this.preferred = preferred;
}

public QuickfixData(QuickfixType type, T params, String title) {
this(type, params, title, false);
}

}

private final String CODE_ACTION_CMD_ID;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,7 @@ public void accept(ReconcileProblem problem) {
CodeAction ca = new CodeAction();
ca.setKind(CodeActionKind.QuickFix);
ca.setTitle(fix.title);
ca.setIsPreferred(fix.preferred);
ca.setDiagnostics(List.of(refDiagnostic));
ca.setCommand(new Command(
fix.title,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ final public class FixDescriptor {

private String[] typeStubs;

private boolean preferred;

public FixDescriptor(String recipeId, List<String> docUris, String label) {
this.recipeId = recipeId;
this.docUris = docUris;
Expand Down Expand Up @@ -59,6 +61,11 @@ public FixDescriptor withTypeStubs(String... typeStubs) {
return this;
}

public FixDescriptor withPreferred(boolean preferred) {
this.preferred = preferred;
return this;
}

public String getRecipeId() {
return recipeId;
}
Expand Down Expand Up @@ -86,5 +93,9 @@ public String getLabel() {
public String[] getTypeStubs() {
return typeStubs;
}

public boolean isPreferred() {
return preferred;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import static org.springframework.ide.vscode.commons.java.SpringProjectUtil.springBootVersionGreaterOrEqual;

import java.net.URI;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -45,11 +46,11 @@

public class NoRequestMappingAnnotationReconciler implements JdtAstReconciler {

private static final String UNSUPPORTED_REQUEST_METHOD = "UNSUPPORTED";
private static final String PROBLEM_LABEL = "Replace with precise mapping annotation";

private static final List<String> SUPPORTED_REQUEST_METHODS = List.of("GET", "POST", "PUT", "DELETE", "PATCH");
private static final String UNSUPPORTED_REQUEST_METHOD = "UNSUPPORTED";

private static final String LABEL = "Replace @RequestMapping with specific @GetMapping, @PostMapping etc.";
private static final List<String> SUPPORTED_REQUEST_METHODS = List.of("GET", "POST", "PUT", "DELETE", "PATCH");

private QuickfixRegistry registry;

Expand Down Expand Up @@ -87,17 +88,19 @@ private void processAnnotation(Annotation a) {
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());
ReconcileProblemImpl problem = new ReconcileProblemImpl(getProblemType(), PROBLEM_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))
createFixDescriptor(uri, range, requestMethods.get(0)).withPreferred(true),
new FixDescriptor(org.openrewrite.java.spring.NoRequestMappingAnnotation.class.getName(), List.of(uri),
"Replace all `@RequestMapping` in file '%s' with `@GetMapping`, `@PostMapping`, etc.".formatted(Paths.get(docUri).getFileName().toString()))
.withRecipeScope(RecipeScope.FILE),
new FixDescriptor(org.openrewrite.java.spring.NoRequestMappingAnnotation.class.getName(), List.of(uri), ReconcileUtils.buildLabel(LABEL, RecipeScope.PROJECT))
new FixDescriptor(org.openrewrite.java.spring.NoRequestMappingAnnotation.class.getName(), List.of(uri),
"Replace all `@RequestMapping` in project '%s' with `@GetMapping`, `@PostMapping`, etc.".formatted(project.getElementName()))
.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());
ReconcileProblemImpl problem = new ReconcileProblemImpl(getProblemType(), PROBLEM_LABEL, a.getStartPosition(), a.getLength());
ReconcileUtils.setRewriteFixes(registry, problem,
requestMethods.stream().map(m -> createFixDescriptor(uri, range, m)).collect(Collectors.toList()));
problemCollector.accept(problem);
Expand All @@ -110,7 +113,7 @@ private void processAnnotation(Annotation a) {
}

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)))
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public static QuickfixType getRewriteQuickFixType(QuickfixRegistry registry) {
public static void setRewriteFixes(QuickfixRegistry registry, ReconcileProblemImpl problem, Collection<FixDescriptor> fixDescritptors) {
QuickfixType quickFixType = getRewriteQuickFixType(registry);
for (FixDescriptor f : fixDescritptors) {
problem.addQuickfix(new QuickfixData<>(quickFixType, f, f.getLabel()));
problem.addQuickfix(new QuickfixData<>(quickFixType, f, f.getLabel(), f.isPreferred()));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ String hello1() {
assertEquals("@RequestMapping(value = \"/1\", method = RequestMethod.GET)", markedStr);

assertEquals(3, problem.getQuickfixes().size());
assertEquals("Replace with '@GetMapping'", problems.get(0).getQuickfixes().get(0).title);
assertEquals("Replace with `@GetMapping`", problems.get(0).getQuickfixes().get(0).title);

}

Expand Down Expand Up @@ -116,7 +116,7 @@ String hello1() {
assertEquals("@RequestMapping(value = \"/1\", method = GET)", markedStr);

assertEquals(3, problem.getQuickfixes().size());
assertEquals("Replace with '@GetMapping'", problems.get(0).getQuickfixes().get(0).title);
assertEquals("Replace with `@GetMapping`", problems.get(0).getQuickfixes().get(0).title);

}

Expand Down Expand Up @@ -151,7 +151,7 @@ String hello1() {
assertEquals("@RequestMapping(value = \"/1\", method = { RequestMethod.GET })", markedStr);

assertEquals(3, problem.getQuickfixes().size());
assertEquals("Replace with '@GetMapping'", problems.get(0).getQuickfixes().get(0).title);
assertEquals("Replace with `@GetMapping`", problems.get(0).getQuickfixes().get(0).title);

}

Expand Down Expand Up @@ -231,11 +231,11 @@ String hello1() {
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);
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
Expand Down

0 comments on commit d1e9ed5

Please sign in to comment.