SONAR-20021 Fix Quality gate failure and issue with adhoc rule definition

2 files changed, 34 insertions, 10 deletions

diff --git a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRule.java b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRule.java
index 2edc54cf8a3..389a81361f1 100644
--- a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRule.java
+++ b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRule.java
@@ -19,7 +19,7 @@
  */
 package org.sonar.api.batch.sensor.rule.internal;
 
-import java.util.Collections;
+import java.util.EnumMap;
 import java.util.Map;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
@@ -43,6 +43,10 @@ public class DefaultAdHocRule extends DefaultStorable implements AdHocRule, NewA
   private String engineId;
   private String ruleId;
 
+  private CleanCodeAttribute cleanCodeAttribute;
+
+  private Map<SoftwareQuality, org.sonar.api.issue.impact.Severity> impacts = new EnumMap<>(SoftwareQuality.class);
+
   public DefaultAdHocRule() {
     super(null);
   }
@@ -59,6 +63,7 @@ public class DefaultAdHocRule extends DefaultStorable implements AdHocRule, NewA
 
   @Override
   public DefaultAdHocRule addDefaultImpact(SoftwareQuality softwareQuality, org.sonar.api.issue.impact.Severity severity) {
+    impacts.put(softwareQuality, severity);
     return this;
   }
 
@@ -93,8 +98,7 @@ public class DefaultAdHocRule extends DefaultStorable implements AdHocRule, NewA
     checkState(isNotBlank(engineId), "Engine id is mandatory on ad hoc rule");
     checkState(isNotBlank(ruleId), "Rule id is mandatory on ad hoc rule");
     checkState(isNotBlank(name), "Name is mandatory on every ad hoc rule");
-    checkState(severity != null, "Severity is mandatory on every ad hoc rule");
-    checkState(type != null, "Type is mandatory on every ad hoc rule");
+    checkState(!impacts.isEmpty() || (severity != null && type != null), "Impact should be provided, or Severity and Type instead");
     storage.store(this);
   }
 
@@ -105,13 +109,13 @@ public class DefaultAdHocRule extends DefaultStorable implements AdHocRule, NewA
 
   @Override
   public Map<SoftwareQuality, org.sonar.api.issue.impact.Severity> defaultImpacts() {
-    return Collections.emptyMap();
+    return impacts;
   }
 
   @CheckForNull
   @Override
   public CleanCodeAttribute cleanCodeAttribute() {
-    return null;
+    return cleanCodeAttribute;
   }
 
   @Override
@@ -145,7 +149,8 @@ public class DefaultAdHocRule extends DefaultStorable implements AdHocRule, NewA
   }
 
   @Override
-  public DefaultAdHocRule cleanCodeAttribute(CleanCodeAttribute attribute) {
+  public DefaultAdHocRule cleanCodeAttribute(CleanCodeAttribute cleanCodeAttribute) {
+    this.cleanCodeAttribute = cleanCodeAttribute;
     return this;
   }
 
diff --git a/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRuleTest.java b/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRuleTest.java
index 354a729c0bb..8cd63fe79c4 100644
--- a/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRuleTest.java
+++ b/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRuleTest.java
@@ -23,6 +23,8 @@ import org.junit.Test;
 import org.sonar.api.batch.rule.Severity;
 import org.sonar.api.batch.sensor.internal.SensorStorage;
 import org.sonar.api.batch.sensor.rule.NewAdHocRule;
+import org.sonar.api.issue.impact.SoftwareQuality;
+import org.sonar.api.rules.CleanCodeAttribute;
 import org.sonar.api.rules.RuleType;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -42,7 +44,9 @@ public class DefaultAdHocRuleTest {
       .name("name")
       .description("desc")
       .severity(Severity.BLOCKER)
-      .type(RuleType.CODE_SMELL);
+      .type(RuleType.CODE_SMELL)
+      .addDefaultImpact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.HIGH)
+      .cleanCodeAttribute(CleanCodeAttribute.CONVENTIONAL);
     rule.save();
 
     assertThat(rule.engineId()).isEqualTo("engine");
@@ -51,7 +55,8 @@ public class DefaultAdHocRuleTest {
     assertThat(rule.description()).isEqualTo("desc");
     assertThat(rule.severity()).isEqualTo(Severity.BLOCKER);
     assertThat(rule.type()).isEqualTo(RuleType.CODE_SMELL);
-
+    assertThat(rule.defaultImpacts()).containsEntry(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.HIGH);
+    assertThat(rule.cleanCodeAttribute()).isEqualTo(CleanCodeAttribute.CONVENTIONAL);
     verify(storage).store(any(DefaultAdHocRule.class));
   }
 
@@ -70,6 +75,20 @@ public class DefaultAdHocRuleTest {
   }
 
   @Test
+  public void type_and_severity_are_optional() {
+    SensorStorage storage = mock(SensorStorage.class);
+    new DefaultAdHocRule(storage)
+      .engineId("engine")
+      .ruleId("ruleId")
+      .name("name")
+      .addDefaultImpact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.HIGH)
+      .save();
+
+    verify(storage).store(any(DefaultAdHocRule.class));
+  }
+
+
+  @Test
   public void fail_to_store_if_no_engine_id() {
     SensorStorage storage = mock(SensorStorage.class);
     NewAdHocRule rule = new DefaultAdHocRule(storage)
@@ -129,7 +148,7 @@ public class DefaultAdHocRuleTest {
 
     assertThatThrownBy(() -> rule.save())
       .isInstanceOf(IllegalStateException.class)
-      .hasMessageContaining("Severity is mandatory");
+      .hasMessageContaining("Impact should be provided, or Severity and Type instead");
   }
 
   @Test
@@ -144,6 +163,6 @@ public class DefaultAdHocRuleTest {
 
     assertThatThrownBy(() -> rule.save())
       .isInstanceOf(IllegalStateException.class)
-      .hasMessageContaining("Type is mandatory");
+      .hasMessageContaining("Impact should be provided, or Severity and Type instead");
   }
 }