SONAR-5195 Rule templates should not define technical debt informations

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java
index 831a05f1155d8a463e4829092b195017e0ff1079..ba5870588ca0ac3fc7d0d471d1cd99d0958c60ad 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java
@@ -687,6 +687,9 @@ public interface RulesDefinition extends ServerExtension {
       if ((Strings.isNullOrEmpty(debtSubCharacteristic) && debtRemediationFunction != null) || (!Strings.isNullOrEmpty(debtSubCharacteristic) && debtRemediationFunction == null)) {
         throw new IllegalStateException(String.format("Both debt sub-characteristic and debt remediation function should be defined on rule '%s'", this));
       }
+      if (!Strings.isNullOrEmpty(debtSubCharacteristic) && template) {
+        throw new IllegalStateException(String.format("'%s' is a rule template, it should not define technical debt.", this));
+      }
     }
 
     @Override

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java
index 1027e8ba3fc68de86301394b4c6ae10935bfcd72..43a88eaffe04c283c3e57b7bd863287e420734aa 100644 (file)
--- a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java
@@ -340,4 +340,26 @@ public class RulesDefinitionTest {
     }
   }
 
+  /**
+   * SONAR-5195
+   */
+  @Test
+  public void fail_if_rule_template_define_technical_debt() {
+    RulesDefinition.NewRepository newRepo = context.createRepository("squid", "java");
+    RulesDefinition.NewRule newRule = newRepo.createRule("XPath rule")
+      .setTemplate(true)
+      .setName("Insufficient branch coverage")
+      .setHtmlDescription("This rule allows to define some homemade Java rules with help of an XPath expression.")
+      .setSeverity(Severity.MAJOR)
+      .setDebtSubCharacteristic(RulesDefinition.SubCharacteristics.UNIT_TESTS);
+    newRule.setDebtRemediationFunction(newRule.debtRemediationFunctions().linearWithOffset("1h", "10min"));
+
+    try {
+      newRepo.done();
+      fail();
+    } catch (IllegalStateException e) {
+      assertThat(e).hasMessage("'[repository=squid, key=XPath rule]' is a rule template, it should not define technical debt.");
+    }
+  }
+
 }

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/DeprecatedRulesDefinition.java b/sonar-server/src/main/java/org/sonar/server/rule/DeprecatedRulesDefinition.java
index 02a04434f313cc028a02d4c5800458884bd0d5ae..b5a822d32096be0fb901e5522ce2ae62376281a2 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/DeprecatedRulesDefinition.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/DeprecatedRulesDefinition.java
@@ -90,10 +90,11 @@ public class DeprecatedRulesDefinition implements RulesDefinition {
       }
       for (org.sonar.api.rules.Rule rule : repository.createRules()) {
         NewRule newRule = newRepository.createRule(rule.getKey());
+        boolean isTemplate = Cardinality.MULTIPLE.equals(rule.getCardinality());
         newRule.setName(ruleName(repository.getKey(), rule));
         newRule.setHtmlDescription(ruleDescription(repository.getKey(), rule));
         newRule.setInternalKey(rule.getConfigKey());
-        newRule.setTemplate(Cardinality.MULTIPLE.equals(rule.getCardinality()));
+        newRule.setTemplate(isTemplate);
         newRule.setSeverity(rule.getSeverity().toString());
         newRule.setStatus(rule.getStatus() == null ? RuleStatus.defaultStatus() : RuleStatus.valueOf(rule.getStatus()));
         newRule.setTags(rule.getTags());
@@ -103,28 +104,32 @@ public class DeprecatedRulesDefinition implements RulesDefinition {
           newParam.setDescription(paramDescription(repository.getKey(), rule.getKey(), param));
           newParam.setType(RuleParamType.parse(param.getType()));
         }
-        updateRuleDebtDefinitions(newRule, repository.getKey(), rule.getKey(), ruleDebts);
+        updateRuleDebtDefinitions(newRule, repository.getKey(), rule.getKey(), isTemplate, ruleDebts);
       }
       newRepository.done();
     }
   }
 
-  private void updateRuleDebtDefinitions(NewRule newRule, String repoKey, String ruleKey, List<RuleDebt> ruleDebts) {
+  private void updateRuleDebtDefinitions(NewRule newRule, String repoKey, String ruleKey, boolean isTemplate, List<RuleDebt> ruleDebts) {
     RuleDebt ruleDebt = findRequirement(ruleDebts, repoKey, ruleKey);
     if (ruleDebt != null) {
-      newRule.setDebtSubCharacteristic(ruleDebt.subCharacteristicKey());
-      String function = ruleDebt.function();
-      String coefficient = ruleDebt.coefficient();
-      String offset = ruleDebt.offset();
-
-      if (DebtRemediationFunction.Type.LINEAR.name().equals(function) && coefficient != null) {
-        newRule.setDebtRemediationFunction(newRule.debtRemediationFunctions().linear(coefficient));
-      } else if (DebtRemediationFunction.Type.CONSTANT_ISSUE.name().equals(function) && offset != null) {
-        newRule.setDebtRemediationFunction(newRule.debtRemediationFunctions().constantPerIssue(offset));
-      } else if (DebtRemediationFunction.Type.LINEAR_OFFSET.name().equals(function) && coefficient != null && offset != null) {
-        newRule.setDebtRemediationFunction(newRule.debtRemediationFunctions().linearWithOffset(coefficient, offset));
+      if (isTemplate) {
+        LOG.warn(String.format("'%s:%s' is a rule template, it should not define technical debt. Its debt definition will be ignored.", repoKey, ruleKey));
       } else {
-        throw new IllegalArgumentException(String.format("Debt definition on rule '%s:%s' is invalid", repoKey, ruleKey));
+        newRule.setDebtSubCharacteristic(ruleDebt.subCharacteristicKey());
+        String function = ruleDebt.function();
+        String coefficient = ruleDebt.coefficient();
+        String offset = ruleDebt.offset();
+
+        if (DebtRemediationFunction.Type.LINEAR.name().equals(function) && coefficient != null) {
+          newRule.setDebtRemediationFunction(newRule.debtRemediationFunctions().linear(coefficient));
+        } else if (DebtRemediationFunction.Type.CONSTANT_ISSUE.name().equals(function) && offset != null) {
+          newRule.setDebtRemediationFunction(newRule.debtRemediationFunctions().constantPerIssue(offset));
+        } else if (DebtRemediationFunction.Type.LINEAR_OFFSET.name().equals(function) && coefficient != null && offset != null) {
+          newRule.setDebtRemediationFunction(newRule.debtRemediationFunctions().linearWithOffset(coefficient, offset));
+        } else {
+          throw new IllegalArgumentException(String.format("Debt definition on rule '%s:%s' is invalid", repoKey, ruleKey));
+        }
       }
     }
   }

diff --git a/sonar-server/src/test/java/org/sonar/server/rule/DeprecatedRulesDefinitionTest.java b/sonar-server/src/test/java/org/sonar/server/rule/DeprecatedRulesDefinitionTest.java
index 59bfb301f1ed4822fa779998c84f866f0d88262a..367df67e9082e579472748a723e027f7d198df8c 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/rule/DeprecatedRulesDefinitionTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/DeprecatedRulesDefinitionTest.java
@@ -32,6 +32,7 @@ import org.sonar.api.rules.RuleRepository;
 import org.sonar.api.server.debt.DebtRemediationFunction;
 import org.sonar.api.server.rule.RulesDefinition;
 import org.sonar.api.utils.ValidationMessages;
+import org.sonar.check.Cardinality;
 import org.sonar.core.i18n.RuleI18nManager;
 import org.sonar.server.debt.DebtModelPluginRepository;
 import org.sonar.server.debt.DebtModelXMLExporter;
@@ -214,4 +215,45 @@ public class DeprecatedRulesDefinitionTest {
     assertThat(context.repositories()).isEmpty();
   }
 
+  /**
+   * SONAR-5195
+   */
+  @Test
+  public void remove_debt_on_rule_templates() {
+    RulesDefinition.Context context = new RulesDefinition.Context();
+
+    List<DebtModelXMLExporter.RuleDebt> ruleDebts = newArrayList(
+      new DebtModelXMLExporter.RuleDebt()
+        .setSubCharacteristicKey("MEMORY_EFFICIENCY")
+        .setRuleKey(RuleKey.of("squid", "XPath"))
+        .setFunction(DebtRemediationFunction.Type.LINEAR.name())
+        .setCoefficient("1d")
+    );
+
+    Reader javaModelReader = mock(Reader.class);
+    when(debtModelRepository.createReaderForXMLFile("java")).thenReturn(javaModelReader);
+    when(debtModelRepository.getContributingPluginList()).thenReturn(newArrayList("java"));
+    when(importer.importXML(eq(javaModelReader), any(ValidationMessages.class))).thenReturn(ruleDebts);
+
+    new DeprecatedRulesDefinition(i18n, new RuleRepository[]{new RuleRepository("squid", "java") {
+      @Override
+      public List<Rule> createRules() {
+        return newArrayList(
+          Rule.create("squid", "XPath", "XPath rule")
+            .setCardinality(Cardinality.MULTIPLE)
+            .setDescription("This rule allows to define some homemade Java rules with help of an XPath expression.")
+        );
+      }
+    }}, debtModelRepository, importer).define(context);
+
+    assertThat(context.repositories()).hasSize(1);
+    RulesDefinition.Repository repo = context.repository("squid");
+    assertThat(repo.rules()).hasSize(1);
+
+    RulesDefinition.Rule rule = repo.rule("XPath");
+    assertThat(rule).isNotNull();
+    assertThat(rule.debtSubCharacteristic()).isNull();
+    assertThat(rule.debtRemediationFunction()).isNull();
+  }
+
 }