From 86f9348775a64871925e6f007b1afd35680d0da8 Mon Sep 17 00:00:00 2001 From: Martin Lippert Date: Wed, 1 Nov 2017 12:06:06 +0100 Subject: [PATCH 1/3] fixed issue with live hover to autowired showing up even without the dependencies being wired successfully --- .../autowired/AutowiredHoverProvider.java | 105 ++++++++---------- .../ComponentInjectionsHoverProvider.java | 3 +- .../boot/java/livehover/LiveHoverUtils.java | 4 +- .../test/AutowiredHoverProviderTest.java | 45 ++++++++ 4 files changed, 94 insertions(+), 63 deletions(-) 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 df8421d98..3285f967b 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 @@ -28,7 +28,6 @@ import org.springframework.ide.vscode.boot.java.utils.ASTUtils; import org.springframework.ide.vscode.commons.boot.app.cli.SpringBootApp; import org.springframework.ide.vscode.commons.boot.app.cli.livebean.LiveBean; import org.springframework.ide.vscode.commons.boot.app.cli.livebean.LiveBeansModel; -import org.springframework.ide.vscode.commons.util.BadLocationException; import org.springframework.ide.vscode.commons.util.Log; import org.springframework.ide.vscode.commons.util.StringUtil; import org.springframework.ide.vscode.commons.util.text.TextDocument; @@ -43,18 +42,25 @@ public class AutowiredHoverProvider implements HoverProvider { @Override public Collection getLiveHoverHints(Annotation annotation, TextDocument doc, SpringBootApp[] runningApps) { try { - for (SpringBootApp bootApp : runningApps) { - try { - LiveBeansModel liveBeans = bootApp.getBeans(); - if (liveBeans != null && !liveBeans.isEmpty()) { - Range range = getLiveHoverHint(annotation, doc, liveBeans); - if (range != null) { - return ImmutableList.of(range); + LiveBean definedBean = getDefinedBean(annotation); + if (definedBean != null) { + for (SpringBootApp app : runningApps) { + try { + List relevantBeans = LiveHoverUtils.findRelevantBeans(app, definedBean).collect(Collectors.toList()); + + if (!relevantBeans.isEmpty()) { + for (LiveBean bean : relevantBeans) { + String[] dependencies = bean.getDependencies(); + if (dependencies != null && dependencies.length > 0) { + Range hoverRange = doc.toRange(annotation.getStartPosition(), annotation.getLength()); + return ImmutableList.of(hoverRange); + } + } } } - } - catch (Exception e) { - Log.log(e); + catch (Exception e) { + Log.log(e); + } } } } @@ -65,27 +71,6 @@ public class AutowiredHoverProvider implements HoverProvider { return null; } - public Range getLiveHoverHint(Annotation annotation, TextDocument doc, LiveBeansModel beansModel) { - try { - TypeDeclaration declaringType = ASTUtils.findDeclaringType(annotation); - if (declaringType != null) { - String type = declaringType.resolveBinding().getQualifiedName(); - if (type != null && beansModel != null) { - List beansOfType = beansModel.getBeansOfType(type); - if (!beansOfType.isEmpty()) { - Range hoverRange = doc.toRange(annotation.getStartPosition(), annotation.getLength()); - return hoverRange; - } - } - } - } - catch (BadLocationException e) { - Log.log(e); - } - - return null; - } - @Override public CompletableFuture provideHover(ASTNode node, Annotation annotation, ITypeBinding type, int offset, TextDocument doc, SpringBootApp[] runningApps) { @@ -99,6 +84,8 @@ public class AutowiredHoverProvider implements HoverProvider { hover.append("**Injection report for " + LiveHoverUtils.showBean(definedBean) + "**\n\n"); boolean hasInterestingApp = false; + boolean hasAutowiring = false; + for (SpringBootApp app : runningApps) { LiveBeansModel beans = app.getBeans(); List relevantBeans = LiveHoverUtils.findRelevantBeans(app, definedBean).collect(Collectors.toList()); @@ -113,11 +100,11 @@ public class AutowiredHoverProvider implements HoverProvider { for (LiveBean bean : relevantBeans) { hover.append("\n\n"); - addAutomaticallyWired(hover, annotation, beans, bean); + hasAutowiring |= addAutomaticallyWired(hover, annotation, beans, bean); } } } - if (hasInterestingApp) { + if (hasInterestingApp && hasAutowiring) { System.out.println(hover); return CompletableFuture .completedFuture(new Hover(ImmutableList.of(Either.forLeft(hover.toString())))); @@ -132,7 +119,7 @@ public class AutowiredHoverProvider implements HoverProvider { if (declaringType != null) { ITypeBinding beanType = declaringType.resolveBinding(); if (beanType != null) { - String id = getBeanId(annotation, beanType); + String id = getBeanId(declaringType, beanType); if (StringUtil.hasText(id)) { return LiveBean.builder().id(id).type(beanType.getQualifiedName()).build(); } @@ -141,38 +128,36 @@ public class AutowiredHoverProvider implements HoverProvider { return null; } - private String getBeanId(Annotation annotation, ITypeBinding beanType) { - return ASTUtils.getAttribute(annotation, "value").flatMap(ASTUtils::getFirstString) - .orElseGet(() -> { - String typeName = beanType.getName(); - if (StringUtil.hasText(typeName)) { - return Character.toLowerCase(typeName.charAt(0)) + typeName.substring(1); - } - return null; - }); + private String getBeanId(TypeDeclaration declaringType, ITypeBinding beanType) { + // TODO: take specific bean declarations into account like @Component at declaring type + String typeName = beanType.getName(); + if (StringUtil.hasText(typeName)) { + return Character.toLowerCase(typeName.charAt(0)) + typeName.substring(1); + } + return null; } - private void addAutomaticallyWired(StringBuilder hover, Annotation annotation, LiveBeansModel beans, LiveBean bean) { - TypeDeclaration typeDecl = ASTUtils.findDeclaringType(annotation); - if (typeDecl != null) { - String[] dependencies = bean.getDependencies(); + private boolean addAutomaticallyWired(StringBuilder hover, Annotation annotation, LiveBeansModel beans, LiveBean bean) { + boolean result = false; + String[] dependencies = bean.getDependencies(); - if (dependencies != null && dependencies.length > 0) { - hover.append(LiveHoverUtils.showBean(bean) + " got autowired with:\n\n"); + if (dependencies != null && dependencies.length > 0) { + result = true; + hover.append(LiveHoverUtils.showBean(bean) + " got autowired with:\n\n"); - boolean firstDependency = true; - for (String injectedBean : dependencies) { - if (!firstDependency) { - hover.append("\n"); - } - List dependencyBeans = beans.getBeansOfName(injectedBean); - for (LiveBean dependencyBean : dependencyBeans) { - hover.append("- " + LiveHoverUtils.showBean(dependencyBean)); - } - firstDependency = false; + boolean firstDependency = true; + for (String injectedBean : dependencies) { + if (!firstDependency) { + hover.append("\n"); } + List dependencyBeans = beans.getBeansOfName(injectedBean); + for (LiveBean dependencyBean : dependencyBeans) { + hover.append("- " + LiveHoverUtils.showBean(dependencyBean)); + } + firstDependency = false; } } + return result; } } diff --git a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/livehover/ComponentInjectionsHoverProvider.java b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/livehover/ComponentInjectionsHoverProvider.java index 39c053d20..f145b3964 100644 --- a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/livehover/ComponentInjectionsHoverProvider.java +++ b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/livehover/ComponentInjectionsHoverProvider.java @@ -25,7 +25,8 @@ import org.springframework.ide.vscode.commons.util.StringUtil; public class ComponentInjectionsHoverProvider extends AbstractInjectedIntoHoverProvider { - @Override protected void addAutomaticallyWiredContructor(StringBuilder hover, Annotation annotation, LiveBeansModel beans, LiveBean bean) { + @Override + protected void addAutomaticallyWiredContructor(StringBuilder hover, Annotation annotation, LiveBeansModel beans, LiveBean bean) { TypeDeclaration typeDecl = ASTUtils.findDeclaringType(annotation); if (typeDecl != null) { MethodDeclaration[] constructors = ASTUtils.findConstructors(typeDecl); diff --git a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/livehover/LiveHoverUtils.java b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/livehover/LiveHoverUtils.java index 1968b48fa..ac1f96887 100644 --- a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/livehover/LiveHoverUtils.java +++ b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/livehover/LiveHoverUtils.java @@ -38,10 +38,10 @@ public class LiveHoverUtils { public static Stream findRelevantBeans(SpringBootApp app, LiveBean definedBean) { LiveBeansModel beansModel = app.getBeans(); - if (beansModel!=null) { + if (beansModel != null) { Stream relevantBeans = beansModel.getBeansOfName(definedBean.getId()).stream(); String type = definedBean.getType(); - if (type!=null) { + if (type != null) { relevantBeans = relevantBeans.filter(bean -> type.equals(bean.getType())); } return relevantBeans; 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 305e00b3f..b9f8f717f 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 @@ -194,4 +194,49 @@ public class AutowiredHoverProviderTest { editor.assertNoHover("@Autowired"); } + @Test + public void noHoversWhenRunningAppDoesntHaveDependenciesForTheAutowiring() throws Exception { + LiveBeansModel beans = LiveBeansModel.builder() + .add(LiveBean.builder() + .id("autowiredClass") + .type("com.example.AutowiredClass") + .build() + ) + .add(LiveBean.builder() + .id("dependencyA") + .type("com.example.DependencyA") + .build() + ) + .add(LiveBean.builder() + .id("dependencyB") + .type("com.example.DependencyB") + .build() + ) + .build(); + mockAppProvider.builder() + .isSpringBootApp(true) + .processId("111") + .processName("the-app") + .beans(beans) + .build(); + + Editor editor = harness.newEditor(LanguageId.JAVA, + "package com.example;\n" + + "\n" + + "import org.springframework.beans.factory.annotation.Autowired;\n" + + "import org.springframework.stereotype.Component;\n" + + "\n" + + "@Component\n" + + "public class AutowiredClass {\n" + + "\n" + + " @Autowired\n" + + " public AutowiredClass(DependencyA depA, DependencyB depB) {\n" + + " }\n" + + "}\n" + ); + + editor.assertHighlights("@Component"); + editor.assertNoHover("@Autowired"); + } + } From 1ce27595f131b2bfa83430a7760b9dd523f5c9c1 Mon Sep 17 00:00:00 2001 From: Martin Lippert Date: Wed, 1 Nov 2017 16:00:47 +0100 Subject: [PATCH 2/3] added missing copyright statement --- .../vscode/boot/java/utils/CompilationUnitCache.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java index b42c7362d..86b170f07 100644 --- a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java +++ b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java @@ -1,3 +1,13 @@ +/******************************************************************************* + * Copyright (c) 2017 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 + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ package org.springframework.ide.vscode.boot.java.utils; import java.net.URI; From 49c9517b8ac6d0ea9ca9bedab68f5fd64a91c5ff Mon Sep 17 00:00:00 2001 From: Martin Lippert Date: Wed, 1 Nov 2017 16:29:19 +0100 Subject: [PATCH 3/3] disable lsp4e go to symbol in workspace keybinding altogether as long as there is no fix in LSP4E --- .../java/ls/BootJavaLanguageServerPlugin.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/eclipse-language-servers/org.springframework.tooling.boot.java.ls/src/org/springframework/tooling/boot/java/ls/BootJavaLanguageServerPlugin.java b/eclipse-language-servers/org.springframework.tooling.boot.java.ls/src/org/springframework/tooling/boot/java/ls/BootJavaLanguageServerPlugin.java index b068daf68..3425e8079 100644 --- a/eclipse-language-servers/org.springframework.tooling.boot.java.ls/src/org/springframework/tooling/boot/java/ls/BootJavaLanguageServerPlugin.java +++ b/eclipse-language-servers/org.springframework.tooling.boot.java.ls/src/org/springframework/tooling/boot/java/ls/BootJavaLanguageServerPlugin.java @@ -10,6 +10,13 @@ *******************************************************************************/ package org.springframework.tooling.boot.java.ls; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import org.eclipse.jface.bindings.Binding; +import org.eclipse.ui.PlatformUI; +import org.eclipse.ui.keys.IBindingService; import org.eclipse.ui.plugin.AbstractUIPlugin; import org.osgi.framework.BundleContext; @@ -22,6 +29,8 @@ import org.osgi.framework.BundleContext; public class BootJavaLanguageServerPlugin extends AbstractUIPlugin { public static final String ID = "org.springframework.tooling.boot.java.ls"; + + private static final Object LSP4E_COMMAND_SYMBOL_IN_WORKSPACE = "org.eclipse.lsp4e.symbolinworkspace"; // The shared instance private static BootJavaLanguageServerPlugin plugin; @@ -34,6 +43,8 @@ public class BootJavaLanguageServerPlugin extends AbstractUIPlugin { public void start(BundleContext context) throws Exception { plugin = this; super.start(context); + + deactivateDuplicateKeybindings(); } @Override @@ -46,4 +57,42 @@ public class BootJavaLanguageServerPlugin extends AbstractUIPlugin { return plugin; } + private void deactivateDuplicateKeybindings() { + IBindingService service = PlatformUI.getWorkbench().getService(IBindingService.class); + if (service != null) { + List newBindings = new ArrayList<>(); + Binding[] bindings = service.getBindings(); + + for (Binding binding : bindings) { + String commandId = null; + + if (binding != null && binding.getParameterizedCommand() != null && binding.getParameterizedCommand().getCommand() != null) { + commandId = binding.getParameterizedCommand().getCommand().getId(); + + if (commandId == null) { + newBindings.add(binding); + } + else if (!commandId.equals(LSP4E_COMMAND_SYMBOL_IN_WORKSPACE)) { + newBindings.add(binding); + } + } + else { + newBindings.add(binding); + } + } + + PlatformUI.getWorkbench().getDisplay().asyncExec(new Runnable() { + @Override + public void run() { + try { + service.savePreferences(service.getActiveScheme(), + newBindings.toArray(new Binding[newBindings.size()])); + } catch (IOException e) { + e.printStackTrace(); + } + } + }); + } + } + }