SONAR-16290 Fix bug parsing certain source code lines and improve logs

4 files changed, 33 insertions, 13 deletions

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/CompositeBlameCommand.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/CompositeBlameCommand.java
index 0bc735dc44b..762e20b65d0 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/CompositeBlameCommand.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/CompositeBlameCommand.java
@@ -41,6 +41,7 @@ import org.sonar.api.notifications.AnalysisWarnings;
 import org.sonar.api.scan.filesystem.PathResolver;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
+import org.sonar.api.utils.log.Profiler;
 
 public class CompositeBlameCommand extends BlameCommand {
   private static final Logger LOG = Loggers.get(CompositeBlameCommand.class);
@@ -66,8 +67,10 @@ public class CompositeBlameCommand extends BlameCommand {
       if (cloneIsInvalid(gitBaseDir)) {
         return;
       }
-
+      Profiler profiler = Profiler.create(LOG);
+      profiler.startDebug("Collecting committed files");
       Set<String> committedFiles = collectAllCommittedFiles(repo);
+      profiler.stopDebug();
       nativeGitEnabled = nativeCmd.isEnabled();
       ExecutorService executorService = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors(), new GitThreadFactory());
 
@@ -113,10 +116,10 @@ public class CompositeBlameCommand extends BlameCommand {
   }
 
   private void blame(BlameOutput output, Git git, File gitBaseDir, InputFile inputFile, String filename) {
-    LOG.debug("Blame file {}", filename);
     List<BlameLine> blame = null;
     if (nativeGitEnabled) {
       try {
+        LOG.debug("Blame file (native) {}", filename);
         blame = nativeCmd.blame(gitBaseDir.toPath(), filename);
       } catch (Exception e) {
         LOG.debug("Native git blame failed. Falling back to jgit: " + filename, e);
@@ -125,6 +128,7 @@ public class CompositeBlameCommand extends BlameCommand {
     }
 
     if (blame == null) {
+      LOG.debug("Blame file (JGit) {}", filename);
       blame = jgitCmd.blame(git, filename);
     }
 
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java
index 1f3dbdaf0d6..06a216d51ab 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.scm.git;
 
-import java.io.BufferedReader;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.nio.file.Path;
@@ -27,6 +26,7 @@ import java.time.Instant;
 import java.util.Date;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Scanner;
 import java.util.function.Consumer;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -92,10 +92,11 @@ public class GitBlameCommand {
     Process p = pb.start();
     try {
       InputStream processStdOutput = p.getInputStream();
-      try (BufferedReader br = new BufferedReader(new InputStreamReader(processStdOutput, UTF_8))) {
-        String outputLine;
-        while ((outputLine = br.readLine()) != null) {
-          stdOutLineConsumer.accept(outputLine);
+      // don't use BufferedReader#readLine because it will also parse CR, which may be part of the actual source code line
+      try (Scanner scanner = new Scanner(new InputStreamReader(processStdOutput, UTF_8))) {
+        scanner.useDelimiter("\n");
+        while (scanner.hasNext()) {
+          stdOutLineConsumer.accept(scanner.next());
         }
       }
 
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitThreadFactory.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitThreadFactory.java
index 5316c86e7b6..179b81d4900 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitThreadFactory.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitThreadFactory.java
@@ -19,9 +19,6 @@
  */
 package org.sonar.scm.git;
 
-import java.util.concurrent.ForkJoinPool;
-import java.util.concurrent.ForkJoinPool.ForkJoinWorkerThreadFactory;
-import java.util.concurrent.ForkJoinWorkerThread;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -33,6 +30,7 @@ public class GitThreadFactory implements ThreadFactory {
   public Thread newThread(Runnable r) {
     Thread t = new Thread(r);
     t.setName(NAME_PREFIX + count.getAndIncrement());
+    t.setDaemon(true);
     return t;
   }
 }
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java
index 4961048e7e2..1ea15883137 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java
@@ -27,7 +27,6 @@ import java.nio.file.Path;
 import java.util.Date;
 import java.util.LinkedList;
 import java.util.List;
-import org.apache.commons.io.FileUtils;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.junit.Before;
@@ -61,6 +60,19 @@ public class GitBlameCommandTest {
   }
 
   @Test
+  public void should_read_lines_only_based_on_new_line() throws Exception {
+    Path baseDir = createNewTempFolder().toPath();
+    String filePath = "file.txt";
+    createFile(filePath, "test1\rtest2\r\ttest3", baseDir);
+    Git git = createRepository(baseDir);
+    createFile(filePath, "line", baseDir);
+    commit(git, filePath);
+
+    List<BlameLine> blame = blameCommand.blame(baseDir, "file.txt");
+    assertThat(blame).hasSize(1);
+  }
+
+  @Test
   public void blame_collects_all_lines() throws Exception {
     File projectDir = createNewTempFolder();
     javaUnzip("dummy-git.zip", projectDir);
@@ -145,7 +157,7 @@ public class GitBlameCommandTest {
     Git git = createRepository(baseDir);
     String filePath = "file.txt";
     createFile(filePath, "line", baseDir);
-    commitWithNoEamil(git, filePath);
+    commitWithNoEmail(git, filePath);
 
     GitBlameCommand blameCommand = new GitBlameCommand();
     List<BlameLine> blame = blameCommand.blame(baseDir, filePath);
@@ -156,11 +168,16 @@ public class GitBlameCommandTest {
     assertThat(blameLine.date()).isNotNull();
   }
 
-  private void commitWithNoEamil(Git git, String path) throws GitAPIException {
+  private void commitWithNoEmail(Git git, String path) throws GitAPIException {
     git.add().addFilepattern(path).call();
     git.commit().setCommitter("joe", "").setMessage("msg").call();
   }
 
+  private void commit(Git git, String path) throws GitAPIException {
+    git.add().addFilepattern(path).call();
+    git.commit().setCommitter("joe", "email@email.com").setMessage("msg").call();
+  }
+
   private File createNewTempFolder() throws IOException {
     //This is needed for Windows, otherwise the created File point to invalid (shortened by Windows) temp folder path
     return temp.newFolder().toPath().toRealPath(LinkOption.NOFOLLOW_LINKS).toFile();