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

plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/OneExternalIssuePerLineSensor.java

@@ -27,6 +27,8 @@ import org.sonar.api.batch.sensor.Sensor;
 import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.batch.sensor.SensorDescriptor;
 import org.sonar.api.batch.sensor.issue.NewExternalIssue;
+import org.sonar.api.issue.impact.SoftwareQuality;
+import org.sonar.api.rules.CleanCodeAttribute;
 import org.sonar.api.rules.RuleType;
 import org.sonar.xoo.Xoo;
 
@@ -63,6 +65,9 @@ public class OneExternalIssuePerLineSensor implements Sensor {
         .description("blah blah")
         .severity(Severity.BLOCKER)
         .type(RuleType.BUG)
+        .cleanCodeAttribute(CleanCodeAttribute.CLEAR)
+        .addDefaultImpact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)
+        .addDefaultImpact(SoftwareQuality.RELIABILITY, org.sonar.api.issue.impact.Severity.LOW)
         .save();
     }
   }
@@ -78,6 +83,8 @@ public class OneExternalIssuePerLineSensor implements Sensor {
           .at(file.selectLine(line))
           .message("This issue is generated on each line"))
         .severity(Severity.valueOf(SEVERITY))
+        //Overrides default impact from the adhoc rule
+        .addImpact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.HIGH)
         .remediationEffortMinutes(EFFORT)
         .type(TYPE)
         .save();

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;
   }
 

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));
   }
 
@@ -69,6 +74,20 @@ public class DefaultAdHocRuleTest {
     verify(storage).store(any(DefaultAdHocRule.class));
   }
 
+  @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);
@@ -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");
   }
 }

sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java

@@ -57,7 +57,11 @@ import org.sonar.api.batch.sensor.rule.AdHocRule;
 import org.sonar.api.batch.sensor.symbol.NewSymbolTable;
 import org.sonar.api.batch.sensor.symbol.internal.DefaultSymbolTable;
 import org.sonar.api.config.Configuration;
+import org.sonar.api.issue.impact.Severity;
+import org.sonar.api.issue.impact.SoftwareQuality;
 import org.sonar.api.measures.CoreMetrics;
+import org.sonar.api.rules.CleanCodeAttribute;
+import org.sonar.api.rules.RuleType;
 import org.sonar.api.utils.KeyValueFormat;
 import org.sonar.core.metric.ScannerMetrics;
 import org.sonar.core.util.CloseableIterator;
@@ -257,11 +261,34 @@ public class DefaultSensorStorage implements SensorStorage {
     if (description != null) {
       builder.setDescription(description);
     }
-    builder.setSeverity(Constants.Severity.valueOf(adHocRule.severity().name()));
-    builder.setType(ScannerReport.IssueType.valueOf(adHocRule.type().name()));
+
+
+    org.sonar.api.batch.rule.Severity severity = adHocRule.severity();
+    if (severity != null) {
+      builder.setSeverity(Constants.Severity.valueOf(severity.name()));
+    }
+
+    RuleType type = adHocRule.type();
+    if (type != null) {
+      builder.setType(ScannerReport.IssueType.valueOf(type.name()));
+    }
+    builder.addAllDefaultImpacts(mapImpacts(adHocRule.defaultImpacts()));
+
+    CleanCodeAttribute cleanCodeAttribute = adHocRule.cleanCodeAttribute();
+    if (cleanCodeAttribute != null) {
+      builder.setCleanCodeAttribute(cleanCodeAttribute.name());
+    }
     writer.appendAdHocRule(builder.build());
   }
 
+  private static List<ScannerReport.Impact> mapImpacts(Map<SoftwareQuality, Severity> impactsMap) {
+    return impactsMap.entrySet().stream()
+      .map(e -> ScannerReport.Impact.newBuilder()
+        .setSoftwareQuality(e.getKey().name())
+        .setSeverity(e.getValue().name()).build())
+      .collect(toList());
+  }
+
   @Override
   public void store(NewHighlighting newHighlighting) {
     DefaultHighlighting highlighting = (DefaultHighlighting) newHighlighting;

sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java

@@ -23,6 +23,7 @@ import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import org.assertj.core.groups.Tuple;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -46,12 +47,19 @@ import org.sonar.api.batch.sensor.issue.internal.DefaultExternalIssue;
 import org.sonar.api.batch.sensor.issue.internal.DefaultIssue;
 import org.sonar.api.batch.sensor.issue.internal.DefaultIssueLocation;
 import org.sonar.api.batch.sensor.measure.internal.DefaultMeasure;
+import org.sonar.api.batch.sensor.rule.internal.DefaultAdHocRule;
 import org.sonar.api.batch.sensor.symbol.internal.DefaultSymbolTable;
 import org.sonar.api.config.internal.MapSettings;
+import org.sonar.api.issue.impact.Severity;
+import org.sonar.api.issue.impact.SoftwareQuality;
 import org.sonar.api.measures.CoreMetrics;
+import org.sonar.api.rules.CleanCodeAttribute;
+import org.sonar.api.rules.RuleType;
 import org.sonar.core.metric.ScannerMetrics;
+import org.sonar.core.util.CloseableIterator;
 import org.sonar.scanner.cpd.index.SonarCpdBlockIndex;
 import org.sonar.scanner.issue.IssuePublisher;
+import org.sonar.scanner.protocol.Constants;
 import org.sonar.scanner.protocol.output.FileStructure;
 import org.sonar.scanner.protocol.output.ScannerReport;
 import org.sonar.scanner.protocol.output.ScannerReportReader;
@@ -347,4 +355,42 @@ public class DefaultSensorStorageTest {
     assertThat(contextPropertiesCache.getAll()).containsOnly(entry("foo", "bar"));
   }
 
+  @Test
+  public void store_whenAdhocRuleIsSpecified_shouldWriteAdhocRuleToReport() throws IOException {
+
+    underTest.store(new DefaultAdHocRule().ruleId("ruleId").engineId("engineId")
+      .name("name")
+      .addDefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH)
+      .addDefaultImpact(SoftwareQuality.RELIABILITY, Severity.MEDIUM)
+      .cleanCodeAttribute(CleanCodeAttribute.CLEAR)
+      .severity(org.sonar.api.batch.rule.Severity.MAJOR)
+      .type(RuleType.CODE_SMELL)
+      .description("description"));
+
+    try (CloseableIterator<ScannerReport.AdHocRule> adhocRuleIt = reportReader.readAdHocRules()) {
+      ScannerReport.AdHocRule adhocRule = adhocRuleIt.next();
+      assertThat(adhocRule).extracting(r -> r.getRuleId(), r -> r.getName(), r -> r.getSeverity(), r -> r.getType(), r -> r.getDescription())
+        .containsExactlyInAnyOrder("ruleId", "name", Constants.Severity.MAJOR, ScannerReport.IssueType.CODE_SMELL, "description");
+      assertThat(adhocRule.getDefaultImpactsList()).hasSize(2).extracting(i -> i.getSoftwareQuality(), i -> i.getSeverity())
+        .containsExactlyInAnyOrder(
+          Tuple.tuple(SoftwareQuality.MAINTAINABILITY.name(), Severity.HIGH.name()),
+          Tuple.tuple(SoftwareQuality.RELIABILITY.name(), Severity.MEDIUM.name()));
+      assertThat(adhocRule.getCleanCodeAttribute())
+        .isEqualTo(CleanCodeAttribute.CLEAR.name());
+    }
+  }
+
+  @Test
+  public void store_whenAdhocRuleIsSpecifiedWithOptionalFieldEmpty_shouldWriteAdhocRuleToReport() throws IOException {
+    underTest.store(new DefaultAdHocRule().ruleId("ruleId").engineId("engineId")
+      .name("name")
+      .description("description"));
+    try (CloseableIterator<ScannerReport.AdHocRule> adhocRuleIt = reportReader.readAdHocRules()) {
+      ScannerReport.AdHocRule adhocRule = adhocRuleIt.next();
+      assertThat(adhocRule).extracting(r -> r.getSeverity(), r -> r.getType())
+        .containsExactlyInAnyOrder(Constants.Severity.UNSET_SEVERITY, ScannerReport.IssueType.UNSET);
+      assertThat(adhocRule.getDefaultImpactsList()).isEmpty();
+      assertThat(adhocRule.getCleanCodeAttribute()).isNullOrEmpty();
+    }
+  }
 }