SONAR-6887 Improve message when missing revision or date on blame

diff --git a/sonar-batch/src/main/java/org/sonar/batch/scm/DefaultBlameOutput.java b/sonar-batch/src/main/java/org/sonar/batch/scm/DefaultBlameOutput.java
index 24e1783e97b5df8e5a7629f3091b2d6a174e3583..00a4029330f52103ae71c64c14d3e9f8b4b8d31d 100644 (file)
--- a/sonar-batch/src/main/java/org/sonar/batch/scm/DefaultBlameOutput.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/scm/DefaultBlameOutput.java
@@ -70,10 +70,10 @@ class DefaultBlameOutput implements BlameOutput {
   public synchronized void blameResult(InputFile file, List<BlameLine> lines) {
     Preconditions.checkNotNull(file);
     Preconditions.checkNotNull(lines);
-    Preconditions.checkArgument(allFilesToBlame.contains(file), "It was not expected to blame file " + file.relativePath());
+    Preconditions.checkArgument(allFilesToBlame.contains(file), "It was not expected to blame file %s", file.relativePath());
 
     if (lines.size() != file.lines()) {
-      LOG.debug("Ignoring blame result since provider returned " + lines.size() + " blame lines but file " + file.relativePath() + " has " + file.lines() + " lines");
+      LOG.debug("Ignoring blame result since provider returned {} blame lines but file {} has {} lines", lines.size(), file.relativePath(), file.lines());
       return;
     }
 
@@ -82,8 +82,9 @@ class DefaultBlameOutput implements BlameOutput {
     scmBuilder.setComponentRef(batchComponent.batchId());
     Map<String, Integer> changesetsIdByRevision = new HashMap<>();
 
+    int lineId = 1;
     for (BlameLine line : lines) {
-      validateLine(line);
+      validateLine(line, lineId, file);
       Integer changesetId = changesetsIdByRevision.get(line.revision());
       if (changesetId == null) {
         addChangeset(scmBuilder, line);
@@ -91,6 +92,7 @@ class DefaultBlameOutput implements BlameOutput {
         changesetsIdByRevision.put(line.revision(), changesetId);
       }
       scmBuilder.addChangesetIndexByLine(changesetId);
+      lineId++;
     }
     writer.writeComponentChangesets(scmBuilder.build());
     allFilesToBlame.remove(file);
@@ -98,9 +100,9 @@ class DefaultBlameOutput implements BlameOutput {
     progressReport.message(count + "/" + total + " files analyzed");
   }
 
-  private static void validateLine(BlameLine line) {
-    Preconditions.checkNotNull(line.revision(), "Blame revision cannot be null");
-    Preconditions.checkNotNull(line.date(), "Blame date cannot be null");
+  private static void validateLine(BlameLine line, int lineId, InputFile file) {
+    Preconditions.checkArgument(StringUtils.isNotBlank(line.revision()), "Blame revision is blank for file %s at line %s", file.relativePath(), lineId);
+    Preconditions.checkArgument(line.date() != null, "Blame date is null for file %s at line %s", file.relativePath(), lineId);
   }
 
   private static void addChangeset(Builder scmBuilder, BlameLine line) {
@@ -131,7 +133,7 @@ class DefaultBlameOutput implements BlameOutput {
   private static String removeNonAsciiCharacters(String inputString) {
     return NON_ASCII_CHARS.matcher(inputString).replaceAll("_");
   }
-  
+
   public void finish() {
     progressReport.stop(count + "/" + total + " files analyzed");
     if (!allFilesToBlame.isEmpty()) {

diff --git a/sonar-batch/src/test/java/org/sonar/batch/scm/DefaultBlameOutputTest.java b/sonar-batch/src/test/java/org/sonar/batch/scm/DefaultBlameOutputTest.java
index e7f00149111b5a303ecd7bc95d9cb60d0253aedb..3060951b4e913ffb7712ebd60866e2a0dd2071ef 100644 (file)
--- a/sonar-batch/src/test/java/org/sonar/batch/scm/DefaultBlameOutputTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/scm/DefaultBlameOutputTest.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.batch.scm;
 
+import java.util.Arrays;
+import java.util.Date;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -30,11 +32,7 @@ import org.sonar.api.batch.scm.BlameLine;
 import org.sonar.batch.index.BatchComponent;
 import org.sonar.batch.index.BatchComponentCache;
 
-import java.util.Arrays;
-import java.util.Date;
-
 import static org.mockito.Matchers.any;
-
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -75,8 +73,8 @@ public class DefaultBlameOutputTest {
   public void shouldFailIfNullDate() {
     InputFile file = new DefaultInputFile("foo", "src/main/java/Foo.java").setLines(1);
 
-    thrown.expect(NullPointerException.class);
-    thrown.expectMessage("Blame date cannot be null");
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("Blame date is null for file src/main/java/Foo.java at line 1");
 
     new DefaultBlameOutput(null, componentCache, Arrays.<InputFile>asList(file))
       .blameResult(file, Arrays.asList(new BlameLine().revision("1").author("guy")));
@@ -86,8 +84,8 @@ public class DefaultBlameOutputTest {
   public void shouldFailIfNullRevision() {
     InputFile file = new DefaultInputFile("foo", "src/main/java/Foo.java").setLines(1);
 
-    thrown.expect(NullPointerException.class);
-    thrown.expectMessage("Blame revision cannot be null");
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("Blame revision is blank for file src/main/java/Foo.java at line 1");
 
     new DefaultBlameOutput(null, componentCache, Arrays.<InputFile>asList(file))
       .blameResult(file, Arrays.asList(new BlameLine().date(new Date()).author("guy")));