Skip to content

Commit 12f8cbe

Browse files
author
Vincent Potucek
committed
[fix] NPE due to workingTreeIterator being null for git ignored files. #911
- #911 Signed-off-by: Vincent Potucek <[email protected]>
1 parent bf74d6f commit 12f8cbe

File tree

2 files changed

+225
-7
lines changed

2 files changed

+225
-7
lines changed

‎lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public boolean isClean(Project project, ObjectId treeSha, String relativePathUni
9696
DirCacheIterator dirCacheIterator = treeWalk.getTree(INDEX, DirCacheIterator.class);
9797
WorkingTreeIterator workingTreeIterator = treeWalk.getTree(WORKDIR, WorkingTreeIterator.class);
9898

99-
boolean hasTree = treeIterator != null;
99+
boolean hasTree = treeIterator != null && workingTreeIterator != null;
100100
boolean hasDirCache = dirCacheIterator != null;
101101

102102
if (!hasTree) {

‎lib-extra/src/test/java/com/diffplug/spotless/extra/GitRachetMergeBaseTest.java‎

Lines changed: 224 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*/
1616
package com.diffplug.spotless.extra;
1717

18+
import static org.junit.jupiter.api.Assertions.assertThrows;
19+
1820
import java.io.File;
1921
import java.io.IOException;
2022
import java.util.Arrays;
@@ -57,6 +59,208 @@ void test() throws IllegalStateException, GitAPIException, IOException {
5759
}
5860
}
5961

62+
@Test // https://github.com/diffplug/spotless/issues/911
63+
void testGitIgnoredDirectory_ShouldNotThrowNPE() throws IllegalStateException, GitAPIException, IOException {
64+
try (Git git = initRepo()) {
65+
// Create a directory with files and commit them
66+
setFile("useless/Wow.java").toContent("class Wow {}");
67+
setFile("useless/Another.java").toContent("class Another {}");
68+
addAndCommit(git, "Add useless package");
69+
70+
// Now ignore the entire directory and commit
71+
setFile(".gitignore").toContent("useless/");
72+
addAndCommit(git, "Ignore useless directory");
73+
74+
// The files in the useless directory are now committed but gitignored
75+
// This should not throw NPE
76+
ratchetFrom("main").onlyDirty("useless/Wow.java", "useless/Another.java");
77+
}
78+
}
79+
80+
@Test
81+
void testNewUntrackedFile_ShouldBeDirty() throws IllegalStateException, GitAPIException, IOException {
82+
try (Git git = initRepo()) {
83+
// Create and commit initial file
84+
setFile("committed.java").toContent("class Committed {}");
85+
addAndCommit(git, "Initial commit");
86+
87+
// Create a new file that's not tracked at all (not in gitignore either)
88+
setFile("new_untracked.java").toContent("class NewUntracked {}");
89+
90+
// The new untracked file should be considered dirty
91+
ratchetFrom("main").onlyDirty("new_untracked.java");
92+
}
93+
}
94+
95+
@Test
96+
void testModifiedTrackedFile_ShouldBeDirty() throws IllegalStateException, GitAPIException, IOException {
97+
try (Git git = initRepo()) {
98+
// Create and commit initial file
99+
setFile("Main.java").toContent("class Main { void old() {} }");
100+
addAndCommit(git, "Initial commit");
101+
102+
// Modify the file
103+
setFile("Main.java").toContent("class Main { void newMethod() {} }");
104+
105+
// The modified file should be considered dirty
106+
ratchetFrom("main").onlyDirty("Main.java");
107+
}
108+
}
109+
110+
@Test
111+
void testDeletedFile_ShouldBeDirty() throws IllegalStateException, GitAPIException, IOException {
112+
try (Git git = initRepo()) {
113+
// Create and commit multiple files
114+
setFile("keep.java").toContent("class Keep {}");
115+
setFile("delete.java").toContent("class Delete {}");
116+
addAndCommit(git, "Initial commit");
117+
118+
// Delete one file
119+
new File(rootFolder(), "delete.java").delete();
120+
121+
// The deleted file should be considered dirty
122+
ratchetFrom("main").onlyDirty("delete.java");
123+
}
124+
}
125+
126+
@Test
127+
void testRenamedFile_ShouldBeDirty() throws IllegalStateException, GitAPIException, IOException {
128+
try (Git git = initRepo()) {
129+
// Create and commit initial file
130+
setFile("OldName.java").toContent("class OldName {}");
131+
addAndCommit(git, "Initial commit");
132+
133+
// Rename the file (Git sees this as delete + add)
134+
File oldFile = new File(rootFolder(), "OldName.java");
135+
File newFile = new File(rootFolder(), "NewName.java");
136+
oldFile.renameTo(newFile);
137+
138+
// Both old and new files should be considered dirty
139+
ratchetFrom("main").onlyDirty("OldName.java", "NewName.java");
140+
}
141+
}
142+
143+
@Test
144+
void testStagedButUncommittedChanges_ShouldBeDirty() throws IllegalStateException, GitAPIException, IOException {
145+
try (Git git = initRepo()) {
146+
// Create and commit initial file
147+
setFile("Test.java").toContent("class Test {}");
148+
addAndCommit(git, "Initial commit");
149+
150+
// Modify and stage the file but don't commit
151+
setFile("Test.java").toContent("class Test { void newMethod() {} }");
152+
git.add().addFilepattern("Test.java").call();
153+
154+
// The staged but uncommitted file should be considered dirty
155+
ratchetFrom("main").onlyDirty("Test.java");
156+
}
157+
}
158+
159+
@Test
160+
void testMultipleBranchesWithDifferentFiles() throws IllegalStateException, GitAPIException, IOException {
161+
try (Git git = initRepo()) {
162+
// Initial commit
163+
setFile("base.txt").toContent("base");
164+
addAndCommit(git, "Initial commit");
165+
166+
// Branch A changes
167+
git.checkout().setCreateBranch(true).setName("branch-a").call();
168+
setFile("a-only.txt").toContent("a content");
169+
addAndCommit(git, "Branch A commit");
170+
171+
// Branch B changes
172+
git.checkout().setName("main").call();
173+
git.checkout().setCreateBranch(true).setName("branch-b").call();
174+
setFile("b-only.txt").toContent("b content");
175+
addAndCommit(git, "Branch B commit");
176+
177+
// Check from both branches - each should only see their own changes as dirty
178+
git.checkout().setName("main").call();
179+
ratchetFrom("branch-a").onlyDirty("a-only.txt");
180+
ratchetFrom("branch-b").onlyDirty("b-only.txt");
181+
}
182+
}
183+
184+
@Test
185+
void testNestedDirectoryStructure() throws IllegalStateException, GitAPIException, IOException {
186+
try (Git git = initRepo()) {
187+
// Create nested directory structure
188+
setFile("src/main/java/com/example/Main.java").toContent("package com.example; class Main {}");
189+
setFile("src/main/java/com/example/Util.java").toContent("package com.example; class Util {}");
190+
setFile("src/test/java/com/example/MainTest.java").toContent("package com.example; class MainTest {}");
191+
addAndCommit(git, "Add nested structure");
192+
193+
// Modify only one nested file
194+
setFile("src/main/java/com/example/Util.java").toContent("package com.example; class Util { void newMethod() {} }");
195+
196+
// Only the modified nested file should be dirty
197+
ratchetFrom("main").onlyDirty("src/main/java/com/example/Util.java");
198+
}
199+
}
200+
201+
@Test
202+
void testNonExistentReference_ShouldThrowException() throws IllegalStateException, GitAPIException, IOException {
203+
try (Git git = initRepo()) {
204+
setFile("test.txt").toContent("test");
205+
addAndCommit(git, "Initial commit");
206+
207+
// Trying to ratchet from non-existent branch should throw
208+
assertThrows(IllegalArgumentException.class, () -> {
209+
ratchetFrom("nonexistent-branch");
210+
});
211+
}
212+
}
213+
214+
@Test
215+
void testBinaryFile_ShouldBeHandled() throws IllegalStateException, GitAPIException, IOException {
216+
try (Git git = initRepo()) {
217+
// Create and commit binary file
218+
setFile("image.png").toContent("binary content that looks like an image");
219+
addAndCommit(git, "Add binary file");
220+
221+
// Modify binary content
222+
setFile("image.png").toContent("modified binary content");
223+
224+
// Binary file should be detected as dirty
225+
ratchetFrom("main").onlyDirty("image.png");
226+
}
227+
}
228+
229+
@Test
230+
void testSymlink_ShouldBeHandled() throws IllegalStateException, GitAPIException, IOException {
231+
try (Git git = initRepo()) {
232+
// This test would require creating actual symlinks
233+
// For now, we'll test that the code doesn't break with special files
234+
setFile("regular.txt").toContent("regular file");
235+
setFile("special.txt").toContent("special file");
236+
addAndCommit(git, "Add files");
237+
238+
// Modify one file
239+
setFile("special.txt").toContent("modified special file");
240+
241+
// Should correctly identify the modified file
242+
ratchetFrom("main").onlyDirty("special.txt");
243+
}
244+
}
245+
246+
@Test
247+
void testMultipleProjectsInSameRepo() throws IllegalStateException, GitAPIException, IOException {
248+
try (Git git = initRepo()) {
249+
// Simulate multiple projects in same repo
250+
setFile("project1/src/Main.java").toContent("class Main {}");
251+
setFile("project2/src/Other.java").toContent("class Other {}");
252+
setFile("shared/common.txt").toContent("shared");
253+
addAndCommit(git, "Add projects");
254+
255+
// Modify files in different "projects"
256+
setFile("project1/src/Main.java").toContent("class Main { void change() {} }");
257+
setFile("shared/common.txt").toContent("modified shared");
258+
259+
// Should detect all modified files
260+
ratchetFrom("main").onlyDirty("project1/src/Main.java", "shared/common.txt");
261+
}
262+
}
263+
60264
static class GitRatchetSimple extends GitRatchet<File> {
61265
@Override
62266
protected File getDir(File project) {
@@ -86,9 +290,15 @@ class Asserter {
86290
}
87291

88292
private void assertClean(int i, String filename, boolean expected) throws IOException {
89-
boolean actual = ratchet.isClean(rootFolder(), shas[i], newFile(filename));
293+
File file = new File(rootFolder(), filename);
294+
if (!file.exists()) {
295+
throw new AssertionError("File " + filename + " does not exist");
296+
}
297+
298+
boolean actual = ratchet.isClean(rootFolder(), shas[i], file);
90299
if (actual != expected) {
91-
throw new AssertionError("Expected " + filename + " to be " + (expected ? "clean" : "dirty") + " relative to " + ratchetFroms[i]);
300+
throw new AssertionError("Expected " + filename + " to be " + (expected ? "clean" : "dirty") +
301+
" relative to " + ratchetFroms[i] + " but was " + (actual ? "clean" : "dirty"));
92302
}
93303
}
94304

@@ -106,16 +316,23 @@ public void allDirty() throws IOException {
106316

107317
public void onlyDirty(String... filenames) throws IOException {
108318
List<String> dirtyFiles = Arrays.asList(filenames);
109-
for (File file : rootFolder().listFiles()) {
110-
if (!file.isFile()) {
319+
for (File file : getAllFilesRecursive(rootFolder())) {
320+
if (file.isDirectory()) {
111321
continue;
112322
}
113-
boolean expectedClean = !dirtyFiles.contains(file.getName());
323+
String relativePath = rootFolder().toPath().relativize(file.toPath()).toString();
324+
boolean expectedClean = !dirtyFiles.contains(relativePath);
114325
for (int i = 0; i < shas.length; i++) {
115-
assertClean(i, file.getName(), expectedClean);
326+
assertClean(i, relativePath, expectedClean);
116327
}
117328
}
118329
}
330+
331+
private List<File> getAllFilesRecursive(File dir) {
332+
return Arrays.stream(dir.listFiles())
333+
.flatMap(file -> file.isDirectory() ? getAllFilesRecursive(file).stream() : Arrays.stream(new File[]{file}))
334+
.toList();
335+
}
119336
}
120337

121338
private Git initRepo() throws IllegalStateException, GitAPIException, IOException {
@@ -131,4 +348,5 @@ private void addAndCommit(Git git, String message) throws GitAPIException {
131348
git.add().addFilepattern(".").call();
132349
git.commit().setMessage(message).call();
133350
}
351+
134352
}

0 commit comments

Comments
 (0)