]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-13165 Scanner warns that it wasn't able to detected changed lines on files...
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Thu, 5 Mar 2020 17:45:56 +0000 (11:45 -0600)
committersonartech <sonartech@sonarsource.com>
Fri, 6 Mar 2020 20:04:32 +0000 (20:04 +0000)
sonar-plugin-api/src/main/java/org/sonar/api/batch/scm/ScmProvider.java
sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java
sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java

index e7bda351dd351b6aee53514739257101db390e28..be44d13eb4106a380f5e3bad122796549411c5b5 100644 (file)
@@ -68,8 +68,9 @@ public abstract class ScmProvider {
 
   /**
    * Return a map between paths given as argument and the corresponding line numbers which are new compared to the provided target branch.
-   * If null is returned or if a path is not included in the map, an imprecise fallback mechanism will be used to detect which lines
-   * are new (based on SCM dates).
+   * If nothing is returned for a file, the scanner will consider that the provider was unable to determine changes for that file and it will
+   * assume that nothing was changed in it.
+   * If null is returned, an imprecise fallback mechanism will be used to detect which lines are new (based on SCM dates).
    *
    * @param files Absolute path of files of interest
    * @return null if the SCM provider was not able to compute the new lines
index d934999ab9b2d32aefe77553d25844ee8d554a7d..47dd8f402b51f4f01121e5586c8ab816cce2fed1 100644 (file)
@@ -89,18 +89,20 @@ public class ChangedLinesPublisher implements ReportPublisherStep {
 
     for (Map.Entry<Path, DefaultInputFile> e : changedFiles.entrySet()) {
       DefaultInputFile inputFile = e.getValue();
-      Set<Integer> changedLines = pathSetMap.getOrDefault(e.getKey(), Collections.emptySet());
+      Set<Integer> changedLines = pathSetMap.get(e.getKey());
 
-      // detect unchanged last empty line
-      if (changedLines.size() + 1 == inputFile.lines() && inputFile.lineLength(inputFile.lines()) == 0) {
-        changedLines.add(inputFile.lines());
-      }
-
-      if (changedLines.isEmpty()) {
+      if (changedLines == null) {
         LOG.warn("File '{}' was detected as changed but without having changed lines", e.getKey().toAbsolutePath());
+        // assume that no line was changed
+        writeChangedLines(writer, e.getValue().scannerId(), Collections.emptySet());
+      } else {
+        // detect unchanged last empty line
+        if (changedLines.size() + 1 == inputFile.lines() && inputFile.lineLength(inputFile.lines()) == 0) {
+          changedLines.add(inputFile.lines());
+        }
+        count++;
+        writeChangedLines(writer, e.getValue().scannerId(), changedLines);
       }
-      count++;
-      writeChangedLines(writer, e.getValue().scannerId(), changedLines);
     }
     return count;
   }
index ee1f0b37382879322556b4a6e54d7f35e03241dd..bcbd48c843cf9bfe8f8d1c0f91f93e2985e95771 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.scanner.report;
 
+import com.google.common.collect.ImmutableMap;
 import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -34,6 +35,7 @@ import org.sonar.api.batch.fs.internal.DefaultInputFile;
 import org.sonar.api.batch.fs.internal.DefaultInputProject;
 import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
 import org.sonar.api.batch.scm.ScmProvider;
+import org.sonar.api.utils.log.LogTester;
 import org.sonar.scanner.fs.InputModuleHierarchy;
 import org.sonar.scanner.protocol.output.ScannerReportReader;
 import org.sonar.scanner.protocol.output.ScannerReportWriter;
@@ -42,6 +44,7 @@ import org.sonar.scanner.scan.filesystem.InputComponentStore;
 import org.sonar.scanner.scm.ScmConfiguration;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assumptions.assumeThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
@@ -61,6 +64,9 @@ public class ChangedLinesPublisherTest {
   @Rule
   public TemporaryFolder temp = new TemporaryFolder();
 
+  @Rule
+  public LogTester logTester = new LogTester();
+
   private ChangedLinesPublisher publisher = new ChangedLinesPublisher(scmConfiguration, project, inputComponentStore, branchConfiguration);
 
   @Before
@@ -114,16 +120,22 @@ public class ChangedLinesPublisherTest {
   @Test
   public void write_changed_files() {
     DefaultInputFile fileWithChangedLines = createInputFile("path1", "l1\nl2\nl3\n");
-    DefaultInputFile fileWithoutChangedLines = createInputFile("path2", "l1\nl2\nl3\n");
-    Set<Path> paths = new HashSet<>(Arrays.asList(BASE_DIR.resolve("path1"), BASE_DIR.resolve("path2")));
+    DefaultInputFile fileNotReturned = createInputFile("path2", "l1\nl2\nl3\n");
+    DefaultInputFile fileWithoutChangedLines = createInputFile("path3", "l1\nl2\nl3\n");
+
+    Set<Path> paths = new HashSet<>(Arrays.asList(BASE_DIR.resolve("path1"), BASE_DIR.resolve("path2"), BASE_DIR.resolve("path3")));
     Set<Integer> lines = new HashSet<>(Arrays.asList(1, 10));
-    when(provider.branchChangedLines(TARGET_BRANCH, BASE_DIR, paths)).thenReturn(Collections.singletonMap(BASE_DIR.resolve("path1"), lines));
-    when(inputComponentStore.allChangedFilesToPublish()).thenReturn(Arrays.asList(fileWithChangedLines, fileWithoutChangedLines));
+    when(provider.branchChangedLines(TARGET_BRANCH, BASE_DIR, paths))
+      .thenReturn(ImmutableMap.of(BASE_DIR.resolve("path1"), lines, BASE_DIR.resolve("path3"), Collections.emptySet()));
+    when(inputComponentStore.allChangedFilesToPublish()).thenReturn(Arrays.asList(fileWithChangedLines, fileNotReturned, fileWithoutChangedLines));
 
     publisher.publish(writer);
 
     assertPublished(fileWithChangedLines, new HashSet<>(Arrays.asList(1, 10)));
     assertPublished(fileWithoutChangedLines, Collections.emptySet());
+    assertPublished(fileNotReturned, Collections.emptySet());
+
+    assumeThat(logTester.logs()).contains("File '/root/path2' was detected as changed but without having changed lines");
   }
 
   @Test
@@ -170,10 +182,6 @@ public class ChangedLinesPublisherTest {
     assertThat(reader.readComponentChangedLines(file.scannerId()).getLineList()).containsExactlyElementsOf(lines);
   }
 
-  private void assertNotPublished(DefaultInputFile file) {
-    assertThat(new File(temp.getRoot(), "changed-lines-" + file.scannerId() + ".pb")).doesNotExist();
-  }
-
   private void assertNotPublished() {
     assertThat(temp.getRoot().list()).isEmpty();
   }