Merge pull request #21 from ciaranmooney/pmd-path-mismatch

Path mis-match when git-dir with relative path elements provided

Author: yangziwen

diff-core/src/main/java/io/github/yangziwen/diff/calculate/DiffEntryWrapper.java

@@ -1,6 +1,7 @@
 package io.github.yangziwen.diff.calculate;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.List;
 
 import org.eclipse.jgit.diff.DiffEntry;
@@ -53,7 +54,14 @@ public String getAbsoluteNewPath() {
     }
 
     public File getNewFile() {
-        return new File(gitDir, diffEntry.getNewPath());
+        return new File(normaliseParent(gitDir), diffEntry.getNewPath());
     }
 
+    private static String normaliseParent(File directory) {
+        try {
+            return directory.getCanonicalPath();
+        } catch (IOException e) {
+            return "";
+        }
+    }
 }

diff-core/src/test/java/io/github/yangziwen/diff/calculate/DiffCalculatorTest.java

@@ -77,7 +77,7 @@ public void testCalculateDiff() throws Exception {
             Assert.assertEquals(1, wrappers.size());
 
             DiffEntryWrapper wrapper = wrappers.get(0);
-            Assert.assertEquals(fileToChange, wrapper.getNewFile());
+            Assert.assertEquals(fileToChange.getAbsolutePath(), wrapper.getNewFile().getAbsolutePath());
 
             List<Edit> edits = wrapper.getEditList();
 
@@ -138,7 +138,7 @@ public void testDoCalculateCommitDiffWhenFileChanged() throws Exception {
             Assert.assertEquals(1, wrappers.size());
 
             DiffEntryWrapper wrapper = wrappers.get(0);
-            Assert.assertEquals(fileToChange, wrapper.getNewFile());
+            Assert.assertEquals(fileToChange.getAbsolutePath(), wrapper.getNewFile().getAbsolutePath());
 
             List<Edit> edits = wrapper.getEditList();
 
@@ -238,7 +238,7 @@ public void testDoCalculateIndexedDiffWhenFileChanged() throws Exception {
             Assert.assertEquals(1, wrappers.size());
 
             DiffEntryWrapper wrapper = wrappers.get(0);
-            Assert.assertEquals(fileToChange, wrapper.getNewFile());
+            Assert.assertEquals(fileToChange.getAbsolutePath(), wrapper.getNewFile().getAbsolutePath());
 
             List<Edit> edits = wrapper.getEditList();
 
@@ -281,7 +281,7 @@ public void testDoCalculateIndexedDiffWhenFileAdded() throws Exception {
             Assert.assertEquals(1, wrappers.size());
 
             DiffEntryWrapper wrapper = wrappers.get(0);
-            Assert.assertEquals(fileToAdd, wrapper.getNewFile());
+            Assert.assertEquals(fileToAdd.getAbsolutePath(), wrapper.getAbsoluteNewPath());
 
             List<Edit> edits = wrapper.getEditList();
 

diff-core/src/test/java/io/github/yangziwen/diff/calculate/DiffEntryWrapperTest.java

@@ -1,21 +1,16 @@
 package io.github.yangziwen.diff.calculate;
 
+import java.io.File;
+import java.io.IOException;
 import java.util.Arrays;
 
 import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.diff.DiffEntry.ChangeType;
 import org.eclipse.jgit.diff.Edit;
-import org.eclipse.jgit.diff.Edit.Type;
 import org.junit.Assert;
 import org.junit.Test;
-import org.junit.runner.RunWith;
 import org.mockito.Mockito;
-import org.powermock.api.mockito.PowerMockito;
-import org.powermock.core.classloader.annotations.PrepareForTest;
-import org.powermock.modules.junit4.PowerMockRunner;
 
-@RunWith(PowerMockRunner.class)
-@PrepareForTest({ Edit.class })
 public class DiffEntryWrapperTest {
 
     @Test
@@ -30,14 +25,64 @@ public void testIsDeleted() {
 
     @Test
     public void testIsAllDeletedEdits() {
-        Edit edit1 = PowerMockito.mock(Edit.class);
-        Edit edit2 = PowerMockito.mock(Edit.class);
-        PowerMockito.when(edit1.getType()).thenReturn(Type.DELETE);
-        PowerMockito.when(edit2.getType()).thenReturn(Type.DELETE);
         DiffEntryWrapper wrapper = DiffEntryWrapper.builder()
-                .editList(Arrays.asList(edit1, edit2))
+                .editList(Arrays.asList(buildDelete(), buildDelete()))
                 .build();
         Assert.assertTrue(wrapper.isAllDeletedEdits());
     }
 
+    @Test
+    public void getAbsoluteNewPath() throws IOException {
+        String file = "file";
+        String directory = "/some/directory";
+        DiffEntryWrapper wrapper = DiffEntryWrapper.builder()
+                .gitDir(new File(directory))
+                .diffEntry(new DummyDiffEntry(file))
+                .build();
+        String result = wrapper.getAbsoluteNewPath();
+        Assert.assertEquals(new File(directory, file).getCanonicalPath(), result);
+    }
+
+    @Test
+    public void getAbsoluteNewPathWithRelativeGitDir() throws IOException {
+        String file = "file";
+        DiffEntryWrapper wrapper = DiffEntryWrapper.builder()
+                .gitDir(new File("."))
+                .diffEntry(new DummyDiffEntry(file))
+                .build();
+        String result = wrapper.getAbsoluteNewPath();
+        Assert.assertEquals(new File(".", file).getCanonicalPath(), result);
+    }
+
+    @Test
+    public void getAbsoluteNewPathException() throws IOException {
+        File mock = Mockito.mock(File.class);
+        Mockito.when(mock.getCanonicalPath()).thenThrow(new IOException(""));
+        DiffEntryWrapper wrapper = DiffEntryWrapper.builder()
+                .gitDir(mock)
+                .diffEntry(new DummyDiffEntry(""))
+                .build();
+        String result = wrapper.getAbsoluteNewPath();
+        Assert.assertEquals("/", result);
+    }
+
+    // A "delete" edit is one where: beginA < endA && beginB == endB
+    private static Edit buildDelete() {
+        return new Edit(5, 10, 8, 8);
+    }
+
+    private class DummyDiffEntry extends DiffEntry {
+
+        private final String filePath;
+
+        DummyDiffEntry(String filePath) {
+            super();
+            this.filePath = filePath;
+        }
+
+        @Override
+        public String getNewPath() {
+            return filePath;
+        }
+    }
 }