SLVSCODE-152 Show Node-related settings on issue with Node path

Author: jblievremont

pom.xml

@@ -23,7 +23,7 @@
   </issueManagement>
 
   <properties>
-    <sonarlint.core.version>4.14.1.22501</sonarlint.core.version>
+    <sonarlint.core.version>4.14.1.22545</sonarlint.core.version>
     <!-- analyzers used for tests -->
     <sonar.java.version>6.5.0.22421</sonar.java.version>
     <sonar.javascript.version>6.2.1.12157</sonar.javascript.version>
@@ -209,7 +209,7 @@
         <configuration>
           <trimStackTrace>false</trimStackTrace>
           <environmentVariables>
-            <PATH>${basedir}/src/test/resources/fake-ts-project/node;${env.PATH}</PATH>
+            <PATH>${basedir}/src/test/resources/fake-ts-project/node${path.separator}${env.PATH}</PATH>
           </environmentVariables>
         </configuration>
       </plugin>

src/main/java/org/sonarsource/sonarlint/ls/AnalysisManager.java

@@ -53,6 +53,7 @@
 import org.eclipse.lsp4j.Range;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
+import org.sonarsource.sonarlint.core.client.api.common.Language;
 import org.sonarsource.sonarlint.core.client.api.common.PluginDetails;
 import org.sonarsource.sonarlint.core.client.api.common.analysis.AnalysisResults;
 import org.sonarsource.sonarlint.core.client.api.common.analysis.ClientInputFile;
@@ -248,7 +249,10 @@ private void analyze(URI fileUri, boolean shouldFetchServerIssues) {
       }
       SkippedPluginsNotifier.notifyOnceForSkippedPlugins(analysisResults.results, analysisResults.allPlugins, client);
 
-      telemetry.analysisDoneOnSingleLanguage(analysisResults.results.languagePerFile().values().iterator().next(), analysisResults.analysisTime);
+      Collection<Language> analyzedLanguages = analysisResults.results.languagePerFile().values();
+      if (!analyzedLanguages.isEmpty()) {
+        telemetry.analysisDoneOnSingleLanguage(analyzedLanguages.iterator().next(), analysisResults.analysisTime);
+      }
 
       // Ignore files with parsing error
       analysisResults.results.failedAnalysisFiles().stream()
@@ -504,12 +508,6 @@ private void analyzeAllOpenJavaFiles() {
     }
   }
 
-  private void analyzeAllOpenFiles() {
-    for (URI fileUri : fileContentPerFileURI.keySet()) {
-      analyzeAsync(fileUri, false);
-    }
-  }
-
   private Map<String, String> configureJavaProperties(URI fileUri) {
     Optional<GetJavaConfigResponse> cachedJavaConfigOpt = ofNullable(javaConfigPerFileURI.get(fileUri)).orElse(empty());
     return cachedJavaConfigOpt.map(cachedJavaConfig -> {

src/main/java/org/sonarsource/sonarlint/ls/DefaultClientInputFile.java

@@ -97,7 +97,7 @@ private static Language toSqLanguage(@Nullable String clientLanguageId) {
     if (clientLanguageId == null) {
       return null;
     }
-    // See https://microsoft.github.io/language-server-protocol/specification#textdocumentitem
+    // See https://microsoft.github.io/language-server-protocol/specification#textDocumentItem
     switch (clientLanguageId) {
       case "javascript":
       case "javascriptreact":
@@ -114,8 +114,13 @@ private static Language toSqLanguage(@Nullable String clientLanguageId) {
         return Language.HTML;
       case "oraclesql":
         return Language.PLSQL;
+      case "apex":
+      case "apex-anon":
+        // See https://github.com/forcedotcom/salesforcedx-vscode/blob/5e4b7715d1cb3d1ee2780780ed63f70f58e93b20/packages/salesforcedx-vscode-apex/package.json#L273
+        return Language.APEX;
       default:
-        return null;
+        // Other supported languages map to the same key as the one used in SonarQube/SonarCloud
+        return Language.forKey(clientLanguageId).orElse(null);
     }
   }
 }

src/main/java/org/sonarsource/sonarlint/ls/NodeJsRuntime.java

@@ -21,19 +21,16 @@
 
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Supplier;
-import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.apache.commons.lang.StringUtils;
 import org.sonarsource.sonarlint.core.NodeJsHelper;
 import org.sonarsource.sonarlint.core.client.api.common.Version;
 import org.sonarsource.sonarlint.ls.settings.SettingsManager;
 import org.sonarsource.sonarlint.ls.settings.WorkspaceSettings;
-import org.sonarsource.sonarlint.ls.settings.WorkspaceSettingsChangeListener;
 
-public class NodeJsRuntime implements WorkspaceSettingsChangeListener {
+public class NodeJsRuntime {
 
   private final SettingsManager settingsManager;
   private final Supplier<NodeJsHelper> nodeJsHelperFactory;
@@ -82,13 +79,4 @@ public Version getNodeJsVersion() {
     }
     return nodeJsVersion;
   }
-
-  @Override
-  public void onChange(@CheckForNull WorkspaceSettings oldValue, WorkspaceSettings newValue) {
-    if (oldValue == null || !(Objects.equals(oldValue.pathToNodeExecutable(), newValue.pathToNodeExecutable()))) {
-      init = false;
-      nodeJsPath = null;
-      nodeJsVersion = null;
-    }
-  }
 }

src/main/java/org/sonarsource/sonarlint/ls/SkippedPluginsNotifier.java

@@ -19,13 +19,15 @@
  */
 package org.sonarsource.sonarlint.ls;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Supplier;
 import org.eclipse.lsp4j.MessageActionItem;
-import org.eclipse.lsp4j.MessageParams;
 import org.eclipse.lsp4j.MessageType;
 import org.eclipse.lsp4j.ShowMessageRequestParams;
 import org.sonarsource.sonarlint.core.client.api.common.Language;
@@ -40,6 +42,8 @@ public class SkippedPluginsNotifier {
 
   private static final Set<String> displayedMessages = new HashSet<>();
 
+  public static final MessageActionItem ACTION_OPEN_SETTINGS = new MessageActionItem("Open Settings");
+
   private SkippedPluginsNotifier() {
   }
 
@@ -58,36 +62,30 @@ public static void notifyOnceForSkippedPlugins(AnalysisResults analysisResults,
             String content = String.format(
               "Java runtime version %s or later is required. Current version is %s.",runtimeRequirement.getMinVersion(), runtimeRequirement.getCurrentVersion()
             );
-            openJavaSettingsRequest(client, formatMessage(title, content));
+            showMessageWithOpenSettingsAction(client, formatMessage(title, content), client::openJavaHomeSettings);
           } else if (runtimeRequirement.getRuntime() == SkipReason.UnsatisfiedRuntimeRequirement.RuntimeRequirement.NODEJS) {
             String content = String.format(
               "Node.js runtime version %s or later is required.", runtimeRequirement.getMinVersion());
             if (runtimeRequirement.getCurrentVersion() != null) {
               content += String.format(" Current version is %s.", runtimeRequirement.getCurrentVersion());
             }
-            // TODO Show link to Node path property in the settings
-            showMessageWithoutAction(client, formatMessage(title, content));
+            showMessageWithOpenSettingsAction(client, formatMessage(title, content), client::openPathToNodeSettings);
           }
         }
       });
     });
   }
 
-  private static void openJavaSettingsRequest(SonarLintExtendedLanguageClient client, String message) {
+  private static void showMessageWithOpenSettingsAction(SonarLintExtendedLanguageClient client, String message, Supplier<CompletableFuture<Void>> callback) {
     if (displayedMessages.add(message)) {
-      ShowMessageRequestParams params = new ShowMessageRequestParams(singletonList(new MessageActionItem("Open Java Settings")));
+      ShowMessageRequestParams params = new ShowMessageRequestParams(singletonList(ACTION_OPEN_SETTINGS));
       params.setType(MessageType.Error);
       params.setMessage(message);
-      client.showMessageRequest(params).thenAccept(action -> client.openJavaHomeSettings());
-    }
-  }
-
-  private static void showMessageWithoutAction(SonarLintExtendedLanguageClient client, String longMessage) {
-    if (displayedMessages.add(longMessage)) {
-      MessageParams params = new MessageParams();
-      params.setType(MessageType.Error);
-      params.setMessage(longMessage);
-      client.showMessage(params);
+      client.showMessageRequest(params).thenAccept(action -> {
+        if (ACTION_OPEN_SETTINGS.equals(action)) {
+          callback.get();
+        }
+      });
     }
   }
 
@@ -103,4 +101,9 @@ private static void showMessageWithoutAction(SonarLintExtendedLanguageClient cli
   private static String formatMessage(String title, String content) {
     return String.format("%s: %s", title, content);
   }
+
+  @VisibleForTesting
+  static void clearMessages() {
+    displayedMessages.clear();
+  }
 }

src/main/java/org/sonarsource/sonarlint/ls/SonarLintExtendedLanguageClient.java

@@ -38,6 +38,9 @@ public interface SonarLintExtendedLanguageClient extends LanguageClient {
   @JsonRequest("sonarlint/openJavaHomeSettings")
   CompletableFuture<Void> openJavaHomeSettings();
 
+  @JsonRequest("sonarlint/openPathToNodeSettings")
+  CompletableFuture<Void> openPathToNodeSettings();
+
   @JsonRequest("sonarlint/showRuleDescription")
   CompletableFuture<Void> showRuleDescription(ShowRuleDescriptionParams params);
 

src/main/java/org/sonarsource/sonarlint/ls/SonarLintLanguageServer.java

@@ -107,7 +107,6 @@ public class SonarLintLanguageServer implements SonarLintExtendedLanguageServer,
     this.workspaceFoldersManager = new WorkspaceFoldersManager();
     this.settingsManager = new SettingsManager(this.client, this.workspaceFoldersManager);
     this.nodeJsRuntime = new NodeJsRuntime(settingsManager);
-    this.settingsManager.addListener(nodeJsRuntime);
     this.enginesFactory = new EnginesFactory(analyzers, lsLogOutput, nodeJsRuntime);
     this.settingsManager.addListener(telemetry);
     this.settingsManager.addListener(lsLogOutput);

src/main/java/org/sonarsource/sonarlint/ls/Utils.java

@@ -22,17 +22,14 @@
 import com.google.gson.Gson;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonSyntaxException;
-import java.util.Iterator;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.ThreadFactory;
 import javax.annotation.CheckForNull;
 import org.eclipse.lsp4j.jsonrpc.ResponseErrorException;
 import org.eclipse.lsp4j.jsonrpc.messages.ResponseError;
 import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
-import org.sonarsource.sonarlint.core.client.api.common.Language;
 
 public class Utils {
 
@@ -65,15 +62,4 @@ public static void interrupted(InterruptedException e) {
     LOG.debug("Interrupted!", e);
     Thread.currentThread().interrupt();
   }
-
-  public static Language[] toLanguageArray(Set<Language> languages) {
-    Language[] languagesArray = new Language[languages.size()];
-    Iterator<Language> iterator = languages.iterator();
-    for (int i = 0; i < languages.size(); i++) {
-      languagesArray[i] = iterator.next();
-    }
-    return languagesArray;
-  }
-
-
 }

src/test/java/org/sonarsource/sonarlint/ls/DefaultClientInputFileTest.java

@@ -0,0 +1,67 @@
+/*
+ * SonarLint Language Server
+ * Copyright (C) 2009-2020 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonarsource.sonarlint.ls;
+
+
+import java.util.stream.Stream;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.sonarsource.sonarlint.core.client.api.common.Language;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class DefaultClientInputFileTest {
+
+  @ParameterizedTest(name = "Should detect {0} as {1}")
+  @MethodSource("provideParametersForLanguageDetection")
+  void shouldDetectLanguage(String clientLanguageId, Language expected) {
+    assertThat(new DefaultClientInputFile(null, null, "", false, clientLanguageId).language())
+      .isEqualTo(expected);
+  }
+
+  private static Stream<Arguments> provideParametersForLanguageDetection() {
+    return Stream.of(
+      Arguments.of("javascript", Language.JS),
+      Arguments.of("javascriptreact", Language.JS),
+      Arguments.of("vue", Language.JS),
+      Arguments.of("vue component", Language.JS),
+      Arguments.of("babel es6 javascript", Language.JS),
+
+      Arguments.of("python", Language.PYTHON),
+
+      Arguments.of("typescript", Language.TS),
+      Arguments.of("typescriptreact", Language.TS),
+
+      Arguments.of("html", Language.HTML),
+
+      Arguments.of("oraclesql", Language.PLSQL),
+      Arguments.of("plsql", Language.PLSQL),
+
+      Arguments.of("apex", Language.APEX),
+      Arguments.of("apex-anon", Language.APEX),
+
+      Arguments.of("php", Language.PHP),
+      Arguments.of("java", Language.JAVA),
+
+      Arguments.of("unknown", null)
+    );
+  }
+}

src/test/java/org/sonarsource/sonarlint/ls/NodeJsRuntimeTest.java

@@ -19,20 +19,20 @@
  */
 package org.sonarsource.sonarlint.ls;
 
-import java.io.File;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
-import org.mockito.Mockito;
 import org.sonarsource.sonarlint.core.NodeJsHelper;
 import org.sonarsource.sonarlint.core.client.api.common.Version;
 import org.sonarsource.sonarlint.ls.settings.SettingsManager;
 import org.sonarsource.sonarlint.ls.settings.WorkspaceSettings;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 class NodeJsRuntimeTest {
 
@@ -82,47 +82,4 @@ void shouldLazyInitializeWithNodeSettings() {
     verify(settings).pathToNodeExecutable();
     verify(nodeJsHelper, times(1)).detect(temp.getFileName());
   }
-
-  @Test
-  void shouldKeepOldSettingsWhenNotChanged() {
-    when(settings.pathToNodeExecutable()).thenReturn(null);
-    WorkspaceSettings newSettings = mock(WorkspaceSettings.class);
-    when(newSettings.pathToNodeExecutable()).thenReturn(null);
-
-    assertThat(underTest.nodeVersion()).isNull();
-    assertThat(underTest.getNodeJsPath()).isNull();
-    assertThat(underTest.getNodeJsVersion()).isNull();
-
-    underTest.onChange(settings, newSettings);
-
-    assertThat(underTest.nodeVersion()).isNull();
-    assertThat(underTest.getNodeJsPath()).isNull();
-    assertThat(underTest.getNodeJsVersion()).isNull();
-
-    verify(nodeJsHelper, times(1)).detect(null);
-  }
-
-  @Test
-  void shouldRedetectWhenNodePathChanges() {
-    when(settings.pathToNodeExecutable()).thenReturn(null);
-    WorkspaceSettings newSettings = mock(WorkspaceSettings.class);
-    String newPathToNodeSettings = temp.getFileName().toString();
-    when(newSettings.pathToNodeExecutable()).thenReturn(newPathToNodeSettings);
-
-    assertThat(underTest.nodeVersion()).isNull();
-    assertThat(underTest.getNodeJsPath()).isNull();
-    assertThat(underTest.getNodeJsVersion()).isNull();
-
-    underTest.onChange(settings, newSettings);
-
-    when(settings.pathToNodeExecutable()).thenReturn(newPathToNodeSettings);
-    when(nodeJsHelper.getNodeJsPath()).thenReturn(temp.resolve("node"));
-    String version = "12.34.56";
-    when(nodeJsHelper.getNodeJsVersion()).thenReturn(Version.create(version));
-
-    assertThat(underTest.nodeVersion()).isEqualTo(version);
-
-    verify(nodeJsHelper, times(1)).detect(null);
-    verify(nodeJsHelper, times(1)).detect(temp.getFileName());
-  }
 }