From d1e9ed51eab0e528e563af9f1e0b75646e66e68d Mon Sep 17 00:00:00 2001
From: aboyko <aboyko@vmware.com>
Date: Tue, 26 Sep 2023 22:07:19 -0400
Subject: [PATCH] Quickfix sorting. Preferred fix. Wording for RequestMapping
 quickfixes

---
 .../plugin.xml                                | 16 ++++
 .../boot/ls/BootLanguageServerPlugin.java     | 11 +++
 .../ls/markers/DelegateMarkerResolution.java  | 84 +++++++++++++++++++
 .../LSPBootCodeActionMarkerResolution.java    | 30 +++++++
 .../languageserver/quickfix/Quickfix.java     | 11 ++-
 .../util/SimpleLanguageServer.java            |  1 +
 .../commons/rewrite/java/FixDescriptor.java   | 11 +++
 .../NoRequestMappingAnnotationReconciler.java | 21 +++--
 .../boot/java/reconcilers/ReconcileUtils.java |  2 +-
 ...equestMappingAnnotationReconcilerTest.java | 16 ++--
 10 files changed, 183 insertions(+), 20 deletions(-)
 create mode 100644 eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/markers/DelegateMarkerResolution.java
 create mode 100644 eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/markers/LSPBootCodeActionMarkerResolution.java

diff --git a/eclipse-language-servers/org.springframework.tooling.boot.ls/plugin.xml b/eclipse-language-servers/org.springframework.tooling.boot.ls/plugin.xml
index b90c73d6e..9942e4a07 100644
--- a/eclipse-language-servers/org.springframework.tooling.boot.ls/plugin.xml
+++ b/eclipse-language-servers/org.springframework.tooling.boot.ls/plugin.xml
@@ -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>
@@ -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">
diff --git a/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/BootLanguageServerPlugin.java b/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/BootLanguageServerPlugin.java
index e68b55c6b..fa1315de8 100644
--- a/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/BootLanguageServerPlugin.java
+++ b/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/BootLanguageServerPlugin.java
@@ -17,6 +17,7 @@ import java.util.List;
 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;
@@ -32,6 +33,8 @@ import org.springframework.tooling.boot.ls.prefs.LiveInformationPreferencePage;
  */
 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";
@@ -108,4 +111,12 @@ public class BootLanguageServerPlugin extends AbstractUIPlugin {
 			});
 		}
 	}
+
+	@Override
+	protected void initializeImageRegistry(ImageRegistry reg) {
+		super.initializeImageRegistry(reg);
+		reg.put(SPRING_ICON, imageDescriptorFromPlugin(PLUGIN_ID, "icons/spring_obj.gif"));
+	}
+
+	
 }
diff --git a/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/markers/DelegateMarkerResolution.java b/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/markers/DelegateMarkerResolution.java
new file mode 100644
index 000000000..57ac5bf9f
--- /dev/null
+++ b/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/markers/DelegateMarkerResolution.java
@@ -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;
+	}
+
+}
diff --git a/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/markers/LSPBootCodeActionMarkerResolution.java b/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/markers/LSPBootCodeActionMarkerResolution.java
new file mode 100644
index 000000000..f9282d944
--- /dev/null
+++ b/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/markers/LSPBootCodeActionMarkerResolution.java
@@ -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);
+	}
+
+}
diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/quickfix/Quickfix.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/quickfix/Quickfix.java
index 79d6ae0ba..238a364ea 100644
--- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/quickfix/Quickfix.java
+++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/quickfix/Quickfix.java
@@ -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
@@ -30,13 +30,20 @@ public class Quickfix<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;
diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java
index 19afa6c80..393706cac 100644
--- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java
+++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java
@@ -804,6 +804,7 @@ public final class SimpleLanguageServer implements Sts4LanguageServer, SpringInd
 								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,
diff --git a/headless-services/commons/commons-rewrite/src/main/java/org/springframework/ide/vscode/commons/rewrite/java/FixDescriptor.java b/headless-services/commons/commons-rewrite/src/main/java/org/springframework/ide/vscode/commons/rewrite/java/FixDescriptor.java
index 17079fcc5..e89138651 100644
--- a/headless-services/commons/commons-rewrite/src/main/java/org/springframework/ide/vscode/commons/rewrite/java/FixDescriptor.java
+++ b/headless-services/commons/commons-rewrite/src/main/java/org/springframework/ide/vscode/commons/rewrite/java/FixDescriptor.java
@@ -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;
@@ -59,6 +61,11 @@ final public class FixDescriptor {
 		return this;
 	}
 	
+	public FixDescriptor withPreferred(boolean preferred) {
+		this.preferred = preferred;
+		return this;
+	}
+	
 	public String getRecipeId() {
 		return recipeId;
 	}
@@ -86,5 +93,9 @@ final public class FixDescriptor {
 	public String[] getTypeStubs() {
 		return typeStubs;
 	}
+	
+	public boolean isPreferred() {
+		return preferred;
+	}
 
 }
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 0072ca194..8f9a7b213 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
@@ -13,6 +13,7 @@ package org.springframework.ide.vscode.boot.java.reconcilers;
 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;
@@ -45,11 +46,11 @@ import org.springframework.ide.vscode.commons.rewrite.java.NoRequestMappingAnnot
 
 public class NoRequestMappingAnnotationReconciler implements JdtAstReconciler {
 	
+	private static final String PROBLEM_LABEL = "Replace with precise mapping annotation";
+	
 	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 List<String> SUPPORTED_REQUEST_METHODS = List.of("GET", "POST", "PUT", "DELETE", "PATCH");
 	
 	private QuickfixRegistry registry;
 
@@ -87,17 +88,19 @@ public class NoRequestMappingAnnotationReconciler implements JdtAstReconciler {
 						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);
@@ -110,7 +113,7 @@ public class NoRequestMappingAnnotationReconciler implements JdtAstReconciler {
 	}
 	
 	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);
diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/ReconcileUtils.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/ReconcileUtils.java
index b46a3ac2a..bbc8ad7b1 100644
--- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/ReconcileUtils.java
+++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/ReconcileUtils.java
@@ -59,7 +59,7 @@ public class ReconcileUtils {
 	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()));
 		}
 	}
 	
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 acd047003..c84bff80d 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
@@ -81,7 +81,7 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest
 		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);
 		
 	}
 
@@ -116,7 +116,7 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest
 		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);
 		
 	}
 	
@@ -151,7 +151,7 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest
 		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);
 		
 	}
 	
@@ -231,11 +231,11 @@ public class NoRequestMappingAnnotationReconcilerTest extends BaseReconcilerTest
 		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