From 58c804cf4406b6ef89522b98b09c2bc76b9a14e2 Mon Sep 17 00:00:00 2001 From: Martin Lippert Date: Wed, 22 Nov 2023 15:25:46 +0100 Subject: [PATCH] GH-758: restrict request mapping snippet completions to classes that are annotated as a spring controller --- .../BootJavaCompletionEngineConfigurer.java | 75 ++++++++++++++++++- .../ide/vscode/boot/java/Annotations.java | 2 + .../annotations/AnnotationHierarchies.java | 2 +- .../java/snippets/JavaSnippetContext.java | 2 +- .../test/RequestMappingSnippetTests.java | 36 +++++---- .../java/example/SampleNonController.java | 8 ++ 6 files changed, 106 insertions(+), 19 deletions(-) create mode 100644 headless-services/spring-boot-language-server/src/test/resources/test-projects/test-request-mapping-completions/src/main/java/example/SampleNonController.java diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/app/BootJavaCompletionEngineConfigurer.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/app/BootJavaCompletionEngineConfigurer.java index 4f3d86624..4d0cc4c13 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/app/BootJavaCompletionEngineConfigurer.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/app/BootJavaCompletionEngineConfigurer.java @@ -10,14 +10,20 @@ *******************************************************************************/ package org.springframework.ide.vscode.boot.app; +import java.util.Collection; import java.util.HashMap; import java.util.Map; +import org.eclipse.jdt.core.dom.ASTNode; +import org.eclipse.jdt.core.dom.IAnnotationBinding; +import org.eclipse.jdt.core.dom.ITypeBinding; +import org.eclipse.jdt.core.dom.TypeDeclaration; import org.eclipse.lsp4j.CompletionItemKind; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.ide.vscode.boot.java.Annotations; +import org.springframework.ide.vscode.boot.java.annotations.AnnotationHierarchies; import org.springframework.ide.vscode.boot.java.data.DataRepositoryCompletionProcessor; import org.springframework.ide.vscode.boot.java.handlers.BootJavaCompletionEngine; import org.springframework.ide.vscode.boot.java.handlers.CompletionProvider; @@ -25,6 +31,7 @@ import org.springframework.ide.vscode.boot.java.scope.ScopeCompletionProcessor; import org.springframework.ide.vscode.boot.java.snippets.JavaSnippet; import org.springframework.ide.vscode.boot.java.snippets.JavaSnippetContext; import org.springframework.ide.vscode.boot.java.snippets.JavaSnippetManager; +import org.springframework.ide.vscode.boot.java.utils.ASTUtils; import org.springframework.ide.vscode.boot.java.utils.CompilationUnitCache; import org.springframework.ide.vscode.boot.java.value.ValueCompletionProcessor; import org.springframework.ide.vscode.boot.metadata.ProjectBasedPropertyIndexProvider; @@ -46,8 +53,13 @@ public class BootJavaCompletionEngineConfigurer { // STS3 bundle. Therefore do not include templates if client is Eclipse // TODO: REMOVE this check once STS3 is no longer supported if (LspClient.currentClient() != LspClient.Client.ECLIPSE) { + + JavaSnippetContext webControllerContext = new CompositeJavaSnippetContext( + JavaSnippetContext.AT_ROOT_LEVEL, + new AnnotatedTypeDeclarationContext(Annotations.CONTROLLER)); + snippetManager.add( - new JavaSnippet("@RequestMapping(..) {..}", JavaSnippetContext.AT_ROOT_LEVEL, CompletionItemKind.Method, + new JavaSnippet("@RequestMapping(..) {..}", webControllerContext, CompletionItemKind.Method, ImmutableList.of("org.springframework.web.bind.annotation.RequestMapping", "org.springframework.web.bind.annotation.RequestMethod", "org.springframework.web.bind.annotation.RequestParam"), @@ -56,7 +68,7 @@ public class BootJavaCompletionEngineConfigurer { + " return new ${SomeData}(${cursor});\n" + "}\n", "RequestMapping")); snippetManager.add( - new JavaSnippet("@GetMapping(..) {..}", JavaSnippetContext.AT_ROOT_LEVEL, CompletionItemKind.Method, + new JavaSnippet("@GetMapping(..) {..}", webControllerContext, CompletionItemKind.Method, ImmutableList.of("org.springframework.web.bind.annotation.GetMapping", "org.springframework.web.bind.annotation.RequestParam"), "@GetMapping(\"${path}\")\n" @@ -64,7 +76,7 @@ public class BootJavaCompletionEngineConfigurer { + " return new ${SomeData}(${cursor});\n" + "}\n", "GetMapping")); snippetManager.add( - new JavaSnippet("@PostMapping(..) {..}", JavaSnippetContext.AT_ROOT_LEVEL, CompletionItemKind.Method, + new JavaSnippet("@PostMapping(..) {..}", webControllerContext, CompletionItemKind.Method, ImmutableList.of("org.springframework.web.bind.annotation.PostMapping", "org.springframework.web.bind.annotation.RequestBody"), "@PostMapping(\"${path}\")\n" @@ -72,7 +84,7 @@ public class BootJavaCompletionEngineConfigurer { + " //TODO: process POST request\n" + " ${cursor}\n" + " return ${entity};\n" + "}\n", "PostMapping")); snippetManager.add( - new JavaSnippet("@PutMapping(..) {..}", JavaSnippetContext.AT_ROOT_LEVEL, CompletionItemKind.Method, + new JavaSnippet("@PutMapping(..) {..}", webControllerContext, CompletionItemKind.Method, ImmutableList.of("org.springframework.web.bind.annotation.PutMapping", "org.springframework.web.bind.annotation.RequestBody", "org.springframework.web.bind.annotation.PathVariable"), @@ -102,4 +114,59 @@ public class BootJavaCompletionEngineConfigurer { return new BootJavaCompletionEngine(cuCache, providers, snippetManager); } + /** + * checks if the type declaration that belongs to the node is annotated with the required annotation + */ + private class AnnotatedTypeDeclarationContext implements JavaSnippetContext { + + private final String requiredAnnotation; + + public AnnotatedTypeDeclarationContext(String requiredAnnotation) { + this.requiredAnnotation = requiredAnnotation; + } + + @Override + public boolean appliesTo(ASTNode node) { + TypeDeclaration type = ASTUtils.findDeclaringType(node); + if (type != null) { + ITypeBinding binding = type.resolveBinding(); + if (binding != null) { + IAnnotationBinding[] annotations = binding.getAnnotations(); + if (annotations != null) { + for (int i = 0; i < annotations.length; i++) { + ITypeBinding annotationType = annotations[i].getAnnotationType(); + if (AnnotationHierarchies.isMetaAnnotation(annotationType, (name) -> requiredAnnotation.equals(name))) { + return true; + } + } + } + } + } + return false; + } + } + + /** + * composite implementation for snippet contexts that allows you to combine multiple context checks + * The check only passes if ALL individual context checks are successful + */ + private class CompositeJavaSnippetContext implements JavaSnippetContext { + + private final JavaSnippetContext[] contexts; + + public CompositeJavaSnippetContext(JavaSnippetContext... contexts) { + this.contexts = contexts; + } + + @Override + public boolean appliesTo(ASTNode node) { + for (JavaSnippetContext context : contexts) { + if (!context.appliesTo(node)) { + return false; + } + } + return contexts.length > 0; + } + } + } diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Annotations.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Annotations.java index 07d2f4b2b..c5b1be8a1 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Annotations.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/Annotations.java @@ -23,6 +23,8 @@ public class Annotations { public static final String COMPONENT = "org.springframework.stereotype.Component"; public static final String CONFIGURATION = "org.springframework.context.annotation.Configuration"; + public static final String CONTROLLER = "org.springframework.stereotype.Controller"; + public static final String REPOSITORY = "org.springframework.stereotype.Repository"; public static final String REPOSITORY_DEFINITION = "org.springframework.data.repository.RepositoryDefinition"; public static final String NO_REPO_BEAN = "org.springframework.data.repository.NoRepositoryBean"; diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java index aede53c43..1afb324a3 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java @@ -118,7 +118,7 @@ public abstract class AnnotationHierarchies { } } - private static boolean isMetaAnnotation(ITypeBinding candidate, Predicate isKeyAnnotationName) { + public static boolean isMetaAnnotation(ITypeBinding candidate, Predicate isKeyAnnotationName) { return findTransitiveSupers(candidate, new HashSet<>()) .anyMatch(sa -> isKeyAnnotationName.test(sa.getQualifiedName())); } diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/snippets/JavaSnippetContext.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/snippets/JavaSnippetContext.java index a023f90a2..8ceebd97e 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/snippets/JavaSnippetContext.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/snippets/JavaSnippetContext.java @@ -15,12 +15,12 @@ import org.eclipse.jdt.core.dom.SimpleName; import org.eclipse.jdt.core.dom.TypeDeclaration; public interface JavaSnippetContext { + JavaSnippetContext BOOT_MEMBERS = (node) -> node instanceof TypeDeclaration || node instanceof SimpleName; JavaSnippetContext AT_ROOT_LEVEL = (node) -> { return (node instanceof TypeDeclaration) || (node instanceof SimpleName && node.getParent() != null && node.getParent() instanceof TypeDeclaration); }; - boolean appliesTo(ASTNode node); } diff --git a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/requestmapping/test/RequestMappingSnippetTests.java b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/requestmapping/test/RequestMappingSnippetTests.java index fb8e40bab..0e820ce84 100644 --- a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/requestmapping/test/RequestMappingSnippetTests.java +++ b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/requestmapping/test/RequestMappingSnippetTests.java @@ -10,14 +10,13 @@ *******************************************************************************/ package org.springframework.ide.vscode.boot.java.requestmapping.test; +import static org.junit.jupiter.api.Assertions.assertEquals; + import java.io.InputStream; import java.util.List; import org.apache.commons.io.IOUtils; - -import static org.junit.jupiter.api.Assertions.assertEquals; import org.eclipse.lsp4j.CompletionItem; -import org.eclipse.lsp4j.CompletionList; import org.eclipse.lsp4j.InsertTextMode; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -38,6 +37,9 @@ import org.springframework.test.context.junit.jupiter.SpringExtension; @Import(HoverTestConf.class) public class RequestMappingSnippetTests { + private static final String CONTROLLER_CLASSNAME = "SampleController"; + private static final String NON_CONTROLLER_CLASSNAME = "SampleNonController"; + @Autowired private BootLanguageServerHarness harness; private Editor editor; @@ -50,7 +52,7 @@ public class RequestMappingSnippetTests { @Test void testAnnotationMarkerOnkyPrefix() throws Exception { - prepareCase("@<*>"); + prepareCase(CONTROLLER_CLASSNAME, "@<*>"); List completions = editor.getCompletions(); assertEquals(4, completions.size()); @@ -73,7 +75,7 @@ public class RequestMappingSnippetTests { @Test void testSimpleGetPrefix() throws Exception { - prepareCase("Get<*>"); + prepareCase(CONTROLLER_CLASSNAME, "Get<*>"); List completions = editor.getCompletions(); assertEquals(1, completions.size()); @@ -84,7 +86,7 @@ public class RequestMappingSnippetTests { @Test void testSimplePostPrefix() throws Exception { - prepareCase("Post<*>"); + prepareCase(CONTROLLER_CLASSNAME, "Post<*>"); List completions = editor.getCompletions(); assertEquals(1, completions.size()); @@ -95,7 +97,7 @@ public class RequestMappingSnippetTests { @Test void testAnnotationAndTextPrefixForGet() throws Exception { - prepareCase("@G<*>"); + prepareCase(CONTROLLER_CLASSNAME, "@G<*>"); List completions = editor.getCompletions(); assertEquals(1, completions.size()); @@ -106,7 +108,7 @@ public class RequestMappingSnippetTests { @Test void testAnnotationAndTextPrefixForGet2() throws Exception { - prepareCase("@Get<*>"); + prepareCase(CONTROLLER_CLASSNAME, "@Get<*>"); List completions = editor.getCompletions(); assertEquals(1, completions.size()); @@ -117,7 +119,7 @@ public class RequestMappingSnippetTests { @Test void testAnnotationAndTextPrefixForGet3() throws Exception { - prepareCase("@GetM<*>"); + prepareCase(CONTROLLER_CLASSNAME, "@GetM<*>"); List completions = editor.getCompletions(); assertEquals(1, completions.size()); @@ -128,7 +130,7 @@ public class RequestMappingSnippetTests { @Test void testSnippetExtractionIntoCode() throws Exception { - prepareCase("Get<*>"); + prepareCase(CONTROLLER_CLASSNAME, "Get<*>"); List completions = editor.getCompletions(); assertEquals(1, completions.size()); @@ -152,12 +154,20 @@ public class RequestMappingSnippetTests { + "}\n" + ""); } + + @Test + void testSnippetsNotShowUpForNonControllerClasses() throws Exception { + prepareCase(NON_CONTROLLER_CLASSNAME, "@<*>"); + + List completions = editor.getCompletions(); + assertEquals(0, completions.size()); + } - private void prepareCase(String prefix) throws Exception { - InputStream resource = this.getClass().getResourceAsStream("/test-projects/test-request-mapping-completions/src/main/java/example/SampleController.java"); + private void prepareCase(String className, String prefix) throws Exception { + InputStream resource = this.getClass().getResourceAsStream("/test-projects/test-request-mapping-completions/src/main/java/example/" + className + ".java"); String content = IOUtils.toString(resource); - content = content.replace("class SampleController {", "class SampleController {\n\n" + prefix); + content = content.replace("class " + className + " {", "class " + className + " {\n\n" + prefix); editor = new Editor(harness, content, LanguageId.JAVA); } diff --git a/headless-services/spring-boot-language-server/src/test/resources/test-projects/test-request-mapping-completions/src/main/java/example/SampleNonController.java b/headless-services/spring-boot-language-server/src/test/resources/test-projects/test-request-mapping-completions/src/main/java/example/SampleNonController.java new file mode 100644 index 000000000..ef60daf4c --- /dev/null +++ b/headless-services/spring-boot-language-server/src/test/resources/test-projects/test-request-mapping-completions/src/main/java/example/SampleNonController.java @@ -0,0 +1,8 @@ +package example; + +import org.springframework.stereotype.Service; + +@Service +public class SampleNonController { + +}