SONAR-8281 Recompute coverage measures from line data to ensure consistency

5 files changed, 98 insertions, 60 deletions

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MeasuresPublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MeasuresPublisher.java
index e00f9793cb1..8eb6fe00251 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MeasuresPublisher.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/MeasuresPublisher.java
@@ -21,10 +21,13 @@ package org.sonar.scanner.report;
 
 import com.google.common.base.Function;
 import java.io.Serializable;
+import java.util.Collections;
+import java.util.Map;
 import javax.annotation.Nonnull;
 import org.sonar.api.batch.measure.Metric;
 import org.sonar.api.batch.sensor.measure.internal.DefaultMeasure;
-import org.sonar.core.metric.ScannerMetrics;
+import org.sonar.api.measures.CoreMetrics;
+import org.sonar.api.utils.KeyValueFormat;
 import org.sonar.scanner.index.BatchComponent;
 import org.sonar.scanner.index.BatchComponentCache;
 import org.sonar.scanner.protocol.output.ScannerReport;
@@ -37,6 +40,14 @@ import org.sonar.scanner.protocol.output.ScannerReportWriter;
 import org.sonar.scanner.scan.measure.MeasureCache;
 
 import static com.google.common.collect.Iterables.transform;
+import static org.sonar.api.measures.CoreMetrics.CONDITIONS_TO_COVER;
+import static org.sonar.api.measures.CoreMetrics.CONDITIONS_TO_COVER_KEY;
+import static org.sonar.api.measures.CoreMetrics.LINES_TO_COVER;
+import static org.sonar.api.measures.CoreMetrics.LINES_TO_COVER_KEY;
+import static org.sonar.api.measures.CoreMetrics.UNCOVERED_CONDITIONS;
+import static org.sonar.api.measures.CoreMetrics.UNCOVERED_CONDITIONS_KEY;
+import static org.sonar.api.measures.CoreMetrics.UNCOVERED_LINES;
+import static org.sonar.api.measures.CoreMetrics.UNCOVERED_LINES_KEY;
 
 public class MeasuresPublisher implements ReportPublisherStep {
 
@@ -87,29 +98,48 @@ public class MeasuresPublisher implements ReportPublisherStep {
 
   }
 
-  private static final class MetricToKey implements Function<Metric, String> {
-    @Override
-    public String apply(Metric input) {
-      return input.key();
-    }
-  }
-
-  private final BatchComponentCache resourceCache;
+  private final BatchComponentCache componentCache;
   private final MeasureCache measureCache;
-  private final ScannerMetrics scannerMetrics;
 
-  public MeasuresPublisher(BatchComponentCache resourceCache, MeasureCache measureCache, ScannerMetrics scannerMetrics) {
-    this.resourceCache = resourceCache;
+  public MeasuresPublisher(BatchComponentCache resourceCache, MeasureCache measureCache) {
+    this.componentCache = resourceCache;
     this.measureCache = measureCache;
-    this.scannerMetrics = scannerMetrics;
   }
 
   @Override
   public void publish(ScannerReportWriter writer) {
-    for (final BatchComponent resource : resourceCache.all()) {
-      Iterable<DefaultMeasure<?>> scannerMeasures = measureCache.byComponentKey(resource.key());
-      Iterable<ScannerReport.Measure> reportMeasures = transform(scannerMeasures, new MeasureToReportMeasure(resource));
-      writer.writeComponentMeasures(resource.batchId(), reportMeasures);
+    for (final BatchComponent component : componentCache.all()) {
+      // Recompute all coverage measures from line data to take into account the possible merge of several reports
+      DefaultMeasure<String> lineHitsMeasure = (DefaultMeasure<String>) measureCache.byMetric(component.key(), CoreMetrics.COVERAGE_LINE_HITS_DATA_KEY);
+      if (lineHitsMeasure != null) {
+        Map<Integer, Integer> lineHits = KeyValueFormat.parseIntInt(lineHitsMeasure.value());
+        measureCache.put(component.key(), LINES_TO_COVER_KEY, new DefaultMeasure<Integer>().forMetric(LINES_TO_COVER).withValue(lineHits.keySet().size()));
+        measureCache.put(component.key(), UNCOVERED_LINES_KEY,
+          new DefaultMeasure<Integer>().forMetric(UNCOVERED_LINES).withValue((int) lineHits.values()
+            .stream()
+            .filter(hit -> hit == 0)
+            .count()));
+      }
+      DefaultMeasure<String> conditionsMeasure = (DefaultMeasure<String>) measureCache.byMetric(component.key(), CoreMetrics.CONDITIONS_BY_LINE_KEY);
+      DefaultMeasure<String> coveredConditionsMeasure = (DefaultMeasure<String>) measureCache.byMetric(component.key(), CoreMetrics.COVERED_CONDITIONS_BY_LINE_KEY);
+      if (conditionsMeasure != null) {
+        Map<Integer, Integer> conditions = KeyValueFormat.parseIntInt(conditionsMeasure.value());
+        Map<Integer, Integer> coveredConditions = coveredConditionsMeasure != null ? KeyValueFormat.parseIntInt(coveredConditionsMeasure.value()) : Collections.emptyMap();
+        measureCache.put(component.key(), CONDITIONS_TO_COVER_KEY, new DefaultMeasure<Integer>().forMetric(CONDITIONS_TO_COVER).withValue(conditions
+          .values()
+          .stream()
+          .mapToInt(Integer::intValue)
+          .sum()));
+        measureCache.put(component.key(), UNCOVERED_CONDITIONS_KEY,
+          new DefaultMeasure<Integer>().forMetric(UNCOVERED_CONDITIONS)
+            .withValue((int) conditions.keySet()
+              .stream()
+              .mapToInt(line -> conditions.get(line) - coveredConditions.get(line))
+              .sum()));
+      }
+      Iterable<DefaultMeasure<?>> scannerMeasures = measureCache.byComponentKey(component.key());
+      Iterable<ScannerReport.Measure> reportMeasures = transform(scannerMeasures, new MeasureToReportMeasure(component));
+      writer.writeComponentMeasures(component.batchId(), reportMeasures);
     }
   }
 
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java
index 41478c716c1..dab6d71dcbc 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java
@@ -51,6 +51,7 @@ import org.sonar.api.batch.sensor.measure.Measure;
 import org.sonar.api.batch.sensor.measure.internal.DefaultMeasure;
 import org.sonar.api.batch.sensor.symbol.internal.DefaultSymbolTable;
 import org.sonar.api.config.Settings;
+import org.sonar.api.measures.CoreMetrics;
 import org.sonar.api.utils.KeyValueFormat;
 import org.sonar.core.metric.ScannerMetrics;
 import org.sonar.duplications.block.Block;
@@ -134,8 +135,10 @@ public class DefaultSensorStorage implements SensorStorage {
     FILE_FEEDBACK_EDGES_KEY,
     FILE_TANGLE_INDEX_KEY,
     FILE_TANGLES_KEY,
+    // SONARPHP-621
     COMMENTED_OUT_CODE_LINES_KEY);
 
+  // Some Sensors still save those metrics
   private static final List<String> COMPUTED_ON_CE_SIDE_METRICS_KEYS = Arrays.asList(
     TEST_SUCCESS_DENSITY_KEY,
     PUBLIC_DOCUMENTED_API_DENSITY_KEY);
@@ -253,37 +256,42 @@ public class DefaultSensorStorage implements SensorStorage {
       if (previousMeasure != null) {
         measureCache.put(file.key(), metric.key(), new DefaultMeasure<String>()
           .forMetric((Metric<String>) metric)
-          .withValue(KeyValueFormat.format(mergeLineMetric((String) previousMeasure.value(), (String) measure.value()))));
+          .withValue(KeyValueFormat.format(mergeCoverageLineMetric(metric, (String) previousMeasure.value(), (String) measure.value()))));
       } else {
         measureCache.put(file.key(), metric.key(), measure);
       }
     } else {
-      // Other coverage metrics are all integer values
-      DefaultMeasure<?> previousMeasure = measureCache.byMetric(file.key(), metric.key());
-      if (previousMeasure != null) {
-        measureCache.put(file.key(), metric.key(), new DefaultMeasure<Integer>()
-          .forMetric((Metric<Integer>) metric)
-          .withValue(Math.max((Integer) previousMeasure.value(), (Integer) measure.value())));
-      } else {
-        measureCache.put(file.key(), metric.key(), measure);
-      }
+      // Other coverage metrics are all integer values. Just erase value, it will be recomputed at the end anyway
+      measureCache.put(file.key(), metric.key(), measure);
     }
   }
 
   /**
-   * Merge the two line data measures, keeping max value in case they both contains a value for the same line.
+   * Merge the two line coverage data measures. For lines hits use the sum, and for conditions 
+   * keep max value in case they both contains a value for the same line.
    */
-  private Map<Integer, Integer> mergeLineMetric(String value1, String value2) {
+  private static Map<Integer, Integer> mergeCoverageLineMetric(Metric<?> metric, String value1, String value2) {
     Map<Integer, Integer> data1 = KeyValueFormat.parseIntInt(value1);
     Map<Integer, Integer> data2 = KeyValueFormat.parseIntInt(value2);
-    return Stream.of(data1, data2)
-      .map(Map::entrySet)
-      .flatMap(Collection::stream)
-      .collect(
-        Collectors.toMap(
-          Map.Entry::getKey,
-          Map.Entry::getValue,
-          Integer::max));
+    if (metric.key().equals(CoreMetrics.COVERAGE_LINE_HITS_DATA_KEY)) {
+      return Stream.of(data1, data2)
+        .map(Map::entrySet)
+        .flatMap(Collection::stream)
+        .collect(
+          Collectors.toMap(
+            Map.Entry::getKey,
+            Map.Entry::getValue,
+            Integer::sum));
+    } else {
+      return Stream.of(data1, data2)
+        .map(Map::entrySet)
+        .flatMap(Collection::stream)
+        .collect(
+          Collectors.toMap(
+            Map.Entry::getKey,
+            Map.Entry::getValue,
+            Integer::max));
+    }
   }
 
   public boolean isDeprecatedMetric(String metricKey) {
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/source/ZeroCoverageSensor.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/source/ZeroCoverageSensor.java
index 782a7976db8..0d2b51a3f70 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/source/ZeroCoverageSensor.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/source/ZeroCoverageSensor.java
@@ -23,7 +23,6 @@ import com.google.common.base.Function;
 import com.google.common.collect.Sets;
 import java.util.Map;
 import java.util.Set;
-import org.apache.commons.lang.StringUtils;
 import org.sonar.api.batch.Phase;
 import org.sonar.api.batch.fs.FileSystem;
 import org.sonar.api.batch.fs.InputFile;
@@ -38,8 +37,8 @@ import org.sonar.api.batch.sensor.measure.internal.DefaultMeasure;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.api.utils.KeyValueFormat;
 import org.sonar.scanner.scan.measure.MeasureCache;
+import org.sonar.scanner.sensor.coverage.CoverageExclusions;
 
-import static com.google.common.collect.Iterables.concat;
 import static com.google.common.collect.Iterables.transform;
 import static com.google.common.collect.Sets.newHashSet;
 
@@ -61,9 +60,11 @@ public final class ZeroCoverageSensor implements Sensor {
   }
 
   private final MeasureCache measureCache;
+  private final CoverageExclusions coverageExclusions;
 
-  public ZeroCoverageSensor(MeasureCache measureCache) {
+  public ZeroCoverageSensor(MeasureCache measureCache, CoverageExclusions exclusions) {
     this.measureCache = measureCache;
+    this.coverageExclusions = exclusions;
   }
 
   @Override
@@ -75,26 +76,25 @@ public final class ZeroCoverageSensor implements Sensor {
   public void execute(final SensorContext context) {
     FileSystem fs = context.fileSystem();
     for (InputFile f : fs.inputFiles(fs.predicates().hasType(Type.MAIN))) {
+      if (coverageExclusions.isExcluded(f)) {
+        continue;
+      }
       if (!isCoverageMeasuresAlreadyDefined(f)) {
         DefaultMeasure<String> execLines = (DefaultMeasure<String>) measureCache.byMetric(f.key(), CoreMetrics.EXECUTABLE_LINES_DATA_KEY);
         if (execLines != null) {
           storeZeroCoverageForEachExecutableLine(context, f, execLines);
         }
-
       }
     }
   }
 
   private static void storeZeroCoverageForEachExecutableLine(final SensorContext context, InputFile f, DefaultMeasure<String> execLines) {
     NewCoverage newCoverage = context.newCoverage().onFile(f);
-    Map<Integer, String> lineMeasures = KeyValueFormat.parseIntString((String) execLines.value());
-    for (Map.Entry<Integer, String> lineMeasure : lineMeasures.entrySet()) {
+    Map<Integer, Integer> lineMeasures = KeyValueFormat.parseIntInt((String) execLines.value());
+    for (Map.Entry<Integer, Integer> lineMeasure : lineMeasures.entrySet()) {
       int lineIdx = lineMeasure.getKey();
-      if (lineIdx <= f.lines()) {
-        String value = lineMeasure.getValue();
-        if (StringUtils.isNotEmpty(value) && Integer.parseInt(value) > 0) {
-          newCoverage.lineHits(lineIdx, 0);
-        }
+      if (lineIdx <= f.lines() && lineMeasure.getValue() > 0) {
+        newCoverage.lineHits(lineIdx, 0);
       }
     }
     newCoverage.save();
@@ -103,9 +103,7 @@ public final class ZeroCoverageSensor implements Sensor {
   private boolean isCoverageMeasuresAlreadyDefined(InputFile f) {
     Set<String> metricKeys = newHashSet(transform(measureCache.byComponentKey(f.key()), new MeasureToMetricKey()));
     Function<Metric, String> metricToKey = new MetricToKey();
-    Set<String> allCoverageMetricKeys = newHashSet(concat(transform(CoverageType.UNIT.allMetrics(), metricToKey),
-      transform(CoverageType.IT.allMetrics(), metricToKey),
-      transform(CoverageType.OVERALL.allMetrics(), metricToKey)));
+    Set<String> allCoverageMetricKeys = newHashSet(transform(CoverageType.UNIT.allMetrics(), metricToKey));
     return !Sets.intersection(metricKeys, allCoverageMetricKeys).isEmpty();
   }
 
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/coverage/CoverageMediumTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/coverage/CoverageMediumTest.java
index 0f3c28a14f0..f2bc435c8fb 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/coverage/CoverageMediumTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/coverage/CoverageMediumTest.java
@@ -105,9 +105,9 @@ public class CoverageMediumTest {
     File xooFile = new File(srcDir, "sample.xoo");
     FileUtils.write(xooFile, "function foo() {\n  if (a && b) {\nalert('hello');\n}\n}");
     File xooUtCoverageFile = new File(srcDir, "sample.xoo.coverage");
-    FileUtils.write(xooUtCoverageFile, "2:2:2:2");
+    FileUtils.write(xooUtCoverageFile, "2:2:2:2\n4:0");
     File xooItCoverageFile = new File(srcDir, "sample.xoo.itcoverage");
-    FileUtils.write(xooItCoverageFile, "2:2:2:1\n3:1");
+    FileUtils.write(xooItCoverageFile, "2:2:2:1\n3:1\n5:0");
 
     TaskResult result = tester.newTask()
       .properties(ImmutableMap.<String, String>builder()
@@ -128,11 +128,14 @@ public class CoverageMediumTest {
     assertThat(result.coverageFor(file, 3).getHits()).isTrue();
 
     Map<String, List<org.sonar.scanner.protocol.output.ScannerReport.Measure>> allMeasures = result.allMeasures();
-    assertThat(allMeasures.get("com.foo.project:src/sample.xoo")).extracting("metricKey", "intValue.value")
-      .contains(tuple(CoreMetrics.LINES_TO_COVER_KEY, 2),
-        tuple(CoreMetrics.UNCOVERED_LINES_KEY, 0),
-        tuple(CoreMetrics.CONDITIONS_TO_COVER_KEY, 2),
-        tuple(CoreMetrics.UNCOVERED_CONDITIONS_KEY, 1));
+    assertThat(allMeasures.get("com.foo.project:src/sample.xoo")).extracting("metricKey", "intValue.value", "stringValue.value")
+      .contains(tuple(CoreMetrics.LINES_TO_COVER_KEY, 4, ""), // 2, 3, 4, 5
+        tuple(CoreMetrics.UNCOVERED_LINES_KEY, 2, ""), // 4, 5
+        tuple(CoreMetrics.CONDITIONS_TO_COVER_KEY, 2, ""), // 2 x 2
+        tuple(CoreMetrics.UNCOVERED_CONDITIONS_KEY, 0, ""),
+        tuple(CoreMetrics.COVERAGE_LINE_HITS_DATA_KEY, 0, "2=4;3=1;4=0;5=0"),
+        tuple(CoreMetrics.CONDITIONS_BY_LINE_KEY, 0, "2=2"),
+        tuple(CoreMetrics.COVERED_CONDITIONS_BY_LINE_KEY, 0, "2=2"));
   }
 
   @Test
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MeasuresPublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MeasuresPublisherTest.java
index 7f264b74152..37edc1a0389 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MeasuresPublisherTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/MeasuresPublisherTest.java
@@ -31,7 +31,6 @@ import org.junit.rules.TemporaryFolder;
 import org.sonar.api.batch.sensor.measure.internal.DefaultMeasure;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.api.resources.Project;
-import org.sonar.core.metric.ScannerMetrics;
 import org.sonar.core.util.CloseableIterator;
 import org.sonar.scanner.index.BatchComponentCache;
 import org.sonar.scanner.protocol.output.ScannerReport;
@@ -70,7 +69,7 @@ public class MeasuresPublisherTest {
     resourceCache.add(sampleFile, null);
     measureCache = mock(MeasureCache.class);
     when(measureCache.byComponentKey(anyString())).thenReturn(Collections.<DefaultMeasure<?>>emptyList());
-    publisher = new MeasuresPublisher(resourceCache, measureCache, new ScannerMetrics());
+    publisher = new MeasuresPublisher(resourceCache, measureCache);
   }
 
   @Test