SONAR-6183 Validate that coverage measures are correct regarding line number and resource

5 files changed, 228 insertions, 34 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/deprecated/DeprecatedSensorContext.java b/sonar-batch/src/main/java/org/sonar/batch/deprecated/DeprecatedSensorContext.java
index 32837480dbe..669a467dc52 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/deprecated/DeprecatedSensorContext.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/deprecated/DeprecatedSensorContext.java
@@ -60,6 +60,7 @@ public class DeprecatedSensorContext extends DefaultSensorContext implements Sen
     this.index = index;
     this.project = project;
     this.coverageFilter = coverageFilter;
+
   }
 
   public Project getProject() {
@@ -157,12 +158,15 @@ public class DeprecatedSensorContext extends DefaultSensorContext implements Sen
 
   @Override
   public Measure saveMeasure(Resource resource, Metric metric, Double value) {
-    return saveMeasure(resource, new Measure(metric, value));
+    Measure<?> measure = new Measure(metric, value);
+    coverageFilter.validate(measure, resource.getPath());
+    return saveMeasure(resource, measure);
   }
 
   @Override
   public Measure saveMeasure(Resource resource, Measure measure) {
     Resource resourceOrProject = resourceOrProject(resource);
+
     if (coverageFilter.accept(resourceOrProject, measure)) {
       return index.addMeasure(resourceOrProject, measure);
     } else {
@@ -190,11 +194,14 @@ public class DeprecatedSensorContext extends DefaultSensorContext implements Sen
 
   @Override
   public Measure saveMeasure(InputFile inputFile, Metric metric, Double value) {
-    return saveMeasure(getResource(inputFile), metric, value);
+    Measure<?> measure = new Measure(metric, value);
+    coverageFilter.validate(measure, inputFile);
+    return saveMeasure(getResource(inputFile), measure);
   }
 
   @Override
   public Measure saveMeasure(InputFile inputFile, Measure measure) {
+    coverageFilter.validate(measure, inputFile);
     return saveMeasure(getResource(inputFile), measure);
   }
 
diff --git a/sonar-batch/src/main/java/org/sonar/batch/sensor/coverage/CoverageExclusions.java b/sonar-batch/src/main/java/org/sonar/batch/sensor/coverage/CoverageExclusions.java
index 4439043e370..f3ec090b8ff 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/sensor/coverage/CoverageExclusions.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/sensor/coverage/CoverageExclusions.java
@@ -19,13 +19,22 @@
  */
 package org.sonar.batch.sensor.coverage;
 
+import org.sonar.api.batch.fs.FileSystem;
+
+import javax.annotation.CheckForNull;
+
+import org.sonar.api.batch.fs.InputFile;
+import org.sonar.api.utils.KeyValueFormat;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
+
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.Map;
 import java.util.Set;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.CoreProperties;
@@ -42,11 +51,16 @@ public class CoverageExclusions {
 
   private final Settings settings;
   private final Set<Metric> coverageMetrics;
+  private final Set<Metric> byLineMetrics;
   private Collection<WildcardPattern> resourcePatterns;
 
-  public CoverageExclusions(Settings settings) {
+  private final FileSystem fs;
+
+  public CoverageExclusions(Settings settings, FileSystem fs) {
     this.settings = settings;
+    this.fs = fs;
     this.coverageMetrics = new HashSet<>();
+    this.byLineMetrics = new HashSet<>();
     // UT
     coverageMetrics.add(CoreMetrics.COVERAGE);
     coverageMetrics.add(CoreMetrics.LINE_COVERAGE);
@@ -90,10 +104,72 @@ public class CoverageExclusions {
     coverageMetrics.add(CoreMetrics.NEW_OVERALL_UNCOVERED_LINES);
     coverageMetrics.add(CoreMetrics.NEW_OVERALL_UNCOVERED_CONDITIONS);
 
+    byLineMetrics.add(CoreMetrics.OVERALL_CONDITIONS_BY_LINE);
+    byLineMetrics.add(CoreMetrics.OVERALL_COVERED_CONDITIONS_BY_LINE);
+    byLineMetrics.add(CoreMetrics.COVERED_CONDITIONS_BY_LINE);
+    byLineMetrics.add(CoreMetrics.CONDITIONS_BY_LINE);
+    byLineMetrics.add(CoreMetrics.IT_CONDITIONS_BY_LINE);
+    byLineMetrics.add(CoreMetrics.IT_COVERED_CONDITIONS_BY_LINE);
+
     initPatterns();
   }
 
-  public boolean accept(Resource resource, Measure measure) {
+  private boolean isLineMetrics(Metric<?> metric) {
+    return this.byLineMetrics.contains(metric);
+  }
+
+  public void validate(Measure<?> measure, InputFile inputFile) {
+    Metric<?> metric = measure.getMetric();
+
+    if (!isLineMetrics(metric)) {
+      return;
+    }
+
+    Map<Integer, Integer> m = KeyValueFormat.parseIntInt(measure.getData());
+    validatePositiveLine(m, inputFile.absolutePath());
+    validateMaxLine(m, inputFile);
+  }
+
+  @CheckForNull
+  private InputFile getInputFile(String filePath) {
+    return fs.inputFile(fs.predicates().hasRelativePath(filePath));
+  }
+
+  public void validate(Measure<?> measure, String filePath) {
+    Metric<?> metric = measure.getMetric();
+
+    if (!isLineMetrics(metric)) {
+      return;
+    }
+    
+    InputFile inputFile = getInputFile(filePath);
+
+    if (inputFile == null) {
+      throw new IllegalStateException(String.format("Can't create measure for resource '%s': resource is not indexed as a file", filePath));
+    }
+
+    validate(measure, inputFile);
+  }
+
+  private static void validateMaxLine(Map<Integer, Integer> m, InputFile inputFile) {
+    int maxLine = inputFile.lines();
+
+    for (int l : m.keySet()) {
+      if (l > maxLine) {
+        throw new IllegalStateException(String.format("Can't create measure for line %d for file '%s' with %d lines", l, inputFile.absolutePath(), maxLine));
+      }
+    }
+  }
+
+  private static void validatePositiveLine(Map<Integer, Integer> m, String filePath) {
+    for (int l : m.keySet()) {
+      if (l <= 0) {
+        throw new IllegalStateException(String.format("Measure with line %d for file '%s' must be > 0", l, filePath));
+      }
+    }
+  }
+
+  public boolean accept(Resource resource, Measure<?> measure) {
     if (isCoverageMetric(measure.getMetric())) {
       return !hasMatchingPattern(resource);
     } else {
@@ -101,7 +177,7 @@ public class CoverageExclusions {
     }
   }
 
-  private boolean isCoverageMetric(Metric metric) {
+  private boolean isCoverageMetric(Metric<?> metric) {
     return this.coverageMetrics.contains(metric);
   }
 
diff --git a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/coverage/CoverageMediumTest.java b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/coverage/CoverageMediumTest.java
index 12483ea36d0..e8c49dbd0aa 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/coverage/CoverageMediumTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/coverage/CoverageMediumTest.java
@@ -130,7 +130,6 @@ public class CoverageMediumTest {
     assertThat(allMeasures.get("com.foo.project:src/sample.xoo")).extracting("metricKey")
       .doesNotContain(CoreMetrics.LINES_TO_COVER_KEY, CoreMetrics.UNCOVERED_LINES_KEY, CoreMetrics.CONDITIONS_TO_COVER_KEY,
         CoreMetrics.COVERED_CONDITIONS_BY_LINE_KEY);
-
   }
 
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/tests/CoveragePerTestMediumTest.java b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/tests/CoveragePerTestMediumTest.java
index 19ecaeb8df2..b978e7e66bb 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/tests/CoveragePerTestMediumTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/tests/CoveragePerTestMediumTest.java
@@ -19,6 +19,11 @@
  */
 package org.sonar.batch.mediumtest.tests;
 
+import org.hamcrest.Description;
+
+import org.hamcrest.TypeSafeMatcher;
+import org.junit.Rule;
+import org.junit.rules.ExpectedException;
 import com.google.common.collect.ImmutableMap;
 import org.apache.commons.io.FileUtils;
 import org.junit.After;
@@ -37,9 +42,12 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 public class CoveragePerTestMediumTest {
 
-  @org.junit.Rule
+  @Rule
   public TemporaryFolder temp = new TemporaryFolder();
 
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   public BatchMediumTester tester = BatchMediumTester.builder()
     .registerPlugin("xoo", new XooPlugin())
     .addDefaultQProfile("xoo", "Sonar Way")
@@ -56,25 +64,36 @@ public class CoveragePerTestMediumTest {
   }
 
   @Test
-  public void coveragePerTestInReport() throws IOException {
-
-    File baseDir = temp.getRoot();
+  // SONAR-6183
+  public void invalidCoverage() throws IOException {
+    File baseDir = createTestFiles();
     File srcDir = new File(baseDir, "src");
-    srcDir.mkdir();
-    File testDir = new File(baseDir, "test");
-    testDir.mkdir();
 
-    File xooFile = new File(srcDir, "sample.xoo");
-    FileUtils.write(xooFile, "foo");
+    File coverageFile = new File(srcDir, "sample.xoo.coverage");
+    FileUtils.write(coverageFile, "0:2\n");
 
-    File xooFile2 = new File(srcDir, "sample2.xoo");
-    FileUtils.write(xooFile2, "foo");
+    exception.expect(IllegalStateException.class);
+    exception.expectMessage("Error processing line 1 of file");
+    exception.expectCause(new TypeSafeMatcher<Throwable>() {
 
-    File xooTestFile = new File(testDir, "sampleTest.xoo");
-    FileUtils.write(xooTestFile, "failure\nerror\nok\nskipped");
+      @Override
+      public void describeTo(Description description) {
+        // nothing to do
+      }
 
-    File xooTestFile2 = new File(testDir, "sample2Test.xoo");
-    FileUtils.write(xooTestFile2, "test file tests");
+      @Override
+      protected boolean matchesSafely(Throwable item) {
+        return item.getMessage().contains("Line number must be strictly positive");
+      }
+    });
+    runTask(baseDir);
+
+  }
+
+  @Test
+  public void coveragePerTestInReport() throws IOException {
+    File baseDir = createTestFiles();
+    File testDir = new File(baseDir, "test");
 
     File xooTestExecutionFile = new File(testDir, "sampleTest.xoo.test");
     FileUtils.write(xooTestExecutionFile, "some test:4:::OK:UNIT\n" +
@@ -85,18 +104,7 @@ public class CoveragePerTestMediumTest {
     FileUtils.write(xooCoveragePerTestFile, "some test;src/sample.xoo,10,11;src/sample2.xoo,1,2\n" +
       "another test;src/sample.xoo,10,20\n");
 
-    TaskResult result = tester.newTask()
-      .properties(ImmutableMap.<String, String>builder()
-        .put("sonar.task", "scan")
-        .put("sonar.projectBaseDir", baseDir.getAbsolutePath())
-        .put("sonar.projectKey", "com.foo.project")
-        .put("sonar.projectName", "Foo Project")
-        .put("sonar.projectVersion", "1.0-SNAPSHOT")
-        .put("sonar.projectDescription", "Description of Foo Project")
-        .put("sonar.sources", "src")
-        .put("sonar.tests", "test")
-        .build())
-      .start();
+    TaskResult result = runTask(baseDir);
 
     InputFile file = result.inputFile("test/sampleTest.xoo");
     org.sonar.batch.protocol.output.BatchReport.CoverageDetail someTest = result.coveragePerTestFor(file, "some test");
@@ -112,4 +120,41 @@ public class CoveragePerTestMediumTest {
     assertThat(anotherTest.getCoveredFile(0).getCoveredLineList()).containsExactly(10, 20);
   }
 
+  private TaskResult runTask(File baseDir) {
+    return tester.newTask()
+      .properties(ImmutableMap.<String, String>builder()
+        .put("sonar.task", "scan")
+        .put("sonar.projectBaseDir", baseDir.getAbsolutePath())
+        .put("sonar.projectKey", "com.foo.project")
+        .put("sonar.projectName", "Foo Project")
+        .put("sonar.projectVersion", "1.0-SNAPSHOT")
+        .put("sonar.projectDescription", "Description of Foo Project")
+        .put("sonar.sources", "src")
+        .put("sonar.tests", "test")
+        .build())
+      .start();
+  }
+
+  private File createTestFiles() throws IOException {
+    File baseDir = temp.getRoot();
+    File srcDir = new File(baseDir, "src");
+    srcDir.mkdir();
+    File testDir = new File(baseDir, "test");
+    testDir.mkdir();
+
+    File xooFile = new File(srcDir, "sample.xoo");
+    FileUtils.write(xooFile, "foo");
+
+    File xooFile2 = new File(srcDir, "sample2.xoo");
+    FileUtils.write(xooFile2, "foo");
+
+    File xooTestFile = new File(testDir, "sampleTest.xoo");
+    FileUtils.write(xooTestFile, "failure\nerror\nok\nskipped");
+
+    File xooTestFile2 = new File(testDir, "sample2Test.xoo");
+    FileUtils.write(xooTestFile2, "test file tests");
+
+    return baseDir;
+  }
+
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/sensor/coverage/CoverageExclusionsTest.java b/sonar-batch/src/test/java/org/sonar/batch/sensor/coverage/CoverageExclusionsTest.java
index 0e2efc2782a..66e508986df 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/sensor/coverage/CoverageExclusionsTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/sensor/coverage/CoverageExclusionsTest.java
@@ -19,30 +19,97 @@
  */
 package org.sonar.batch.sensor.coverage;
 
+import org.junit.rules.TemporaryFolder;
+
+import org.sonar.api.batch.fs.internal.DefaultFileSystem;
+import org.sonar.api.batch.fs.internal.DefaultInputFile;
+import com.google.common.collect.ImmutableMap;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.sonar.api.config.PropertyDefinitions;
 import org.sonar.api.config.Settings;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.api.measures.Measure;
 import org.sonar.api.resources.File;
 import org.sonar.api.resources.Resource;
+import org.sonar.api.utils.KeyValueFormat;
 import org.sonar.core.config.ExclusionProperties;
 
+import java.util.Map;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class CoverageExclusionsTest {
 
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
+  @Rule
+  public TemporaryFolder temp = new TemporaryFolder();
+
   private Settings settings;
+  private DefaultFileSystem fs;
 
   private CoverageExclusions filter;
 
   @Before
   public void createFilter() {
     settings = new Settings(new PropertyDefinitions(ExclusionProperties.all()));
-    filter = new CoverageExclusions(settings);
+    fs = new DefaultFileSystem(temp.getRoot());
+    filter = new CoverageExclusions(settings, fs);
+  }
+
+  @Test
+  public void shouldValidateStrictlyPositiveLine() {
+    DefaultInputFile file = new DefaultInputFile("module", "testfile");
+    Measure measure = mock(Measure.class);
+    Map<Integer, Integer> map = ImmutableMap.of(0, 3);
+
+    String data = KeyValueFormat.format(map);
+    when(measure.getMetric()).thenReturn(CoreMetrics.IT_CONDITIONS_BY_LINE);
+    when(measure.getData()).thenReturn(data);
+
+    fs.add(file);
+
+    exception.expect(IllegalStateException.class);
+    exception.expectMessage("must be > 0");
+    filter.validate(measure, "testfile");
+  }
+
+  @Test
+  public void shouldValidateFileExists() {
+    DefaultInputFile file = new DefaultInputFile("module", "testfile");
+    Measure measure = mock(Measure.class);
+    Map<Integer, Integer> map = ImmutableMap.of(0, 3);
+
+    String data = KeyValueFormat.format(map);
+    when(measure.getMetric()).thenReturn(CoreMetrics.IT_CONDITIONS_BY_LINE);
+    when(measure.getData()).thenReturn(data);
+
+    fs.add(file);
+
+    exception.expect(IllegalStateException.class);
+    exception.expectMessage("resource is not indexed as a file");
+    filter.validate(measure, "dummy");
+  }
+
+  @Test
+  public void shouldValidateMaxLine() {
+    DefaultInputFile file = new DefaultInputFile("module", "testfile");
+    file.setLines(10);
+    Measure measure = mock(Measure.class);
+    Map<Integer, Integer> map = ImmutableMap.of(11, 3);
+
+    String data = KeyValueFormat.format(map);
+    when(measure.getMetric()).thenReturn(CoreMetrics.COVERED_CONDITIONS_BY_LINE);
+    when(measure.getData()).thenReturn(data);
+
+    exception.expect(IllegalStateException.class);
+    filter.validate(measure, file);
   }
 
   @Test