From 7912f951eb30bd9793d7380569d418483591cc0b Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Fri, 17 Nov 2017 13:55:35 -0800 Subject: [PATCH] Sow @Autowired annotation hover also in any sub-annotation of @Component --- .../annotations/AnnotationHierarchies.java | 77 ++++++++++++++----- .../AnnotationHierarchyAwareLookup.java | 4 +- .../autowired/AutowiredHoverProvider.java | 8 +- .../test/AutowiredHoverProviderTest.java | 54 +++++++++++++ 4 files changed, 116 insertions(+), 27 deletions(-) diff --git a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java index dce7a633f..7feb9a8c7 100644 --- a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java +++ b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java @@ -13,10 +13,17 @@ package org.springframework.ide.vscode.boot.java.annotations; import java.util.Collection; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; +import org.eclipse.jdt.core.dom.Annotation; import org.eclipse.jdt.core.dom.IAnnotationBinding; import org.eclipse.jdt.core.dom.ITypeBinding; +import org.springframework.ide.vscode.commons.util.Log; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.collect.ImmutableList; /** @@ -27,44 +34,76 @@ import com.google.common.collect.ImmutableList; * @author Kris De Volder */ -public class AnnotationHierarchies { +public abstract class AnnotationHierarchies { - protected boolean ignoreAnnotation(String fqname) { + /** + * Note: this cacehe is not just for efficiency! Without it, we also get into trouble accessing the + * AST on multiple threads. AST's coming from CompilationUnit cache should not be accessed on multiple + * threads. + */ + private static Cache> supertypes = CacheBuilder.newBuilder() + .weakKeys() + .expireAfterWrite(30, TimeUnit.SECONDS) + .build(); + + private AnnotationHierarchies() { + } + + protected static boolean ignoreAnnotation(String fqname) { return fqname.startsWith("java."); //mostly intended to capture java.lang.annotation.* types. But really it should be //safe to ignore any type defined by the JRE since it can't possibly be inheriting from a spring annotation. }; - public Collection getDirectSuperAnnotations(ITypeBinding typeBinding) { - IAnnotationBinding[] annotations = typeBinding.getAnnotations(); - if (annotations!=null && annotations.length!=0) { - ImmutableList.Builder superAnnotations = ImmutableList.builder(); - for (IAnnotationBinding ab : annotations) { - ITypeBinding sa = ab.getAnnotationType(); - if (sa!=null) { - if (!ignoreAnnotation(sa.getQualifiedName())) { - superAnnotations.add(sa); + public static Collection getDirectSuperAnnotations(ITypeBinding typeBinding) { + try { + return supertypes.get(typeBinding, () -> { + IAnnotationBinding[] annotations = typeBinding.getAnnotations(); + if (annotations!=null && annotations.length!=0) { + ImmutableList.Builder superAnnotations = ImmutableList.builder(); + for (IAnnotationBinding ab : annotations) { + ITypeBinding sa = ab.getAnnotationType(); + if (sa!=null) { + if (!ignoreAnnotation(sa.getQualifiedName())) { + superAnnotations.add(sa); + } + } } + return superAnnotations.build(); } - } - return superAnnotations.build(); + return ImmutableList.of(); + }); + } catch (ExecutionException e) { + Log.log(e); + return ImmutableList.of(); } - return ImmutableList.of(); } - public Set getTransitiveSuperAnnotations(ITypeBinding typeBinding) { + public static Set getTransitiveSuperAnnotations(ITypeBinding typeBinding) { Set seen = new HashSet<>(); findTransitiveSupers(typeBinding, seen); return seen; } - private void findTransitiveSupers(ITypeBinding typeBinding, Set seen) { + public static Stream findTransitiveSupers(ITypeBinding typeBinding, Set seen) { String qname = typeBinding.getQualifiedName(); if (seen.add(qname)) { - for (ITypeBinding superBinding : getDirectSuperAnnotations(typeBinding)) { - findTransitiveSupers(superBinding, seen); - } + return Stream.concat( + Stream.of(qname), + getDirectSuperAnnotations(typeBinding).stream().flatMap(superBinding -> + findTransitiveSupers(superBinding, seen) + ) + ); } + return Stream.empty(); } + public static boolean isSubtypeOf(Annotation annotation, String fqAnnotationTypeName) { + ITypeBinding annotationType = annotation.resolveTypeBinding(); + if (annotationType!=null) { + return findTransitiveSupers(annotationType, new HashSet<>()) + .anyMatch(superType -> superType.equals(fqAnnotationTypeName)); + } + return false; + } } diff --git a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchyAwareLookup.java b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchyAwareLookup.java index 7e15abceb..caa878931 100644 --- a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchyAwareLookup.java +++ b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchyAwareLookup.java @@ -47,8 +47,6 @@ public class AnnotationHierarchyAwareLookup { } } - private AnnotationHierarchies annotationHierarchies = new AnnotationHierarchies(); - /** * Associates fq anotation type name to a Binding. */ @@ -119,7 +117,7 @@ public class AnnotationHierarchyAwareLookup { isOverriding = binding.isOverriding; } if (!isOverriding) { - for (ITypeBinding superAnnotation : annotationHierarchies.getDirectSuperAnnotations(typeBinding)) { + for (ITypeBinding superAnnotation : AnnotationHierarchies.getDirectSuperAnnotations(typeBinding)) { findElements(superAnnotation, seen, superResult -> { requestor.accept(superResult); requestor.accept(Tuples.of(qname, superResult.getT2())); diff --git a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/autowired/AutowiredHoverProvider.java b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/autowired/AutowiredHoverProvider.java index 2337e786c..b2443edc2 100644 --- a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/autowired/AutowiredHoverProvider.java +++ b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/autowired/AutowiredHoverProvider.java @@ -23,6 +23,7 @@ import org.eclipse.lsp4j.Hover; import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.jsonrpc.messages.Either; import org.springframework.ide.vscode.boot.java.Annotations; +import org.springframework.ide.vscode.boot.java.annotations.AnnotationHierarchies; import org.springframework.ide.vscode.boot.java.handlers.HoverProvider; import org.springframework.ide.vscode.boot.java.livehover.ComponentInjectionsHoverProvider; import org.springframework.ide.vscode.boot.java.livehover.LiveHoverUtils; @@ -42,6 +43,7 @@ import com.google.common.collect.ImmutableList; */ public class AutowiredHoverProvider implements HoverProvider { + @Override public Collection getLiveHoverHints(Annotation annotation, TextDocument doc, SpringBootApp[] runningApps) { try { @@ -120,12 +122,8 @@ public class AutowiredHoverProvider implements HoverProvider { TypeDeclaration declaringType = ASTUtils.findDeclaringType(autowiredAnnotation); if (declaringType != null) { for (Annotation annotation : ASTUtils.getAnnotations(declaringType)) { - String annotationType = ASTUtils.getAnnotationType(annotation); - switch (annotationType) { - case Annotations.COMPONENT: + if (AnnotationHierarchies.isSubtypeOf(annotation, Annotations.COMPONENT)) { return ComponentInjectionsHoverProvider.getDefinedBeanForComponent(annotation); - default: - break; } } //TODO: handler below is an attempt to do something that may work in many cases, but is probably diff --git a/headless-services/boot-java-language-server/src/test/java/org/springframework/ide/vscode/boot/java/autowired/test/AutowiredHoverProviderTest.java b/headless-services/boot-java-language-server/src/test/java/org/springframework/ide/vscode/boot/java/autowired/test/AutowiredHoverProviderTest.java index 34da66695..a1039fa4d 100644 --- a/headless-services/boot-java-language-server/src/test/java/org/springframework/ide/vscode/boot/java/autowired/test/AutowiredHoverProviderTest.java +++ b/headless-services/boot-java-language-server/src/test/java/org/springframework/ide/vscode/boot/java/autowired/test/AutowiredHoverProviderTest.java @@ -308,4 +308,58 @@ public class AutowiredHoverProviderTest { } } + @Test public void bug_152621242_autowired_constructor_on_a_controller() throws Exception { + //https://www.pivotaltracker.com/story/show/152621242 + LiveBeansModel beans = LiveBeansModel.builder() + .add(LiveBean.builder() + .id("myController") + .type("com.example.MyController") + .dependencies("restTemplate") + .build() + ) + .add(LiveBean.builder() + .id("restTemplate") + .type("org.springframework.web.client.RestTemplate") + .build() + ) + .build(); + mockAppProvider.builder() + .isSpringBootApp(true) + .processId("111") + .processName("the-app") + .beans(beans) + .build(); + + + Editor editor = harness.newEditor( + "package com.example;\n" + + "\n" + + "import org.springframework.beans.factory.annotation.Autowired;\n" + + "import org.springframework.stereotype.Component;\n" + + "import org.springframework.stereotype.Controller;\n" + + "import org.springframework.web.client.RestTemplate;\n" + + "\n" + + "@Controller\n" + + "public class MyController {\n" + + "\n" + + " private RestTemplate restClient;\n" + + "\n" + + " @Autowired\n" + + " public MyController(RestTemplate restClient) {\n" + + " this.restClient = restClient;\n" + + " }\n" + + " \n" + + "}" + ); + editor.assertHighlights(/* not yet: "@Controller",*/ "@Autowired"); + for (int i = 1; i <= 2; i++) { + editor.assertHoverContains("@Autowired", 1, + "Bean [id: myController, type: `com.example.MyController`] got autowired with:\n" + + "\n" + + "- Bean: restTemplate \n" + + " Type: `org.springframework.web.client.RestTemplate`"); + } + + } + }