From 23657cf2c16651b4f02f4c9a2490beea0413d277 Mon Sep 17 00:00:00 2001 From: Luolc Date: Wed, 16 Aug 2017 19:12:41 +0800 Subject: [PATCH] Issue #77: git: add line changes detection --- config/import-control.xml | 1 + .../checkstyle/regression/data/GitChange.java | 16 +++++ .../checkstyle/regression/git/DiffParser.java | 35 ++++++++-- .../regression/git/DiffParserTest.java | 68 +++++++++++++++++++ 4 files changed, 116 insertions(+), 4 deletions(-) diff --git a/config/import-control.xml b/config/import-control.xml index 69dde38..b2e1110 100644 --- a/config/import-control.xml +++ b/config/import-control.xml @@ -49,5 +49,6 @@ + diff --git a/src/main/java/com/github/checkstyle/regression/data/GitChange.java b/src/main/java/com/github/checkstyle/regression/data/GitChange.java index ac337ea..b756667 100644 --- a/src/main/java/com/github/checkstyle/regression/data/GitChange.java +++ b/src/main/java/com/github/checkstyle/regression/data/GitChange.java @@ -19,6 +19,8 @@ package com.github.checkstyle.regression.data; +import java.util.List; + import org.immutables.value.Value; /** @@ -32,4 +34,18 @@ public interface GitChange { * @return the path of the changed file */ String path(); + + /** + * The line numbers of the added changes. + * The first line of a file is marked as line zero. + * @return the line numbers of the added codes + */ + List addedLines(); + + /** + * The line numbers of the deleted changes. + * The first line of a file is marked as line zero. + * @return the line numbers of the deleted codes + */ + List deletedLines(); } diff --git a/src/main/java/com/github/checkstyle/regression/git/DiffParser.java b/src/main/java/com/github/checkstyle/regression/git/DiffParser.java index 449ace0..0955187 100644 --- a/src/main/java/com/github/checkstyle/regression/git/DiffParser.java +++ b/src/main/java/com/github/checkstyle/regression/git/DiffParser.java @@ -21,12 +21,16 @@ import java.io.File; import java.io.IOException; +import java.util.Arrays; +import java.util.LinkedList; import java.util.List; import java.util.stream.Collectors; +import org.apache.commons.lang.math.IntRange; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.diff.DiffFormatter; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -36,6 +40,7 @@ import org.eclipse.jgit.storage.file.FileRepositoryBuilder; import org.eclipse.jgit.treewalk.AbstractTreeIterator; import org.eclipse.jgit.treewalk.CanonicalTreeParser; +import org.eclipse.jgit.util.io.DisabledOutputStream; import com.github.checkstyle.regression.data.GitChange; import com.github.checkstyle.regression.data.ImmutableGitChange; @@ -44,6 +49,7 @@ * Parses git diff between PR branch and master for the further use. * @author LuoLiangchen */ +// -@cs[ClassDataAbstractionCoupling] We have to import many classes from JGit public final class DiffParser { /** Prevents instantiation. */ private DiffParser() { @@ -59,7 +65,7 @@ private DiffParser() { */ public static List parse(String repositoryPath, String branchName) throws IOException, GitAPIException { - final List returnValue; + final List returnValue = new LinkedList<>(); final File gitDir = new File(repositoryPath, ".git"); final Repository repository = new FileRepositoryBuilder().setGitDir(gitDir) .readEnvironment().findGitDir().build(); @@ -67,16 +73,20 @@ public static List parse(String repositoryPath, String branchName) try { final TreeParserPair pair = getTreeParserPair(repository, branchName); final Git git = new Git(repository); + final DiffFormatter formatter = new DiffFormatter(DisabledOutputStream.INSTANCE); + formatter.setRepository(repository); try { - returnValue = git.diff() + final List diffs = git.diff() .setOldTree(pair.commonAncestorTreeParser) .setNewTree(pair.prTreeParser) .call() .stream() .filter(entry -> entry.getChangeType() != DiffEntry.ChangeType.DELETE) - .map(DiffParser::convertDiffEntryToGitChange) .collect(Collectors.toList()); + for (DiffEntry diff : diffs) { + returnValue.add(convertDiffEntryToGitChange(diff, formatter)); + } } finally { git.close(); @@ -158,11 +168,28 @@ private static AbstractTreeIterator prepareTreeParser(RevWalk walk, RevCommit co /** * Converts a {@link DiffEntry} to {@link GitChange} for the further use. * @param diffEntry the {@link DiffEntry} instance to be converted + * @param formatter the diff formatter to provide the line changes information * @return the {@link GitChange} instance converted from the given {@link DiffEntry} + * @throws IOException JGit library exception */ - private static GitChange convertDiffEntryToGitChange(DiffEntry diffEntry) { + private static GitChange convertDiffEntryToGitChange( + DiffEntry diffEntry, DiffFormatter formatter) throws IOException { + final List addedLines = formatter.toFileHeader(diffEntry).toEditList().stream() + .filter(edit -> edit.getBeginB() < edit.getEndB()) + .flatMapToInt(edit -> Arrays.stream( + new IntRange(edit.getBeginB(), edit.getEndB() - 1).toArray())) + .boxed() + .collect(Collectors.toList()); + final List deletedLines = formatter.toFileHeader(diffEntry).toEditList().stream() + .filter(edit -> edit.getBeginA() < edit.getEndA()) + .flatMapToInt(edit -> Arrays.stream( + new IntRange(edit.getBeginA(), edit.getEndA() - 1).toArray())) + .boxed() + .collect(Collectors.toList()); return ImmutableGitChange.builder() .path(diffEntry.getNewPath()) + .addAllAddedLines(addedLines) + .addAllDeletedLines(deletedLines) .build(); } diff --git a/src/test/java/com/github/checkstyle/regression/git/DiffParserTest.java b/src/test/java/com/github/checkstyle/regression/git/DiffParserTest.java index 2f4f9bc..b068708 100644 --- a/src/test/java/com/github/checkstyle/regression/git/DiffParserTest.java +++ b/src/test/java/com/github/checkstyle/regression/git/DiffParserTest.java @@ -79,6 +79,7 @@ public void testParseModifyChange() throws Exception { assertEquals("There should be 1 change detected", 1, changes.size()); final GitChange expected = ImmutableGitChange.builder() .path("HelloWorld") + .addAddedLines(0) .build(); assertEquals("The change is not as expected", expected, changes.get(0)); } @@ -203,4 +204,71 @@ public void testParseFilePermissionChange() throws Exception { } } } + + @Test + public void testParseLineChangesAddLine() throws Exception { + try (Repository repository = GitUtils.createNewRepository()) { + final File helloWorld = GitUtils.addAnEmptyFileAndCommit(repository, "HelloWorld"); + Files.write(helloWorld.toPath(), "line 0\n" + .getBytes(Charset.forName("UTF-8")), StandardOpenOption.APPEND); + GitUtils.addAllAndCommit(repository, "add original line"); + GitUtils.createNewBranchAndCheckout(repository, "foo"); + Files.write(helloWorld.toPath(), "line 1 added\nline 2 added\n" + .getBytes(Charset.forName("UTF-8")), StandardOpenOption.APPEND); + GitUtils.addAllAndCommit(repository, "add line 1, 2"); + final List changes = DiffParser.parse( + repository.getDirectory().getParent(), "foo"); + assertEquals("There should be 1 change detected", 1, changes.size()); + final GitChange expected = ImmutableGitChange.builder() + .path("HelloWorld") + .addAddedLines(1, 2) + .build(); + assertEquals("The change is not as expected", expected, changes.get(0)); + } + } + + @Test + public void testParseLineChangesRemoveLine() throws Exception { + try (Repository repository = GitUtils.createNewRepository()) { + final File helloWorld = GitUtils.addAnEmptyFileAndCommit(repository, "HelloWorld"); + Files.write(helloWorld.toPath(), "line 0\nline 1 to be removed\nline 2 to be removed\n" + .getBytes(Charset.forName("UTF-8")), StandardOpenOption.APPEND); + GitUtils.addAllAndCommit(repository, "add original three lines"); + GitUtils.createNewBranchAndCheckout(repository, "foo"); + Files.write(helloWorld.toPath(), "line 0\n" + .getBytes(Charset.forName("UTF-8")), StandardOpenOption.TRUNCATE_EXISTING); + GitUtils.addAllAndCommit(repository, "remove two lines"); + final List changes = DiffParser.parse( + repository.getDirectory().getParent(), "foo"); + assertEquals("There should be 1 change detected", 1, changes.size()); + final GitChange expected = ImmutableGitChange.builder() + .path("HelloWorld") + .addDeletedLines(1, 2) + .build(); + assertEquals("The change is not as expected", expected, changes.get(0)); + } + } + + @Test + public void testParseLineChangesModifyLine() throws Exception { + try (Repository repository = GitUtils.createNewRepository()) { + final File helloWorld = GitUtils.addAnEmptyFileAndCommit(repository, "HelloWorld"); + Files.write(helloWorld.toPath(), "line 0\nline 1\nline 2\n" + .getBytes(Charset.forName("UTF-8")), StandardOpenOption.APPEND); + GitUtils.addAllAndCommit(repository, "add original three lines"); + GitUtils.createNewBranchAndCheckout(repository, "foo"); + Files.write(helloWorld.toPath(), "line 0\nline 1 changed\nline 2 changed\n" + .getBytes(Charset.forName("UTF-8")), StandardOpenOption.TRUNCATE_EXISTING); + GitUtils.addAllAndCommit(repository, "modify two lines"); + final List changes = DiffParser.parse( + repository.getDirectory().getParent(), "foo"); + assertEquals("There should be 1 change detected", 1, changes.size()); + final GitChange expected = ImmutableGitChange.builder() + .path("HelloWorld") + .addAddedLines(1, 2) + .addDeletedLines(1, 2) + .build(); + assertEquals("The change is not as expected", expected, changes.get(0)); + } + } }