SONAR-5677 Fail with a clear message when trying to blame a locally modified file

5 files changed, 122 insertions, 4 deletions

diff --git a/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/GitBlameCommand.java b/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/GitBlameCommand.java
index 5e5b0370cca..be50267fe90 100644
--- a/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/GitBlameCommand.java
+++ b/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/GitBlameCommand.java
@@ -26,11 +26,13 @@ import org.sonar.api.batch.InstantiationStrategy;
 import org.sonar.api.batch.fs.FileSystem;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.scm.BlameCommand;
+import org.sonar.api.batch.scm.BlameLine;
 import org.sonar.api.utils.command.Command;
 import org.sonar.api.utils.command.CommandExecutor;
 import org.sonar.api.utils.command.StreamConsumer;
 
 import java.io.File;
+import java.util.List;
 
 @InstantiationStrategy(InstantiationStrategy.PER_BATCH)
 public class GitBlameCommand implements BlameCommand, BatchComponent {
@@ -52,14 +54,19 @@ public class GitBlameCommand implements BlameCommand, BatchComponent {
     for (InputFile inputFile : files) {
       String filename = inputFile.relativePath();
       Command cl = createCommandLine(fs.baseDir(), filename);
-      GitBlameConsumer consumer = new GitBlameConsumer();
+      GitBlameConsumer consumer = new GitBlameConsumer(filename);
       StringStreamConsumer stderr = new StringStreamConsumer();
 
       int exitCode = execute(cl, consumer, stderr);
       if (exitCode != 0) {
         throw new IllegalStateException("The git blame command [" + cl.toString() + "] failed: " + stderr.getOutput());
       }
-      result.add(inputFile, consumer.getLines());
+      List<BlameLine> lines = consumer.getLines();
+      if (lines.size() == inputFile.lines() - 1) {
+        // SONARPLUGINS-3097 Git do not report blame on last empty line
+        lines.add(lines.get(lines.size() - 1));
+      }
+      result.add(inputFile, lines);
     }
   }
 
diff --git a/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/GitBlameConsumer.java b/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/GitBlameConsumer.java
index 50069a0640f..987c37e8d6f 100644
--- a/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/GitBlameConsumer.java
+++ b/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/GitBlameConsumer.java
@@ -20,6 +20,7 @@
 package org.sonar.plugins.scm.git;
 
 import com.google.common.annotations.VisibleForTesting;
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.batch.scm.BlameLine;
 import org.sonar.api.utils.command.StreamConsumer;
 
@@ -56,6 +57,11 @@ public class GitBlameConsumer implements StreamConsumer {
   private String author = null;
   private String committer = null;
   private Date time = null;
+  private final String filename;
+
+  public GitBlameConsumer(String filename) {
+    this.filename = filename;
+  }
 
   public void consumeLine(String line) {
     if (line == null) {
@@ -126,6 +132,10 @@ public class GitBlameConsumer implements StreamConsumer {
     if (parts.length >= 1) {
       revision = parts[0];
 
+      if (StringUtils.containsOnly(revision, "0")) {
+        throw new IllegalStateException("Unable to blame file " + filename + ". Is file commited?");
+      }
+
       BlameLine oldLine = commitInfo.get(revision);
 
       if (oldLine != null) {
diff --git a/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/JGitBlameCommand.java b/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/JGitBlameCommand.java
index 4f0e99af560..1c586c142ff 100644
--- a/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/JGitBlameCommand.java
+++ b/plugins/sonar-git-plugin/src/main/java/org/sonar/plugins/scm/git/JGitBlameCommand.java
@@ -20,6 +20,7 @@
 package org.sonar.plugins.scm.git;
 
 import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
 import org.sonar.api.BatchComponent;
 import org.sonar.api.batch.InstantiationStrategy;
 import org.sonar.api.batch.fs.FileSystem;
@@ -28,6 +29,7 @@ import org.sonar.api.batch.scm.BlameCommand;
 import org.sonar.api.batch.scm.BlameLine;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -45,6 +47,9 @@ public class JGitBlameCommand implements BlameCommand, BatchComponent {
         org.eclipse.jgit.blame.BlameResult blameResult = git.blame().setFilePath(filename).call();
         List<BlameLine> lines = new ArrayList<BlameLine>();
         for (int i = 0; i < blameResult.getResultContents().size(); i++) {
+          if (blameResult.getSourceAuthor(i) == null || blameResult.getSourceCommit(i) == null || blameResult.getSourceCommitter(i) == null) {
+            throw new IllegalStateException("Unable to blame file " + inputFile.relativePath() + ". No blame info at line " + (i + 1) + ". Is file commited?");
+          }
           lines.add(new org.sonar.api.batch.scm.BlameLine(blameResult.getSourceAuthor(i).getWhen(),
             blameResult.getSourceCommit(i).getName(),
             blameResult.getSourceAuthor(i).getEmailAddress(),
@@ -52,8 +57,10 @@ public class JGitBlameCommand implements BlameCommand, BatchComponent {
         }
         result.add(inputFile, lines);
       }
-    } catch (Exception e) {
-      throw new IllegalStateException("JGit blame failure!", e);
+    } catch (IOException e) {
+      throw new IllegalStateException("Unable to open Git repository", e);
+    } catch (GitAPIException e) {
+      throw new IllegalStateException("Unable to blame", e);
     } finally {
       if (git != null && git.getRepository() != null) {
         git.getRepository().close();
diff --git a/plugins/sonar-git-plugin/src/test/java/org/sonar/plugins/scm/git/GitBlameCommandTest.java b/plugins/sonar-git-plugin/src/test/java/org/sonar/plugins/scm/git/GitBlameCommandTest.java
index 1005ac05723..f393c47c1c6 100644
--- a/plugins/sonar-git-plugin/src/test/java/org/sonar/plugins/scm/git/GitBlameCommandTest.java
+++ b/plugins/sonar-git-plugin/src/test/java/org/sonar/plugins/scm/git/GitBlameCommandTest.java
@@ -106,6 +106,45 @@ public class GitBlameCommandTest {
   }
 
   @Test
+  public void shouldFailOnFileWithLocalModification() throws IOException {
+    File source = new File(baseDir, "src/foo.xoo");
+    FileUtils.write(source, "sample content");
+    DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setAbsolutePath(new File(baseDir, "src/foo.xoo").getAbsolutePath());
+    fs.add(inputFile);
+
+    BlameResult result = mock(BlameResult.class);
+    CommandExecutor commandExecutor = mock(CommandExecutor.class);
+
+    when(commandExecutor.execute(any(Command.class), any(StreamConsumer.class), any(StreamConsumer.class), anyLong())).thenAnswer(new Answer<Integer>() {
+
+      @Override
+      public Integer answer(InvocationOnMock invocation) throws Throwable {
+        StreamConsumer outConsumer = (StreamConsumer) invocation.getArguments()[1];
+        outConsumer.consumeLine("000000000000000000000000000000000000000 68 54 1");
+        outConsumer.consumeLine("author Not Committed Yet");
+        outConsumer.consumeLine("author-mail <not.committed.yet>");
+        outConsumer.consumeLine("author-time 1312534171");
+        outConsumer.consumeLine("author-tz +0200");
+        outConsumer.consumeLine("committer Not Committed Yet");
+        outConsumer.consumeLine("committer-mail <not.committed.yet>");
+        outConsumer.consumeLine("committer-time 1312534171");
+        outConsumer.consumeLine("committer-tz +0200");
+        outConsumer.consumeLine("summary Move to nexus.codehaus.org + configuration of maven release plugin is back");
+        outConsumer.consumeLine("previous 1bec1c3a77f6957175be13e4433110f7fc8e387e pom.xml");
+        outConsumer.consumeLine("filename pom.xml");
+        outConsumer.consumeLine("\t<id>codehaus-nexus-staging</id>");
+        outConsumer.consumeLine("2c68c473da7fc293e12ca50f19380c5118be7ead 72 60 1");
+        outConsumer.consumeLine("\t<url>${sonar.snapshotRepository.url}</url>");
+        return 0;
+      }
+    });
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Unable to blame file src/foo.xoo. Is file commited?");
+    new GitBlameCommand(commandExecutor).blame(fs, Arrays.<InputFile>asList(inputFile), result);
+  }
+
+  @Test
   public void testExecutionError() throws IOException {
     File source = new File(baseDir, "src/foo.xoo");
     FileUtils.write(source, "sample content");
diff --git a/plugins/sonar-git-plugin/src/test/java/org/sonar/plugins/scm/git/JGitBlameCommandTest.java b/plugins/sonar-git-plugin/src/test/java/org/sonar/plugins/scm/git/JGitBlameCommandTest.java
index a750ae3ae15..9831bb787ee 100644
--- a/plugins/sonar-git-plugin/src/test/java/org/sonar/plugins/scm/git/JGitBlameCommandTest.java
+++ b/plugins/sonar-git-plugin/src/test/java/org/sonar/plugins/scm/git/JGitBlameCommandTest.java
@@ -24,6 +24,7 @@ import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.DefaultFileSystem;
@@ -48,6 +49,9 @@ import static org.mockito.Mockito.verify;
 public class JGitBlameCommandTest {
 
   @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
+  @Rule
   public TemporaryFolder temp = new TemporaryFolder();
 
   @Test
@@ -99,6 +103,57 @@ public class JGitBlameCommandTest {
 
   }
 
+  @Test
+  public void testBlameOnModifiedFile() throws IOException {
+    File projectDir = temp.newFolder();
+    javaUnzip(new File("test-repos/dummy-git.zip"), projectDir);
+
+    JGitBlameCommand jGitBlameCommand = new JGitBlameCommand();
+
+    DefaultFileSystem fs = new DefaultFileSystem();
+    File baseDir = new File(projectDir, "dummy-git");
+    fs.setBaseDir(baseDir);
+    String relativePath = "src/main/java/org/dummy/Dummy.java";
+    DefaultInputFile inputFile = new DefaultInputFile("foo", relativePath);
+    fs.add(inputFile);
+
+    // Emulate a modification
+    FileUtils.write(new File(baseDir, relativePath), "modification and \n some new line", true);
+
+    BlameResult blameResult = mock(BlameResult.class);
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Unable to blame file " + relativePath + ". No blame info at line 27. Is file commited?");
+    jGitBlameCommand.blame(fs, Arrays.<InputFile>asList(inputFile), blameResult);
+  }
+
+  @Test
+  public void testBlameOnNewFile() throws IOException {
+    File projectDir = temp.newFolder();
+    javaUnzip(new File("test-repos/dummy-git.zip"), projectDir);
+
+    JGitBlameCommand jGitBlameCommand = new JGitBlameCommand();
+
+    DefaultFileSystem fs = new DefaultFileSystem();
+    File baseDir = new File(projectDir, "dummy-git");
+    fs.setBaseDir(baseDir);
+    String relativePath = "src/main/java/org/dummy/Dummy.java";
+    String relativePath2 = "src/main/java/org/dummy/Dummy2.java";
+    DefaultInputFile inputFile = new DefaultInputFile("foo", relativePath);
+    fs.add(inputFile);
+    DefaultInputFile inputFile2 = new DefaultInputFile("foo", relativePath2);
+    fs.add(inputFile2);
+
+    // Emulate a new file
+    FileUtils.copyFile(new File(baseDir, relativePath), new File(baseDir, relativePath2));
+
+    BlameResult blameResult = mock(BlameResult.class);
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Unable to blame file " + relativePath2 + ". No blame info at line 1. Is file commited?");
+    jGitBlameCommand.blame(fs, Arrays.<InputFile>asList(inputFile, inputFile2), blameResult);
+  }
+
   private static void javaUnzip(File zip, File toDir) {
     try {
       ZipFile zipFile = new ZipFile(zip);