SONAR-22914 Support 2 built-in SCA rules

6 files changed, 59 insertions, 55 deletions

diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGenerator.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGenerator.java
index 42c23791290..40c60e4523e 100644
--- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGenerator.java
+++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/AdvancedRuleDescriptionSectionsGenerator.java
@@ -22,7 +22,6 @@ package org.sonar.server.rule;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
-import org.elasticsearch.common.util.set.Sets;
 import org.jetbrains.annotations.Nullable;
 import org.sonar.api.server.rule.Context;
 import org.sonar.api.server.rule.RuleDescriptionSection;
@@ -31,42 +30,23 @@ import org.sonar.core.util.UuidFactory;
 import org.sonar.db.rule.RuleDescriptionSectionContextDto;
 import org.sonar.db.rule.RuleDescriptionSectionDto;
 
-import static org.sonar.api.rules.RuleType.*;
-
 public class AdvancedRuleDescriptionSectionsGenerator implements RuleDescriptionSectionsGenerator {
   private final UuidFactory uuidFactory;
-  private final LegacyIssueRuleDescriptionSectionsGenerator legacyIssueRuleDescriptionSectionsGenerator;
 
-  public AdvancedRuleDescriptionSectionsGenerator(UuidFactory uuidFactory, LegacyIssueRuleDescriptionSectionsGenerator legacyIssueRuleDescriptionSectionsGenerator) {
+  public AdvancedRuleDescriptionSectionsGenerator(UuidFactory uuidFactory) {
     this.uuidFactory = uuidFactory;
-    this.legacyIssueRuleDescriptionSectionsGenerator = legacyIssueRuleDescriptionSectionsGenerator;
   }
 
   @Override
   public boolean isGeneratorForRule(RulesDefinition.Rule rule) {
-    return !rule.ruleDescriptionSections().isEmpty() && skipHotspotRulesForSonar16635(rule);
-  }
-
-  private static boolean skipHotspotRulesForSonar16635(RulesDefinition.Rule rule) {
-    return !SECURITY_HOTSPOT.equals(rule.type());
+    return !rule.ruleDescriptionSections().isEmpty();
   }
 
   @Override
   public Set<RuleDescriptionSectionDto> generateSections(RulesDefinition.Rule rule) {
-    Set<RuleDescriptionSectionDto> advancedSections = rule.ruleDescriptionSections().stream()
+    return rule.ruleDescriptionSections().stream()
       .map(this::toRuleDescriptionSectionDto)
       .collect(Collectors.toSet());
-    return addLegacySectionToAdvancedSections(advancedSections, rule);
-  }
-
-  /**
-   * This was done to preserve backward compatibility with SonarLint until they stop using htmlDesc field in api/rules/[show|search] endpoints, see SONAR-16635
-   * @deprecated the method should be removed once SonarLint supports rules.descriptionSections fields, I.E in 10.x
-   */
-  @Deprecated(since = "9.6", forRemoval = true)
-  private Set<RuleDescriptionSectionDto> addLegacySectionToAdvancedSections(Set<RuleDescriptionSectionDto> advancedSections, RulesDefinition.Rule rule) {
-    Set<RuleDescriptionSectionDto> legacySection = legacyIssueRuleDescriptionSectionsGenerator.generateSections(rule);
-    return Sets.union(advancedSections, legacySection);
   }
 
   private RuleDescriptionSectionDto toRuleDescriptionSectionDto(RuleDescriptionSection section) {
diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/LegacyHotspotRuleDescriptionSectionsGenerator.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/LegacyHotspotRuleDescriptionSectionsGenerator.java
index 4d1ca46ed98..5c381578b01 100644
--- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/LegacyHotspotRuleDescriptionSectionsGenerator.java
+++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/LegacyHotspotRuleDescriptionSectionsGenerator.java
@@ -46,9 +46,7 @@ public class LegacyHotspotRuleDescriptionSectionsGenerator implements RuleDescri
 
   @Override
   public boolean isGeneratorForRule(RulesDefinition.Rule rule) {
-    // To prevent compatibility issues with SonarLint, this Generator is used for all hotspots rules, regardless of if they expose advanced sections or not. See SONAR-16635.
-    // In the future, the generator should not be used for advanced rules (add condition && rule.ruleDescriptionSections().isEmpty())
-    return SECURITY_HOTSPOT.equals(rule.type());
+    return SECURITY_HOTSPOT.equals(rule.type()) && rule.ruleDescriptionSections().isEmpty();
   }
 
   @Override
@@ -115,7 +113,6 @@ public class LegacyHotspotRuleDescriptionSectionsGenerator implements RuleDescri
       .collect(Collectors.toSet());
   }
 
-
   private static String[] extractSection(String beginning, String description) {
     String endSection = "<h2>";
     int beginningIndex = description.indexOf(beginning);
diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java
index 55fc93dcc70..67c6d2ecedb 100644
--- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java
+++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java
@@ -19,8 +19,10 @@
  */
 package org.sonar.server.rule;
 
-import org.sonar.api.server.rule.RulesDefinition;
+import java.util.Objects;
+import java.util.function.Predicate;
 import org.sonar.api.impl.server.RulesDefinitionContext;
+import org.sonar.api.server.rule.RulesDefinition;
 import org.sonar.server.plugins.ServerPluginRepository;
 import org.springframework.beans.factory.annotation.Autowired;
 
@@ -30,13 +32,13 @@ import org.springframework.beans.factory.annotation.Autowired;
  */
 public class RuleDefinitionsLoader {
 
-  private final RulesDefinition[] pluginDefs;
+  private final RulesDefinition[] rulesDefinitions;
   private final ServerPluginRepository serverPluginRepository;
 
   @Autowired(required = false)
-  public RuleDefinitionsLoader(ServerPluginRepository serverPluginRepository, RulesDefinition[] pluginDefs) {
+  public RuleDefinitionsLoader(ServerPluginRepository serverPluginRepository, RulesDefinition[] rulesDefinitions) {
     this.serverPluginRepository = serverPluginRepository;
-    this.pluginDefs = pluginDefs;
+    this.rulesDefinitions = rulesDefinitions;
   }
 
   /**
@@ -47,11 +49,22 @@ public class RuleDefinitionsLoader {
     this(serverPluginRepository, new RulesDefinition[0]);
   }
 
-  public RulesDefinition.Context load() {
+  public RulesDefinition.Context loadFromPlugins() {
+    return load(Predicate.not(Objects::isNull));
+  }
+
+  public RulesDefinition.Context loadBuiltIn() {
+    return load(Objects::isNull);
+  }
+
+  private RulesDefinition.Context load(Predicate<String> pluginKeyPredicate) {
     RulesDefinition.Context context = new RulesDefinitionContext();
-    for (RulesDefinition pluginDefinition : pluginDefs) {
-      context.setCurrentPluginKey(serverPluginRepository.getPluginKey(pluginDefinition));
-      pluginDefinition.define(context);
+    for (RulesDefinition rulesDefinition : rulesDefinitions) {
+      var pluginKey = serverPluginRepository.getPluginKey(rulesDefinition);
+      if (pluginKeyPredicate.test(pluginKey)) {
+        context.setCurrentPluginKey(pluginKey);
+        rulesDefinition.define(context);
+      }
     }
     context.setCurrentPluginKey(null);
     return context;
diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorResolver.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorResolver.java
index eaba17b5b3a..75328fccf26 100644
--- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorResolver.java
+++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDescriptionSectionsGeneratorResolver.java
@@ -33,7 +33,7 @@ public class RuleDescriptionSectionsGeneratorResolver {
     this.ruleDescriptionSectionsGenerators = ruleDescriptionSectionsGenerators;
   }
 
-  public RuleDescriptionSectionsGenerator getRuleDescriptionSectionsGenerator(RulesDefinition.Rule ruleDef) {
+  RuleDescriptionSectionsGenerator getRuleDescriptionSectionsGenerator(RulesDefinition.Rule ruleDef) {
     Set<RuleDescriptionSectionsGenerator> generatorsFound = ruleDescriptionSectionsGenerators.stream()
       .filter(generator -> generator.isGeneratorForRule(ruleDef))
       .collect(toSet());
diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java
index 5197b44a792..9a022988bab 100644
--- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java
+++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java
@@ -32,7 +32,6 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.sonar.api.Startable;
-import org.sonar.api.resources.Languages;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.server.rule.RulesDefinition;
@@ -49,6 +48,7 @@ import org.sonar.db.rule.RuleDescriptionSectionDto;
 import org.sonar.db.rule.RuleDto;
 import org.sonar.db.rule.RuleRepositoryDto;
 import org.sonar.server.es.metadata.MetadataIndex;
+import org.sonar.server.plugins.DetectPluginChange;
 import org.sonar.server.qualityprofile.ActiveRuleChange;
 import org.sonar.server.qualityprofile.QProfileRules;
 import org.sonar.server.qualityprofile.index.ActiveRuleIndexer;
@@ -74,7 +74,6 @@ public class RulesRegistrant implements Startable {
   private final DbClient dbClient;
   private final RuleIndexer ruleIndexer;
   private final ActiveRuleIndexer activeRuleIndexer;
-  private final Languages languages;
   private final System2 system2;
   private final WebServerRuleFinder webServerRuleFinder;
   private final MetadataIndex metadataIndex;
@@ -83,17 +82,17 @@ public class RulesRegistrant implements Startable {
   private final NewRuleCreator newRuleCreator;
   private final QualityProfileChangesUpdater qualityProfileChangesUpdater;
   private final SonarQubeVersion sonarQubeVersion;
+  private final DetectPluginChange detectPluginChange;
 
   public RulesRegistrant(RuleDefinitionsLoader defLoader, QProfileRules qProfileRules, DbClient dbClient, RuleIndexer ruleIndexer,
-    ActiveRuleIndexer activeRuleIndexer, Languages languages, System2 system2, WebServerRuleFinder webServerRuleFinder,
+    ActiveRuleIndexer activeRuleIndexer, System2 system2, WebServerRuleFinder webServerRuleFinder,
     MetadataIndex metadataIndex, RulesKeyVerifier rulesKeyVerifier, StartupRuleUpdater startupRuleUpdater,
-    NewRuleCreator newRuleCreator, QualityProfileChangesUpdater qualityProfileChangesUpdater, SonarQubeVersion sonarQubeVersion) {
+    NewRuleCreator newRuleCreator, QualityProfileChangesUpdater qualityProfileChangesUpdater, SonarQubeVersion sonarQubeVersion, DetectPluginChange detectPluginChange) {
     this.defLoader = defLoader;
     this.qProfileRules = qProfileRules;
     this.dbClient = dbClient;
     this.ruleIndexer = ruleIndexer;
     this.activeRuleIndexer = activeRuleIndexer;
-    this.languages = languages;
     this.system2 = system2;
     this.webServerRuleFinder = webServerRuleFinder;
     this.metadataIndex = metadataIndex;
@@ -102,28 +101,40 @@ public class RulesRegistrant implements Startable {
     this.newRuleCreator = newRuleCreator;
     this.qualityProfileChangesUpdater = qualityProfileChangesUpdater;
     this.sonarQubeVersion = sonarQubeVersion;
+    this.detectPluginChange = detectPluginChange;
   }
 
   @Override
   public void start() {
     Profiler profiler = Profiler.create(LOG).startInfo("Register rules");
     try (DbSession dbSession = dbClient.openSession(true)) {
-      List<RulesDefinition.Repository> repositories = defLoader.load().repositories();
-      RulesRegistrationContext rulesRegistrationContext = RulesRegistrationContext.create(dbClient, dbSession);
-      rulesKeyVerifier.verifyRuleKeyConsistency(repositories, rulesRegistrationContext);
+      var anyPluginChanged = detectPluginChange.anyPluginChanged();
+      RulesRegistrationContext rulesRegistrationContext = RulesRegistrationContext.create(dbClient, dbSession, !anyPluginChanged);
 
-      for (RulesDefinition.ExtendedRepository repoDef : repositories) {
-        if (languages.get(repoDef.language()) != null) {
-          Set<PluginRuleUpdate> pluginRuleUpdates = registerRules(rulesRegistrationContext, repoDef.rules(), dbSession);
+      List<RulesDefinition.Repository> rulesRepositories = new ArrayList<>(defLoader.loadBuiltIn().repositories());
+      if (anyPluginChanged) {
+        LOG.info("Some plugins have changed, triggering loading of rules from plugins");
+        rulesRepositories.addAll(defLoader.loadFromPlugins().repositories());
+      }
+      rulesKeyVerifier.verifyRuleKeyConsistency(rulesRepositories, rulesRegistrationContext);
+
+      persistRepositories(dbSession, rulesRepositories, anyPluginChanged);
+
+      for (RulesDefinition.Repository repoDef : rulesRepositories) {
+        if (repoDef.language() == null) {
+          throw new IllegalStateException("Language is mandatory for repository " + repoDef.key());
+        }
+        Set<PluginRuleUpdate> pluginRuleUpdates = registerRules(rulesRegistrationContext, repoDef.rules(), dbSession);
+        if (!repoDef.isExternal()) {
+          // External rules are not part of quality profiles
           qualityProfileChangesUpdater.createQprofileChangesForRuleUpdates(dbSession, pluginRuleUpdates);
-          dbSession.commit();
         }
+        dbSession.commit();
       }
       processRemainingDbRules(rulesRegistrationContext, dbSession);
-      List<ActiveRuleChange> changes = removeActiveRulesOnStillExistingRepositories(dbSession, rulesRegistrationContext, repositories);
+      List<ActiveRuleChange> changes = anyPluginChanged ? removeActiveRulesOnStillExistingRepositories(dbSession, rulesRegistrationContext, rulesRepositories) : List.of();
       dbSession.commit();
 
-      persistRepositories(dbSession, repositories);
       // FIXME lack of resiliency, active rules index is corrupted if rule index fails
       // to be updated. Only a single DB commit should be executed.
       ruleIndexer.commitAndIndex(dbSession, rulesRegistrationContext.getAllModified().map(RuleDto::getUuid).collect(Collectors.toSet()));
@@ -147,7 +158,7 @@ public class RulesRegistrant implements Startable {
     }
   }
 
-  private void persistRepositories(DbSession dbSession, List<RulesDefinition.Repository> repositories) {
+  private void persistRepositories(DbSession dbSession, List<RulesDefinition.Repository> repositories, boolean deleteMissing) {
     List<String> keys = repositories.stream().map(RulesDefinition.Repository::key).toList();
     Set<String> existingKeys = dbClient.ruleRepositoryDao().selectAllKeys(dbSession);
 
@@ -157,7 +168,9 @@ public class RulesRegistrant implements Startable {
 
     dbClient.ruleRepositoryDao().update(dbSession, dtos.getOrDefault(true, emptyList()));
     dbClient.ruleRepositoryDao().insert(dbSession, dtos.getOrDefault(false, emptyList()));
-    dbClient.ruleRepositoryDao().deleteIfKeyNotIn(dbSession, keys);
+    if (deleteMissing) {
+      dbClient.ruleRepositoryDao().deleteIfKeyNotIn(dbSession, keys);
+    }
     dbSession.commit();
   }
 
@@ -344,10 +357,10 @@ public class RulesRegistrant implements Startable {
    * Remove active rules on repositories that still exists.
    * <p/>
    * For instance, if the javascript repository do not provide anymore some rules, active rules related to this rules will be removed.
-   * But if the javascript repository do not exists anymore, then related active rules will not be removed.
+   * But if the javascript repository does not exist anymore, then related active rules will not be removed.
    * <p/>
    * The side effect of this approach is that extended repositories will not be managed the same way.
-   * If an extended repository do not exists anymore, then related active rules will be removed.
+   * If an extended repository does not exist anymore, then related active rules will be removed.
    */
   private List<ActiveRuleChange> removeActiveRulesOnStillExistingRepositories(DbSession dbSession, RulesRegistrationContext recorder, List<RulesDefinition.Repository> context) {
     Set<String> existingAndRenamedRepositories = getExistingAndRenamedRepositories(recorder, context);
diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java
index e9c6a8c2d06..7c1496773a6 100644
--- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java
+++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java
@@ -191,8 +191,9 @@ class RulesRegistrationContext {
     checkState(known.contains(ruleDto), "unknown RuleDto");
   }
 
-  static RulesRegistrationContext create(DbClient dbClient, DbSession dbSession) {
+  static RulesRegistrationContext create(DbClient dbClient, DbSession dbSession, boolean onlyBuiltIn) {
     Map<RuleKey, RuleDto> allRules = dbClient.ruleDao().selectAll(dbSession).stream()
+      .filter(rule -> !onlyBuiltIn || rule.getPluginKey() == null)
       .collect(Collectors.toMap(RuleDto::getKey, Function.identity()));
     Map<String, Set<SingleDeprecatedRuleKey>> existingDeprecatedKeysById = loadDeprecatedRuleKeys(dbClient, dbSession);
     Map<String, List<RuleParamDto>> ruleParamsByRuleUuid = loadAllRuleParameters(dbClient, dbSession);