From: Duarte Meneses Date: Thu, 18 Jan 2018 11:16:20 +0000 (+0100) Subject: SONAR-10257 Added Myers diff detection X-Git-Tag: 7.5~1723 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=c4a26a8e449f832a26fbe2428702a9060da24501;p=sonarqube.git SONAR-10257 Added Myers diff detection --- diff --git a/server/sonar-server/pom.xml b/server/sonar-server/pom.xml index 416becba405..c9b9bd499fb 100644 --- a/server/sonar-server/pom.xml +++ b/server/sonar-server/pom.xml @@ -1,6 +1,6 @@ - 4.0.0 @@ -143,6 +143,11 @@ commons-lang commons-lang + + com.googlecode.java-diff-utils + diffutils + 1.2 + org.picocontainer picocontainer diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/GeneratedScmInfo.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/GeneratedScmInfo.java index 408895252a4..87b45649d0d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/GeneratedScmInfo.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/GeneratedScmInfo.java @@ -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 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 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 dbChangesets = dbScmInfo.getAllChangesets(); + Map 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); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImpl.java index 4d592e8a0eb..5dfdf4b7a7b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImpl.java @@ -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 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 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)); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiff.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiff.java index b6f555b3353..4490afd1a4f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiff.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiff.java @@ -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 getNewOrChangedLines(Component component); + int[] getMatchingLines(Component component); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinder.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinder.java index 2255b74823a..e60fa0b62fe 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinder.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinder.java @@ -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 findNewOrChangedLines() { - return walk(0, 0, new HashSet<>()); - } - - private Set walk(int r, int db, HashSet 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 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; } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImpl.java index 3404a2668ea..904072747df 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImpl.java @@ -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 getNewOrChangedLines(Component component) { + public int[] getMatchingLines(Component component) { - List database = new ArrayList<>(); + List 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 report = new ArrayList<>(); + List report; SourceLinesHashesComputer linesHashesComputer = new SourceLinesHashesComputer(); try (CloseableIterator 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(); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImplTest.java index f84703c3044..aa6522f5a31 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImplTest.java @@ -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); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinderTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinderTest.java index 7aa1288d0f0..06c110aeb03 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinderTest.java @@ -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 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 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 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 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 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 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 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 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 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 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 diff = new SourceLinesDiffFinder(database, report).findNewOrChangedLines(); + int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines(); - assertThat(diff).isEmpty(); + assertThat(diff).containsExactly(3, 4); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImplTest.java index 25653123901..1bd89561ef5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffImplTest.java @@ -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); } diff --git a/tests/src/test/java/org/sonarqube/tests/source/NoScmTest.java b/tests/src/test/java/org/sonarqube/tests/source/NoScmTest.java index 28ddb4b046e..de23ed8a23e 100644 --- a/tests/src/test/java/org/sonarqube/tests/source/NoScmTest.java +++ b/tests/src/test/java/org/sonarqube/tests/source/NoScmTest.java @@ -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 {