]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-10257 Added Myers diff detection
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Thu, 18 Jan 2018 11:16:20 +0000 (12:16 +0100)
committerDuarte Meneses <duarte.meneses@sonarsource.com>
Wed, 7 Feb 2018 13:33:55 +0000 (14:33 +0100)
server/sonar-server/pom.xml
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/GeneratedScmInfo.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImpl.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiff.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinder.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImpl.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImplTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinderTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImplTest.java
tests/src/test/java/org/sonarqube/tests/source/NoScmTest.java

index 416becba405fa40a19f7c2f9abf260c5b15f1913..c9b9bd499fb218ef963b37b6b616f6933cce03d0 100644 (file)
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0"
-         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+<project xmlns="http://maven.apache.org/POM/4.0.0" 
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
   <modelVersion>4.0.0</modelVersion>
   <parent>
       <groupId>commons-lang</groupId>
       <artifactId>commons-lang</artifactId>
     </dependency>
+    <dependency>
+      <groupId>com.googlecode.java-diff-utils</groupId>
+      <artifactId>diffutils</artifactId>
+      <version>1.2</version>
+    </dependency>
     <dependency>
       <groupId>org.picocontainer</groupId>
       <artifactId>picocontainer</artifactId>
index 408895252a4636f095c940d09f60bb24b46f0c3f..87b45649d0d304442209d91312475bf929abad0d 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.computation.task.projectanalysis.scm;
 
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -43,19 +44,21 @@ public class GeneratedScmInfo implements ScmInfo {
     return new GeneratedScmInfo(changesets);
   }
 
-  public static ScmInfo create(long analysisDate, Set<Integer> lines, ScmInfo dbScmInfo) {
-    checkState(!lines.isEmpty(), "No changesets");
-
+  public static ScmInfo create(long analysisDate, int[] matches, ScmInfo dbScmInfo) {
     Changeset changeset = Changeset.newChangesetBuilder()
       .setDate(analysisDate)
       .build();
-    Map<Integer, Changeset> changesets = lines.stream()
-      .collect(Collectors.toMap(x -> x, i -> changeset));
 
-    dbScmInfo.getAllChangesets().entrySet().stream()
-      .filter(e -> !lines.contains(e.getKey()))
-      .forEach(e -> changesets.put(e.getKey(), e.getValue()));
+    Map<Integer, Changeset> dbChangesets = dbScmInfo.getAllChangesets();
+    Map<Integer, Changeset> changesets = new LinkedHashMap<>(matches.length);
 
+    for (int i = 0; i < matches.length; i++) {
+      if (matches[i] > 0) {
+        changesets.put(i + 1, dbChangesets.get(matches[i]));
+      } else {
+        changesets.put(i + 1, changeset);
+      }
+    }
     return new GeneratedScmInfo(changesets);
   }
 
index 4d592e8a0ebe7f2fadc2483bf0c9d4081e101429..5dfdf4b7a7ba6832d967e5c87006cc8364401859 100644 (file)
@@ -96,7 +96,7 @@ public class ScmInfoRepositoryImpl implements ScmInfoRepository {
     return Optional.of(GeneratedScmInfo.create(analysisMetadata.getAnalysisDate(), newOrChangedLines));
   }
 
-  private ScmInfo removeAuthorAndRevision(ScmInfo info) {
+  private static ScmInfo removeAuthorAndRevision(ScmInfo info) {
     Map<Integer, Changeset> cleanedScmInfo = info.getAllChangesets().entrySet().stream()
       .collect(Collectors.toMap(Map.Entry::getKey, e -> removeAuthorAndRevision(e.getValue())));
     return new ScmInfoImpl(cleanedScmInfo);
@@ -120,11 +120,9 @@ public class ScmInfoRepositoryImpl implements ScmInfoRepository {
     }
 
     // generate date for new/changed lines
-    Set<Integer> newOrChangedLines = sourceLinesDiff.getNewOrChangedLines(file);
-    if (newOrChangedLines.isEmpty()) {
-      return Optional.of(scmInfo);
-    }
-    return Optional.of(GeneratedScmInfo.create(analysisMetadata.getAnalysisDate(), newOrChangedLines, scmInfo));
+    int[] matchingLines = sourceLinesDiff.getMatchingLines(file);
+
+    return Optional.of(GeneratedScmInfo.create(analysisMetadata.getAnalysisDate(), matchingLines, scmInfo));
   }
 
 }
index b6f555b3353621cacc28df9f18c74cc6d11b9e12..4490afd1a4fc148de36b806a1228e463495e5d8f 100644 (file)
@@ -19,9 +19,8 @@
  */
 package org.sonar.server.computation.task.projectanalysis.source;
 
-import java.util.Set;
 import org.sonar.server.computation.task.projectanalysis.component.Component;
 
 public interface SourceLinesDiff {
-  Set<Integer> getNewOrChangedLines(Component component);
+  int[] getMatchingLines(Component component);
 }
index 2255b74823ac00eed07060eacbb08ea301561226..e60fa0b62fedbcc025b3470b572405c4f80c69b5 100644 (file)
@@ -19,9 +19,9 @@
  */
 package org.sonar.server.computation.task.projectanalysis.source;
 
-import java.util.HashSet;
+import difflib.myers.MyersDiff;
+import difflib.myers.PathNode;
 import java.util.List;
-import java.util.Set;
 
 public class SourceLinesDiffFinder {
 
@@ -33,39 +33,42 @@ public class SourceLinesDiffFinder {
     this.report = report;
   }
 
-  public Set<Integer> findNewOrChangedLines() {
-    return walk(0, 0, new HashSet<>());
-  }
-
-  private Set<Integer> walk(int r, int db, HashSet<Integer> acc) {
+  /**
+   * Creates a diff between the file in the database and the file in the report using Myers' algorithm, and links matching lines between
+   * both files.
+   * @return an array with one entry for each line in the report. Those entries point either to a line in the database, or to 0, 
+   * in which case it means the line was added.
+   */
+  public int[] findMatchingLines() {
+    int[] index = new int[report.size()];
 
-    if (r >= report.size()) {
-      return acc;
-    }
+    int dbLine = database.size();
+    int reportLine = report.size();
+    try {
+      PathNode node = MyersDiff.buildPath(database.toArray(), report.toArray());
 
-    if (db < database.size()) {
-
-      if (report.get(r).equals(database.get(db))) {
-        walk(stepIndex(r), stepIndex(db), acc);
-        return acc;
-      }
+      while (node.prev != null) {
+        PathNode prevNode = node.prev;
 
-      List<String> remainingDatabase = database.subList(db, database.size());
-      if (remainingDatabase.contains(report.get(r))) {
-        int nextDb = db + remainingDatabase.indexOf(report.get(r));
-        walk(r, nextDb, acc);
-        return acc;
+        if (!node.isSnake()) {
+          // additions
+          reportLine -= (node.j - prevNode.j);
+          // removals
+          dbLine -= (node.i - prevNode.i);
+        } else {
+          // matches
+          for (int i = node.i; i > prevNode.i; i--) {
+            index[reportLine - 1] = dbLine;
+            reportLine--;
+            dbLine--;
+          }
+        }
+        node = prevNode;
       }
-
+    } catch (Exception e) {
+      return index;
     }
-
-    acc.add(r+1);
-    walk(stepIndex(r), db, acc);
-    return acc;
-  }
-
-  private static int stepIndex(int r) {
-    return ++r;
+    return index;
   }
 
 }
index 3404a2668eaff4ee542ad0fee50f4bfb93771e7f..904072747df28b8d2818f168f3670fc4bcd28d7a 100644 (file)
@@ -21,7 +21,6 @@ package org.sonar.server.computation.task.projectanalysis.source;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Set;
 import org.sonar.core.hash.SourceLinesHashesComputer;
 import org.sonar.core.util.CloseableIterator;
 import org.sonar.db.DbClient;
@@ -43,14 +42,17 @@ public class SourceLinesDiffImpl implements SourceLinesDiff {
   }
 
   @Override
-  public Set<Integer> getNewOrChangedLines(Component component) {
+  public int[] getMatchingLines(Component component) {
 
-    List<String> database = new ArrayList<>();
+    List<String> database;
     try (DbSession dbSession = dbClient.openSession(false)) {
-      database.addAll(fileSourceDao.selectLineHashes(dbSession, component.getUuid()));
+      database = fileSourceDao.selectLineHashes(dbSession, component.getUuid());
+      if (database == null) {
+        database = new ArrayList<>();
+      }
     }
 
-    List<String> report = new ArrayList<>();
+    List<String> report;
     SourceLinesHashesComputer linesHashesComputer = new SourceLinesHashesComputer();
     try (CloseableIterator<String> lineIterator = sourceLinesRepository.readLines(component)) {
       while (lineIterator.hasNext()) {
@@ -58,9 +60,9 @@ public class SourceLinesDiffImpl implements SourceLinesDiff {
         linesHashesComputer.addLine(line);
       }
     }
-    report.addAll(linesHashesComputer.getLineHashes());
+    report = linesHashesComputer.getLineHashes();
 
-    return new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    return new SourceLinesDiffFinder(database, report).findMatchingLines();
 
   }
 
index f84703c304442affd9b0f778bdc66a4d8b8b1285..aa6522f5a31dfc6f5c376eae3dbb9f3f9b417f88 100644 (file)
@@ -20,7 +20,6 @@
 package org.sonar.server.computation.task.projectanalysis.scm;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.UseDataProvider;
@@ -206,7 +205,7 @@ public class ScmInfoRepositoryImplTest {
   @Test
   public void generate_scm_info_for_new_and_changed_lines_when_report_is_empty() {
     createDbScmInfoWithOneLine("hash");
-    when(diff.getNewOrChangedLines(FILE)).thenReturn(ImmutableSet.of(2, 3));
+    when(diff.getMatchingLines(FILE)).thenReturn(new int[] {1, 0, 0});
     addFileSourceInReport(3);
     ScmInfo scmInfo = underTest.getScmInfo(FILE).get();
     assertThat(scmInfo.getAllChangesets()).hasSize(3);
@@ -216,7 +215,7 @@ public class ScmInfoRepositoryImplTest {
     assertChangeset(scmInfo.getChangesetForLine(3), null, null, analysisDate.getTime());
 
     verify(dbLoader).getScmInfo(FILE);
-    verify(diff).getNewOrChangedLines(FILE);
+    verify(diff).getMatchingLines(FILE);
     verifyNoMoreInteractions(dbLoader);
     verifyZeroInteractions(sourceHashRepository);
     verifyNoMoreInteractions(diff);
index 7aa1288d0f03f08f38b10beeecd963f16c1411c9..06c110aeb030a738d453ef52c50984c79f70be4b 100644 (file)
@@ -21,7 +21,6 @@ package org.sonar.server.computation.task.projectanalysis.source;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Set;
 import org.junit.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -45,9 +44,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - 3");
     report.add("line - 4");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
-    assertThat(diff).isEmpty();
+    assertThat(diff).containsExactly(1, 2, 3, 4, 5);
 
   }
 
@@ -78,9 +77,8 @@ public class SourceLinesDiffFinderTest {
     report.add("    }\n");
     report.add("}\n");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
-
-    assertThat(diff).containsExactlyInAnyOrder(5, 6, 7, 8, 10, 11, 12);
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
+    assertThat(diff).containsExactly(1, 2, 3, 4, 0, 0, 0, 0, 0, 0, 5, 6, 7);
 
   }
 
@@ -99,9 +97,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - 2");
     report.add("line - 3");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
-    assertThat(diff).containsExactlyInAnyOrder(1, 2);
+    assertThat(diff).containsExactly(0, 0, 3, 4);
 
   }
 
@@ -120,9 +118,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - 2 - modified");
     report.add("line - 3 - modified");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
-    assertThat(diff).containsExactlyInAnyOrder(3, 4);
+    assertThat(diff).containsExactly(1, 2, 0, 0);
 
   }
 
@@ -145,9 +143,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - 4");
     report.add("line - 5");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
-    assertThat(diff).containsExactlyInAnyOrder(3, 4);
+    assertThat(diff).containsExactly(1, 2, 0, 0, 5, 6);
 
   }
 
@@ -166,9 +164,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - 1");
     report.add("line - 2");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
-    assertThat(diff).containsExactlyInAnyOrder(1, 2);
+    assertThat(diff).containsExactly(0, 0, 1, 2, 3);
 
   }
 
@@ -189,9 +187,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - 2");
     report.add("line - 3");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
-    assertThat(diff).containsExactlyInAnyOrder(3, 4);
+    assertThat(diff).containsExactly(1, 2, 0, 0, 3, 4);
 
   }
 
@@ -210,9 +208,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - new");
     report.add("line - new");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
-    assertThat(diff).containsExactlyInAnyOrder(4, 5);
+    assertThat(diff).containsExactly(1, 2, 3, 0, 0);
 
   }
 
@@ -231,9 +229,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - 1");
     report.add("line - 2");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
-    assertThat(diff).isEmpty();
+    assertThat(diff).containsExactly(1, 2, 3);
 
   }
 
@@ -254,10 +252,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - 4");
     report.add("line - 5");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
-
-    assertThat(diff).isEmpty();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
+    assertThat(diff).containsExactly(1, 2, 5, 6);
   }
 
   @Test
@@ -273,9 +270,9 @@ public class SourceLinesDiffFinderTest {
     report.add("line - 2");
     report.add("line - 3");
 
-    Set<Integer> diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines();
+    int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
-    assertThat(diff).isEmpty();
+    assertThat(diff).containsExactly(3, 4);
 
   }
 
index 2565312390111d041cf43dc056c50edafe485674..1bd89561ef5b7e405ff63b3a6b9cd599ac05197f 100644 (file)
@@ -82,7 +82,7 @@ public class SourceLinesDiffImplTest {
     setFileContentInReport(FILE_REF, CONTENT);
 
     Component component = fileComponent(FILE_REF);
-    assertThat(underTest.getNewOrChangedLines(component)).isEmpty();
+    assertThat(underTest.getMatchingLines(component)).containsExactly(1, 2, 3, 4, 5, 6, 7);
 
   }
 
index 28ddb4b046e5bab1eb0901ce24b8284c4fb07ecd..de23ed8a23e3849ffc3f55dce48500329e379e7f 100644 (file)
@@ -113,7 +113,7 @@ public class NoScmTest {
     orchestrator.executeBuild(scanner);
     scmData = ws.getScmData(FILE_TO_ANALYSE);
 
-    assertThat(scmData.size()).isEqualTo(4);
+    assertThat(scmData.size()).isEqualTo(3);
     assertThat(scmData.get(1).revision).isEmpty();
     assertThat(scmData.get(1).author).isEmpty();
     assertThat(scmData.get(1).date).isInSameMinuteWindowAs(new Date());
@@ -121,7 +121,16 @@ public class NoScmTest {
     assertThat(scmData.get(5).revision).isEmpty();
     assertThat(scmData.get(5).author).isEmpty();
     assertThat(scmData.get(5).date).isAfter(scmData.get(1).date);
-
+    
+    assertThat(scmData.get(11).revision).isEmpty();
+    assertThat(scmData.get(11).author).isEmpty();
+    assertThat(scmData.get(11).date).isInSameMinuteWindowAs(new Date());
+
+    tester.openBrowser()
+      .openCode("sample-without-scm", "sample-without-scm:src/main/xoo/sample/Sample.xoo")
+      .getSourceViewer()
+      .shouldHaveNewLines(5, 6, 7, 8, 9, 10)
+      .shouldNotHaveNewLines(1, 2, 3, 4, 11, 12, 13);
   }
 
   private File disposableWorkspaceFor(String project) throws IOException {