SLCORE-1619 Trigger a new analysis when binding changes

Author: damien-urruty-sonarsource

backend/core/src/main/java/org/sonarsource/sonarlint/core/analysis/AnalysisService.java

@@ -256,8 +256,7 @@ private AnalysisConfiguration getAnalysisConfigForEngine(String configScopeId, S
     var analysisProperties = analysisConfig.getAnalysisProperties();
     var inferredAnalysisProperties = startChild(trace, "getInferredAnalysisProperties", ANALYSIS_CFG_FOR_ENGINE,
       () -> client.getInferredAnalysisProperties(new GetInferredAnalysisPropertiesParams(
-        configScopeId, filesToAnalyze.stream().map(ClientFile::getUri).toList()
-      )).join().getProperties());
+        configScopeId, filesToAnalyze.stream().map(ClientFile::getUri).toList())).join().getProperties());
     analysisProperties.putAll(inferredAnalysisProperties);
     var activeRules = analysisConfig.getActiveRules().stream().map(r -> {
       var ar = new ActiveRule(r.getRuleKey(), r.getLanguageKey());
@@ -266,8 +265,7 @@ private AnalysisConfiguration getAnalysisConfigForEngine(String configScopeId, S
       return ar;
     }).toList();
     trace.setData("activeRulesCount", activeRules.size());
-    return startChild(trace, "buildAnalysisConfiguration", ANALYSIS_CFG_FOR_ENGINE, () ->
-      AnalysisConfiguration.builder()
+    return startChild(trace, "buildAnalysisConfiguration", ANALYSIS_CFG_FOR_ENGINE, () -> AnalysisConfiguration.builder()
       .addInputFiles(filesToAnalyze.stream().map(BackendInputFile::new).toList())
       .putAllExtraProperties(analysisProperties)
       // properties sent by client using new API were merged above
@@ -515,16 +513,13 @@ public void onPluginsSynchronized(PluginsSynchronizedEvent event) {
     var connectionId = event.connectionId();
     checkIfReadyForAnalysis(configurationRepository.getBoundScopesToConnection(connectionId)
       .stream().map(BoundScope::getConfigScopeId).collect(Collectors.toSet()));
-    configurationRepository.getBoundScopesToConnection(connectionId)
-      .stream().map(BoundScope::getConfigScopeId).forEach(this::autoAnalyzeOpenFiles);
   }
 
   @EventListener
   public void onConfigurationScopeAdded(ConfigurationScopesAddedWithBindingEvent event) {
     var configScopeIds = event.getConfigScopeIds();
     configScopeIds.forEach(schedulerCache::registerModuleIfLeafConfigScope);
     checkIfReadyForAnalysis(configScopeIds);
-    configScopeIds.forEach(this::autoAnalyzeOpenFiles);
   }
 
   @EventListener
@@ -537,22 +532,17 @@ public void onConfigurationScopeRemoved(ConfigurationScopeRemovedEvent event) {
   @EventListener
   public void onBindingConfigurationChanged(BindingConfigChangedEvent event) {
     var configScopeId = event.configScopeId();
-    analysisReadinessByConfigScopeId.remove(configScopeId);
-    if (!checkIfReadyForAnalysis(configScopeId)) {
-      client.didChangeAnalysisReadiness(new DidChangeAnalysisReadinessParams(Set.of(configScopeId), false));
-    }
+    checkIfReadyForAnalysis(Set.of(configScopeId));
   }
 
   @EventListener
   public void onAnalyzerConfigurationSynchronized(AnalyzerConfigurationSynchronized event) {
     checkIfReadyForAnalysis(event.getConfigScopeIds());
-    event.getConfigScopeIds().forEach(this::autoAnalyzeOpenFiles);
   }
 
   @EventListener
   public void onConfigurationScopesSynchronized(ConfigurationScopesSynchronizedEvent event) {
     checkIfReadyForAnalysis(event.getConfigScopeIds());
-    event.getConfigScopeIds().forEach(this::autoAnalyzeOpenFiles);
   }
 
   @EventListener
@@ -651,38 +641,34 @@ private void streamIssue(String configScopeId, UUID analysisId, ConcurrentHashMa
   }
 
   private void checkIfReadyForAnalysis(Set<String> configurationScopeIds) {
-    var readyConfigScopeIds = configurationScopeIds.stream()
-      .filter(this::isReadyForAnalysis)
-      .collect(toSet());
-    saveAndNotifyReadyForAnalysis(readyConfigScopeIds);
-  }
-
-  private boolean checkIfReadyForAnalysis(String configurationScopeId) {
-    if (isReadyForAnalysis(configurationScopeId)) {
-      saveAndNotifyReadyForAnalysis(Set.of(configurationScopeId));
-      return true;
-    }
-    return false;
-  }
-
-  private void saveAndNotifyReadyForAnalysis(Set<String> configScopeIds) {
-    var scopeIdsThatBecameReady = new HashSet<String>();
-    configScopeIds.forEach(configScopeId -> {
-      if (analysisReadinessByConfigScopeId.get(configScopeId) != Boolean.TRUE) {
-        analysisReadinessByConfigScopeId.put(configScopeId, Boolean.TRUE);
-        scopeIdsThatBecameReady.add(configScopeId);
+    var readyConfigScopeIds = new HashSet<String>();
+    var scopeThatBecameReady = new HashSet<String>();
+    var scopeThatBecameNotReady = new HashSet<String>();
+    configurationScopeIds.forEach(configScopeId -> {
+      var readyForAnalysis = isReadyForAnalysis(configScopeId);
+      var wasReady = analysisReadinessByConfigScopeId.put(configScopeId, readyForAnalysis);
+      if (readyForAnalysis && !Boolean.TRUE.equals(wasReady)) {
+        scopeThatBecameReady.add(configScopeId);
+      } else if (!readyForAnalysis && Boolean.TRUE.equals(wasReady)) {
+        scopeThatBecameNotReady.add(configScopeId);
+      }
+      if (readyForAnalysis) {
+        readyConfigScopeIds.add(configScopeId);
       }
     });
-    if (!scopeIdsThatBecameReady.isEmpty()) {
-      reanalyseOpenFiles(scopeIdsThatBecameReady::contains);
-      scopeIdsThatBecameReady.forEach(scopeId -> {
+    if (!scopeThatBecameReady.isEmpty()) {
+      scopeThatBecameReady.forEach(scopeId -> {
         var scheduler = schedulerCache.getOrCreateAnalysisScheduler(scopeId);
         if (scheduler != null) {
           scheduler.wakeUp();
         }
       });
-      client.didChangeAnalysisReadiness(new DidChangeAnalysisReadinessParams(scopeIdsThatBecameReady, true));
+      client.didChangeAnalysisReadiness(new DidChangeAnalysisReadinessParams(scopeThatBecameReady, true));
+    }
+    if (!scopeThatBecameNotReady.isEmpty()) {
+      client.didChangeAnalysisReadiness(new DidChangeAnalysisReadinessParams(scopeThatBecameNotReady, false));
     }
+    reanalyseOpenFiles(readyConfigScopeIds::contains);
   }
 
   private boolean isReadyForAnalysis(String configScopeId) {
@@ -797,8 +783,7 @@ private void analysisStarted(String configurationScopeId, UUID analysisId, List<
 
   private CompletableFuture<AnalysisResult> schedule(String configScopeId, AnalyzeCommand command, UUID analysisId, ArrayList<RawIssue> rawIssues,
     boolean shouldFetchServerIssues, @Nullable Trace trace) {
-    var scheduler = startChild(trace, "getOrCreateAnalysisScheduler", "schedule", () ->
-      schedulerCache.getOrCreateAnalysisScheduler(configScopeId, command.getTrace()));
+    var scheduler = startChild(trace, "getOrCreateAnalysisScheduler", "schedule", () -> schedulerCache.getOrCreateAnalysisScheduler(configScopeId, command.getTrace()));
     startChild(trace, "post", "schedule", () -> scheduler.post(command));
     var result = command.getFutureResult();
     result.exceptionally(exception -> {

its/tests/src/test/java/its/FileExclusionTests.java

@@ -213,10 +213,8 @@ void should_respect_exclusion_settings_on_SQ() {
 
   private static void forceBackendToPullSettings(String configScopeId, String projectKey) {
     // The only way to force a sync of the storage is to unbind/rebind
-    analysisReadinessByConfigScopeId.clear();
     backend.getConfigurationService().didUpdateBinding(new DidUpdateBindingParams(configScopeId, new BindingConfigurationDto(null, null, true)));
     backend.getConfigurationService().didUpdateBinding(new DidUpdateBindingParams(configScopeId, new BindingConfigurationDto(CONNECTION_ID, projectKey, true)));
-    await().atMost(1, MINUTES).untilAsserted(() -> assertThat(analysisReadinessByConfigScopeId).containsEntry(configScopeId, true));
   }
 
   private static SonarLintRpcClientDelegate newDummySonarLintClient() {

its/tests/src/test/java/its/SonarQubeDeveloperEditionTests.java

@@ -555,21 +555,17 @@ void shouldHonorServerSideSettings() {
 
       rpcClientLogs.clear();
       didSynchronizeConfigurationScopes.clear();
-      analysisReadinessByConfigScopeId.clear();
       // Override default file suffixes in global props so that input file is not considered as a Java file
       setSettingsMultiValue(null, "sonar.java.file.suffixes", ".foo");
       backend.getConfigurationService().didUpdateBinding(new DidUpdateBindingParams(configScopeId, new BindingConfigurationDto(CONNECTION_ID, projectKey, false)));
-      await().untilAsserted(() -> assertThat(analysisReadinessByConfigScopeId).containsEntry(configScopeId, true));
       await().untilAsserted(() -> assertThat(rpcClientLogs.stream().anyMatch(s -> s.getMessage().equals("Stored project analyzer configuration"))).isTrue());
 
       analyzeFileAndVerifyNoIssues(configScopeId, "sample-java", "src/main/java/foo/Foo.java");
 
       rpcClientLogs.clear();
-      analysisReadinessByConfigScopeId.clear();
       // Override default file suffixes in project props so that input file is considered as a Java file again
       setSettingsMultiValue(projectKey, "sonar.java.file.suffixes", ".java");
       backend.getConfigurationService().didUpdateBinding(new DidUpdateBindingParams(configScopeId, new BindingConfigurationDto(CONNECTION_ID, projectKey, true)));
-      await().untilAsserted(() -> assertThat(analysisReadinessByConfigScopeId).containsEntry(configScopeId, true));
       await().untilAsserted(() -> assertThat(rpcClientLogs.stream().anyMatch(s -> s.getMessage().equals("Stored project analyzer configuration"))).isTrue());
 
       rawIssues = analyzeFile(configScopeId, "sample-java", "src/main/java/foo/Foo.java");

medium-tests/src/test/java/mediumtest/analysis/AnalysisReadinessMediumTests.java

@@ -29,9 +29,13 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.io.TempDir;
 import org.sonarsource.sonarlint.core.commons.LogTestStartAndEnd;
+import org.sonarsource.sonarlint.core.rpc.protocol.backend.config.binding.BindingConfigurationDto;
+import org.sonarsource.sonarlint.core.rpc.protocol.backend.config.binding.BindingMode;
+import org.sonarsource.sonarlint.core.rpc.protocol.backend.config.binding.DidUpdateBindingParams;
 import org.sonarsource.sonarlint.core.rpc.protocol.backend.config.scope.ConfigurationScopeDto;
 import org.sonarsource.sonarlint.core.rpc.protocol.backend.config.scope.DidAddConfigurationScopesParams;
 import org.sonarsource.sonarlint.core.rpc.protocol.backend.file.DidOpenFileParams;
+import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.RaisedFindingDto;
 import org.sonarsource.sonarlint.core.rpc.protocol.common.ClientFileDto;
 import org.sonarsource.sonarlint.core.rpc.protocol.common.IssueSeverity;
 import org.sonarsource.sonarlint.core.rpc.protocol.common.Language;
@@ -43,6 +47,7 @@
 import static org.awaitility.Awaitility.await;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
@@ -110,6 +115,74 @@ void it_should_change_readiness_and_analyze_xml_file_in_connected_mode(SonarLint
     assertThat(publishedIssues).containsOnlyKeys(fileUri);
   }
 
+  @SonarLintTest
+  void it_should_reanalyse_open_files_after_unbinding(SonarLintTestHarness harness, @TempDir Path baseDir) {
+    var filePath = createFile(baseDir, "pom.xml",
+      """
+        <?xml version="1.0" encoding="UTF-8"?>
+        <project>
+          <modelVersion>4.0.0</modelVersion>
+          <groupId>com.foo</groupId>
+          <artifactId>bar</artifactId>
+          <version>${pom.version}</version>
+        </project>""");
+    var fileUri = filePath.toUri();
+    var server = harness.newFakeSonarQubeServer()
+      .withPlugin(TestPlugin.XML)
+      .withProject("projectKey", project -> project.withQualityProfile("qp"))
+      .withProject("projectKey2", project -> project.withQualityProfile("qp2"))
+      .withQualityProfile("qp", qualityProfile -> qualityProfile.withLanguage("xml").withActiveRule("xml:S3421", activeRule -> activeRule.withSeverity(IssueSeverity.MAJOR)))
+      .withQualityProfile("qp2")
+      .start();
+    var client = harness.newFakeClient()
+      .withInitialFs(CONFIG_SCOPE_ID, baseDir, List.of(new ClientFileDto(fileUri, baseDir.relativize(filePath), CONFIG_SCOPE_ID, false, null, filePath, null, null, true)))
+      .build();
+    var backend = harness.newBackend()
+      .withSonarQubeConnection("connectionId", server.baseUrl())
+      .withBoundConfigScope(CONFIG_SCOPE_ID, "connectionId", "projectKey")
+      .withBackendCapability(FULL_SYNCHRONIZATION)
+      .withExtraEnabledLanguagesInConnectedMode(Language.XML)
+      .start(client);
+
+    verify(client, never()).didChangeAnalysisReadiness(Set.of(CONFIG_SCOPE_ID), true);
+
+    // File opened but not analyzed since analysis is not ready yet
+    backend.getFileService().didOpenFile(new DidOpenFileParams(CONFIG_SCOPE_ID, fileUri));
+    verify(client, never()).raiseIssues(eq(CONFIG_SCOPE_ID), any(), eq(false), any());
+
+    client.waitForSynchronization();
+
+    // analysis is ready
+    await().atMost(1, TimeUnit.SECONDS)
+      .untilAsserted(() -> verify(client).didChangeAnalysisReadiness(Set.of(CONFIG_SCOPE_ID), true));
+    await().atMost(2, TimeUnit.SECONDS).untilAsserted(() -> assertThat(client.getRaisedIssuesForScopeIdAsList(CONFIG_SCOPE_ID)).isNotEmpty());
+
+    var publishedIssues = getPublishedIssues(client, CONFIG_SCOPE_ID);
+    assertThat(publishedIssues).containsOnlyKeys(fileUri);
+    clearInvocations(client);
+
+    // bind to 2nd project
+    backend.getConfigurationService()
+      .didUpdateBinding(new DidUpdateBindingParams(CONFIG_SCOPE_ID, new BindingConfigurationDto("connectionId", "projectKey2", true), BindingMode.MANUAL, null));
+
+    // analysis becomes not ready after binding change
+    verify(client, timeout(200)).didChangeAnalysisReadiness(Set.of(CONFIG_SCOPE_ID), false);
+
+    client.waitForSynchronization();
+
+    // analysis is ready, no issues are raised by the second QP
+    await().atMost(1, TimeUnit.SECONDS)
+      .untilAsserted(() -> verify(client).didChangeAnalysisReadiness(Set.of(CONFIG_SCOPE_ID), true));
+    await().atMost(2, TimeUnit.SECONDS).untilAsserted(() -> assertThat(client.getRaisedIssuesForScopeIdAsList(CONFIG_SCOPE_ID)).isEmpty());
+
+    // bind again to 1st project that should be ready
+    backend.getConfigurationService()
+      .didUpdateBinding(new DidUpdateBindingParams(CONFIG_SCOPE_ID, new BindingConfigurationDto("connectionId", "projectKey", true), BindingMode.MANUAL, null));
+
+    await().atMost(2, TimeUnit.SECONDS).untilAsserted(() -> assertThat(client.getRaisedIssuesForScopeIdAsList(CONFIG_SCOPE_ID))
+      .extracting(RaisedFindingDto::getRuleKey).containsExactly("xml:S3421"));
+  }
+
   private static Path createFile(Path folderPath, String fileName, String content) {
     var filePath = folderPath.resolve(fileName);
     try {

test-utils/src/main/java/org/sonarsource/sonarlint/core/test/utils/server/ServerFixture.java

@@ -709,6 +709,10 @@ public SonarQubeServerBuilder withProject(String projectKey, UnaryOperator<Serve
       return this;
     }
 
+    public SonarQubeServerBuilder withQualityProfile(String qualityProfileKey) {
+      return withQualityProfile(qualityProfileKey, UnaryOperator.identity());
+    }
+
     public SonarQubeServerBuilder withQualityProfile(String qualityProfileKey, UnaryOperator<ServerQualityProfileBuilder> qualityProfileBuilder) {
       var builder = new ServerQualityProfileBuilder(null);
       this.qualityProfilesByKey.put(qualityProfileKey, qualityProfileBuilder.apply(builder));