[fix] NPE due to workingTreeIterator being null for git ignored files. #911

- #911

Signed-off-by: Vincent Potucek <[email protected]>

Author: Vincent Potucek

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

@@ -97,7 +97,7 @@ public boolean isClean(Project project, ObjectId treeSha, String relativePathUni
 				WorkingTreeIterator workingTreeIterator = treeWalk.getTree(WORKDIR, WorkingTreeIterator.class);
 
 				boolean hasTree = treeIterator != null;
-				boolean hasDirCache = dirCacheIterator != null;
+				boolean hasDirCache = dirCacheIterator != null && workingTreeIterator != null;
 
 				if (!hasTree) {
 					// it's not in the tree, so it was added

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

@@ -57,6 +57,54 @@ void test() throws IllegalStateException, GitAPIException, IOException {
 		}
 	}
 
+	@Test
+	void testGitIgnoredDirectory_ShouldNotThrowNPE() throws IllegalStateException, GitAPIException, IOException {
+		try (Git git = initRepo()) {
+			// Create a directory with files and commit them
+			setFile("useless/Wow.java").toContent("class Wow {}");
+			setFile("useless/Another.java").toContent("class Another {}");
+			addAndCommit(git, "Add useless package");
+
+			// Now ignore the entire directory and commit
+			setFile(".gitignore").toContent("useless/");
+			addAndCommit(git, "Ignore useless directory");
+
+			// The files in the useless directory are now committed but gitignored
+			// This should not throw NPE
+			ratchetFrom("main").onlyDirty("useless/Wow.java", "useless/Another.java");
+		}
+	}
+
+	@Test
+	void testNewUntrackedFile_ShouldBeDirty() throws IllegalStateException, GitAPIException, IOException {
+		try (Git git = initRepo()) {
+			// Create and commit initial file
+			setFile("committed.java").toContent("class Committed {}");
+			addAndCommit(git, "Initial commit");
+
+			// Create a new file that's not tracked at all (not in gitignore either)
+			setFile("new_untracked.java").toContent("class NewUntracked {}");
+
+			// The new untracked file should be considered dirty
+			ratchetFrom("main").onlyDirty("new_untracked.java");
+		}
+	}
+
+	@Test
+	void testModifiedTrackedFile_ShouldBeDirty() throws IllegalStateException, GitAPIException, IOException {
+		try (Git git = initRepo()) {
+			// Create and commit initial file
+			setFile("Main.java").toContent("class Main { void old() {} }");
+			addAndCommit(git, "Initial commit");
+
+			// Modify the file
+			setFile("Main.java").toContent("class Main { void newMethod() {} }");
+
+			// The modified file should be considered dirty
+			ratchetFrom("main").onlyDirty("Main.java");
+		}
+	}
+
 	static class GitRatchetSimple extends GitRatchet<File> {
 		@Override
 		protected File getDir(File project) {
@@ -81,14 +129,20 @@ class Asserter {
 		Asserter(String... ratchetFrom) {
 			this.ratchetFroms = ratchetFrom;
 			this.shas = Arrays.stream(ratchetFrom)
-					.map(from -> ratchet.rootTreeShaOf(rootFolder(), from))
-					.toArray(ObjectId[]::new);
+				.map(from -> ratchet.rootTreeShaOf(rootFolder(), from))
+				.toArray(ObjectId[]::new);
 		}
 
 		private void assertClean(int i, String filename, boolean expected) throws IOException {
-			boolean actual = ratchet.isClean(rootFolder(), shas[i], newFile(filename));
+			File file = new File(rootFolder(), filename);
+			if (!file.exists()) {
+				throw new AssertionError("File " + filename + " does not exist");
+			}
+
+			boolean actual = ratchet.isClean(rootFolder(), shas[i], file);
 			if (actual != expected) {
-				throw new AssertionError("Expected " + filename + " to be " + (expected ? "clean" : "dirty") + " relative to " + ratchetFroms[i]);
+				throw new AssertionError("Expected " + filename + " to be " + (expected ? "clean" : "dirty") +
+					" relative to " + ratchetFroms[i] + " but was " + (actual ? "clean" : "dirty"));
 			}
 		}
 
@@ -98,24 +152,31 @@ public void allClean() throws IOException {
 
 		public void allDirty() throws IOException {
 			String[] filenames = Arrays.stream(rootFolder().listFiles())
-					.filter(File::isFile)
-					.map(File::getName)
-					.toArray(String[]::new);
+				.filter(File::isFile)
+				.map(File::getName)
+				.toArray(String[]::new);
 			onlyDirty(filenames);
 		}
 
 		public void onlyDirty(String... filenames) throws IOException {
 			List<String> dirtyFiles = Arrays.asList(filenames);
-			for (File file : rootFolder().listFiles()) {
-				if (!file.isFile()) {
+			for (File file : getAllFilesRecursive(rootFolder())) {
+				if (file.isDirectory()) {
 					continue;
 				}
-				boolean expectedClean = !dirtyFiles.contains(file.getName());
+				String relativePath = rootFolder().toPath().relativize(file.toPath()).toString();
+				boolean expectedClean = !dirtyFiles.contains(relativePath);
 				for (int i = 0; i < shas.length; i++) {
-					assertClean(i, file.getName(), expectedClean);
+					assertClean(i, relativePath, expectedClean);
 				}
 			}
 		}
+
+		private List<File> getAllFilesRecursive(File dir) {
+			return Arrays.stream(dir.listFiles())
+				.flatMap(file -> file.isDirectory() ? getAllFilesRecursive(file).stream() : Arrays.stream(new File[]{file}))
+				.toList();
+		}
 	}
 
 	private Git initRepo() throws IllegalStateException, GitAPIException, IOException {