From d90b5773673760dc7b9bb3639d48f2869d437c0f Mon Sep 17 00:00:00 2001 From: Benjamin Muschko Date: Sat, 7 Mar 2026 12:09:54 -0700 Subject: [PATCH 1/2] Fix hunk apply failure caused by trailing whitespace mismatch --- jgit/build.gradle.kts | 1 + .../openrewrite/jgit/api/ApplyCommand.java | 32 +- .../openrewrite/jgit/patch/FileHeader.java | 3 + .../jgit/api/ApplyCommandTest.java | 274 ++++++++++++++++++ 4 files changed, 309 insertions(+), 1 deletion(-) create mode 100644 jgit/src/test/java/org/openrewrite/jgit/api/ApplyCommandTest.java diff --git a/jgit/build.gradle.kts b/jgit/build.gradle.kts index 045159936..7f6149bb7 100644 --- a/jgit/build.gradle.kts +++ b/jgit/build.gradle.kts @@ -5,6 +5,7 @@ plugins { dependencies { implementation("com.googlecode.javaewah:JavaEWAH:1.1.13") compileOnly("org.slf4j:slf4j-api:1.7.36") + testRuntimeOnly("org.slf4j:slf4j-simple:1.7.36") } java { diff --git a/jgit/src/main/java/org/openrewrite/jgit/api/ApplyCommand.java b/jgit/src/main/java/org/openrewrite/jgit/api/ApplyCommand.java index e127d0ca2..d9ea89f7e 100644 --- a/jgit/src/main/java/org/openrewrite/jgit/api/ApplyCommand.java +++ b/jgit/src/main/java/org/openrewrite/jgit/api/ApplyCommand.java @@ -760,7 +760,8 @@ private boolean canApplyAt(List hunkLines, case ' ': case '-': if (pos >= limit - || !newLines.get(pos).equals(slice(hunkLine, 1))) { + || !equalsIgnoringTrailingWhitespace( + newLines.get(pos), slice(hunkLine, 1))) { return false; } pos++; @@ -777,6 +778,35 @@ private ByteBuffer slice(ByteBuffer b, int off) { return ByteBuffer.wrap(b.array(), newOffset, b.limit() - newOffset); } + private static boolean equalsIgnoringTrailingWhitespace(ByteBuffer a, + ByteBuffer b) { + int aEnd = a.limit(); + while (aEnd > a.position() + && isWhitespace(a.array()[aEnd - 1])) { + aEnd--; + } + int bEnd = b.limit(); + while (bEnd > b.position() + && isWhitespace(b.array()[bEnd - 1])) { + bEnd--; + } + int aLen = aEnd - a.position(); + int bLen = bEnd - b.position(); + if (aLen != bLen) { + return false; + } + for (int i = 0; i < aLen; i++) { + if (a.array()[a.position() + i] != b.array()[b.position() + i]) { + return false; + } + } + return true; + } + + private static boolean isWhitespace(byte b) { + return b == ' ' || b == '\t' || b == '\r'; + } + private boolean isNoNewlineAtEndOfFile(FileHeader fh) { List hunks = fh.getHunks(); if (hunks == null || hunks.isEmpty()) { diff --git a/jgit/src/main/java/org/openrewrite/jgit/patch/FileHeader.java b/jgit/src/main/java/org/openrewrite/jgit/patch/FileHeader.java index f6cda5b16..624df53e7 100644 --- a/jgit/src/main/java/org/openrewrite/jgit/patch/FileHeader.java +++ b/jgit/src/main/java/org/openrewrite/jgit/patch/FileHeader.java @@ -556,6 +556,9 @@ private String parseName(String expect, int ptr, int end) { r = decode(UTF_8, buf, ptr, tab - 1); } + if (r.endsWith("\r")) { + r = r.substring(0, r.length() - 1); + } if (r.equals(DEV_NULL)) r = DEV_NULL; return r; diff --git a/jgit/src/test/java/org/openrewrite/jgit/api/ApplyCommandTest.java b/jgit/src/test/java/org/openrewrite/jgit/api/ApplyCommandTest.java new file mode 100644 index 000000000..c73784090 --- /dev/null +++ b/jgit/src/test/java/org/openrewrite/jgit/api/ApplyCommandTest.java @@ -0,0 +1,274 @@ +/* + * Copyright (C) 2026, OpenRewrite and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.openrewrite.jgit.api; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openrewrite.jgit.lib.Repository; +import org.openrewrite.jgit.util.FileUtils; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for {@link ApplyCommand}, focusing on multi-hunk patch application. + */ +public class ApplyCommandTest { + + private Repository db; + private Git git; + private File trash; + + @BeforeEach + public void setUp() throws Exception { + trash = Files.createTempDirectory("jgit-apply-test").toFile(); + git = Git.init().setDirectory(trash).call(); + db = git.getRepository(); + } + + @AfterEach + public void tearDown() throws Exception { + if (db != null) { + db.close(); + } + if (trash != null) { + FileUtils.delete(trash, FileUtils.RECURSIVE | FileUtils.RETRY); + } + } + + /** + * Test applying a multi-hunk patch where both the file and patch use LF line endings. + */ + @Test + public void testMultiHunkPatchWithLF() throws Exception { + // Create a file with 150 lines using LF endings + StringBuilder fileContent = new StringBuilder(); + for (int i = 1; i <= 150; i++) { + fileContent.append("Line ").append(i).append('\n'); + } + writeFileAndCommit("test.txt", fileContent.toString()); + + // Create a patch with 2 hunks: + // Hunk 1: adds 6 lines around line 10 (net +6) + // Hunk 2: adds 1 line around line 118 (old) / 124 (new) + String patch = "diff --git a/test.txt b/test.txt\n" + + "--- a/test.txt\n" + + "+++ b/test.txt\n" + + "@@ -10,6 +10,12 @@\n" + + " Line 10\n" + + " Line 11\n" + + " Line 12\n" + + "+New Line A1\n" + + "+New Line A2\n" + + "+New Line A3\n" + + "+New Line A4\n" + + "+New Line A5\n" + + "+New Line A6\n" + + " Line 13\n" + + " Line 14\n" + + " Line 15\n" + + "@@ -118,6 +124,7 @@\n" + + " Line 118\n" + + " Line 119\n" + + " Line 120\n" + + "+New Line B\n" + + " Line 121\n" + + " Line 122\n" + + " Line 123\n"; + + ApplyResult result = applyPatch(patch); + assertTrue(result.getUpdatedFiles().size() > 0); + + String resultContent = readFile("test.txt"); + assertTrue(resultContent.contains("New Line A1")); + assertTrue(resultContent.contains("New Line A6")); + assertTrue(resultContent.contains("New Line B")); + } + + /** + * Test applying a patch with CRLF line endings to a file with LF line + * endings. The patch context lines will have trailing \r after + * getRawString strips the \n, causing mismatch with the LF-only file + * lines. + */ + @Test + public void testMultiHunkPatchWithCRLFPatchAndLFFile() throws Exception { + // Create a file with 150 lines using LF endings + StringBuilder fileContent = new StringBuilder(); + for (int i = 1; i <= 150; i++) { + fileContent.append("Line ").append(i).append('\n'); + } + writeFileAndCommit("test.txt", fileContent.toString()); + + // Create the same patch but with CRLF line endings + String patch = "diff --git a/test.txt b/test.txt\r\n" + + "--- a/test.txt\r\n" + + "+++ b/test.txt\r\n" + + "@@ -10,6 +10,12 @@\r\n" + + " Line 10\r\n" + + " Line 11\r\n" + + " Line 12\r\n" + + "+New Line A1\r\n" + + "+New Line A2\r\n" + + "+New Line A3\r\n" + + "+New Line A4\r\n" + + "+New Line A5\r\n" + + "+New Line A6\r\n" + + " Line 13\r\n" + + " Line 14\r\n" + + " Line 15\r\n" + + "@@ -118,6 +124,7 @@\r\n" + + " Line 118\r\n" + + " Line 119\r\n" + + " Line 120\r\n" + + "+New Line B\r\n" + + " Line 121\r\n" + + " Line 122\r\n" + + " Line 123\r\n"; + + ApplyResult result = applyPatch(patch); + assertTrue(result.getUpdatedFiles().size() > 0); + + String resultContent = readFile("test.txt"); + assertTrue(resultContent.contains("New Line A1")); + assertTrue(resultContent.contains("New Line B")); + } + + /** + * Test applying a patch with LF line endings to a file with CRLF line + * endings. The needsCrLfConversion logic should handle this case by + * converting the file to LF before comparison. + */ + @Test + public void testMultiHunkPatchWithLFPatchAndCRLFFile() throws Exception { + // Create a file with 150 lines using CRLF endings + StringBuilder fileContent = new StringBuilder(); + for (int i = 1; i <= 150; i++) { + fileContent.append("Line ").append(i).append("\r\n"); + } + writeFileAndCommit("test.txt", fileContent.toString()); + + // Create a patch with LF line endings + String patch = "diff --git a/test.txt b/test.txt\n" + + "--- a/test.txt\n" + + "+++ b/test.txt\n" + + "@@ -10,6 +10,12 @@\n" + + " Line 10\n" + + " Line 11\n" + + " Line 12\n" + + "+New Line A1\n" + + "+New Line A2\n" + + "+New Line A3\n" + + "+New Line A4\n" + + "+New Line A5\n" + + "+New Line A6\n" + + " Line 13\n" + + " Line 14\n" + + " Line 15\n" + + "@@ -118,6 +124,7 @@\n" + + " Line 118\n" + + " Line 119\n" + + " Line 120\n" + + "+New Line B\n" + + " Line 121\n" + + " Line 122\n" + + " Line 123\n"; + + ApplyResult result = applyPatch(patch); + assertTrue(result.getUpdatedFiles().size() > 0); + + String resultContent = readFile("test.txt"); + assertTrue(resultContent.contains("New Line A1")); + assertTrue(resultContent.contains("New Line B")); + } + + /** + * Test applying a patch where the file has trailing whitespace on some + * context lines but the patch does not. The trailing-whitespace-tolerant + * comparison should handle this. + */ + @Test + public void testMultiHunkPatchWithTrailingWhitespaceDifference() + throws Exception { + // Create a file where some lines have trailing spaces + StringBuilder fileContent = new StringBuilder(); + for (int i = 1; i <= 150; i++) { + if (i == 118 || i == 119 || i == 120) { + fileContent.append("Line ").append(i).append(" \n"); + } else { + fileContent.append("Line ").append(i).append('\n'); + } + } + writeFileAndCommit("test.txt", fileContent.toString()); + + // Patch context lines do NOT have trailing spaces + String patch = "diff --git a/test.txt b/test.txt\n" + + "--- a/test.txt\n" + + "+++ b/test.txt\n" + + "@@ -10,6 +10,12 @@\n" + + " Line 10\n" + + " Line 11\n" + + " Line 12\n" + + "+New Line A1\n" + + "+New Line A2\n" + + "+New Line A3\n" + + "+New Line A4\n" + + "+New Line A5\n" + + "+New Line A6\n" + + " Line 13\n" + + " Line 14\n" + + " Line 15\n" + + "@@ -118,6 +124,7 @@\n" + + " Line 118\n" + + " Line 119\n" + + " Line 120\n" + + "+New Line B\n" + + " Line 121\n" + + " Line 122\n" + + " Line 123\n"; + + ApplyResult result = applyPatch(patch); + assertTrue(result.getUpdatedFiles().size() > 0); + + String resultContent = readFile("test.txt"); + assertTrue(resultContent.contains("New Line A1")); + assertTrue(resultContent.contains("New Line B")); + } + + private void writeFileAndCommit(String name, String content) + throws Exception { + File f = new File(trash, name); + try (FileOutputStream fos = new FileOutputStream(f)) { + fos.write(content.getBytes(StandardCharsets.UTF_8)); + } + git.add().addFilepattern(name).call(); + git.commit().setMessage("initial").call(); + } + + private ApplyResult applyPatch(String patch) throws Exception { + InputStream in = new ByteArrayInputStream( + patch.getBytes(StandardCharsets.UTF_8)); + return git.apply().setPatch(in).call(); + } + + private String readFile(String name) throws IOException { + File f = new File(trash, name); + return new String(Files.readAllBytes(f.toPath()), + StandardCharsets.UTF_8); + } +} From 6c4895145091a43ece9af1d11bbcc83970ed5abe Mon Sep 17 00:00:00 2001 From: Benjamin Muschko Date: Tue, 10 Mar 2026 06:56:49 -0600 Subject: [PATCH 2/2] Improve test coverage: migrate to AssertJ and add negative test Switch ApplyCommandTest from JUnit assertTrue to AssertJ assertions for better failure messages. Add negative test to verify genuinely mismatched context lines are still rejected. Extract multiHunkPatch helper method. --- jgit/build.gradle.kts | 3 + .../jgit/api/ApplyCommandTest.java | 196 ++++++------------ 2 files changed, 72 insertions(+), 127 deletions(-) diff --git a/jgit/build.gradle.kts b/jgit/build.gradle.kts index 7f6149bb7..2a2222fc9 100644 --- a/jgit/build.gradle.kts +++ b/jgit/build.gradle.kts @@ -5,6 +5,9 @@ plugins { dependencies { implementation("com.googlecode.javaewah:JavaEWAH:1.1.13") compileOnly("org.slf4j:slf4j-api:1.7.36") + testImplementation("org.assertj:assertj-core:3.26.3") { + version { strictly("3.26.3") } + } testRuntimeOnly("org.slf4j:slf4j-simple:1.7.36") } diff --git a/jgit/src/test/java/org/openrewrite/jgit/api/ApplyCommandTest.java b/jgit/src/test/java/org/openrewrite/jgit/api/ApplyCommandTest.java index c73784090..6ef3bb041 100644 --- a/jgit/src/test/java/org/openrewrite/jgit/api/ApplyCommandTest.java +++ b/jgit/src/test/java/org/openrewrite/jgit/api/ApplyCommandTest.java @@ -12,6 +12,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.openrewrite.jgit.api.errors.PatchApplyException; import org.openrewrite.jgit.lib.Repository; import org.openrewrite.jgit.util.FileUtils; @@ -23,7 +24,8 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * Tests for {@link ApplyCommand}, focusing on multi-hunk patch application. @@ -56,156 +58,71 @@ public void tearDown() throws Exception { */ @Test public void testMultiHunkPatchWithLF() throws Exception { - // Create a file with 150 lines using LF endings StringBuilder fileContent = new StringBuilder(); for (int i = 1; i <= 150; i++) { fileContent.append("Line ").append(i).append('\n'); } writeFileAndCommit("test.txt", fileContent.toString()); - // Create a patch with 2 hunks: - // Hunk 1: adds 6 lines around line 10 (net +6) - // Hunk 2: adds 1 line around line 118 (old) / 124 (new) - String patch = "diff --git a/test.txt b/test.txt\n" - + "--- a/test.txt\n" - + "+++ b/test.txt\n" - + "@@ -10,6 +10,12 @@\n" - + " Line 10\n" - + " Line 11\n" - + " Line 12\n" - + "+New Line A1\n" - + "+New Line A2\n" - + "+New Line A3\n" - + "+New Line A4\n" - + "+New Line A5\n" - + "+New Line A6\n" - + " Line 13\n" - + " Line 14\n" - + " Line 15\n" - + "@@ -118,6 +124,7 @@\n" - + " Line 118\n" - + " Line 119\n" - + " Line 120\n" - + "+New Line B\n" - + " Line 121\n" - + " Line 122\n" - + " Line 123\n"; + String patch = multiHunkPatch("\n"); ApplyResult result = applyPatch(patch); - assertTrue(result.getUpdatedFiles().size() > 0); + assertThat(result.getUpdatedFiles()).isNotEmpty(); String resultContent = readFile("test.txt"); - assertTrue(resultContent.contains("New Line A1")); - assertTrue(resultContent.contains("New Line A6")); - assertTrue(resultContent.contains("New Line B")); + assertThat(resultContent).contains("New Line A1", "New Line A6", + "New Line B"); } /** * Test applying a patch with CRLF line endings to a file with LF line - * endings. The patch context lines will have trailing \r after - * getRawString strips the \n, causing mismatch with the LF-only file - * lines. + * endings. */ @Test public void testMultiHunkPatchWithCRLFPatchAndLFFile() throws Exception { - // Create a file with 150 lines using LF endings StringBuilder fileContent = new StringBuilder(); for (int i = 1; i <= 150; i++) { fileContent.append("Line ").append(i).append('\n'); } writeFileAndCommit("test.txt", fileContent.toString()); - // Create the same patch but with CRLF line endings - String patch = "diff --git a/test.txt b/test.txt\r\n" - + "--- a/test.txt\r\n" - + "+++ b/test.txt\r\n" - + "@@ -10,6 +10,12 @@\r\n" - + " Line 10\r\n" - + " Line 11\r\n" - + " Line 12\r\n" - + "+New Line A1\r\n" - + "+New Line A2\r\n" - + "+New Line A3\r\n" - + "+New Line A4\r\n" - + "+New Line A5\r\n" - + "+New Line A6\r\n" - + " Line 13\r\n" - + " Line 14\r\n" - + " Line 15\r\n" - + "@@ -118,6 +124,7 @@\r\n" - + " Line 118\r\n" - + " Line 119\r\n" - + " Line 120\r\n" - + "+New Line B\r\n" - + " Line 121\r\n" - + " Line 122\r\n" - + " Line 123\r\n"; + String patch = multiHunkPatch("\r\n"); ApplyResult result = applyPatch(patch); - assertTrue(result.getUpdatedFiles().size() > 0); + assertThat(result.getUpdatedFiles()).isNotEmpty(); String resultContent = readFile("test.txt"); - assertTrue(resultContent.contains("New Line A1")); - assertTrue(resultContent.contains("New Line B")); + assertThat(resultContent).contains("New Line A1", "New Line B"); } /** * Test applying a patch with LF line endings to a file with CRLF line - * endings. The needsCrLfConversion logic should handle this case by - * converting the file to LF before comparison. + * endings. */ @Test public void testMultiHunkPatchWithLFPatchAndCRLFFile() throws Exception { - // Create a file with 150 lines using CRLF endings StringBuilder fileContent = new StringBuilder(); for (int i = 1; i <= 150; i++) { fileContent.append("Line ").append(i).append("\r\n"); } writeFileAndCommit("test.txt", fileContent.toString()); - // Create a patch with LF line endings - String patch = "diff --git a/test.txt b/test.txt\n" - + "--- a/test.txt\n" - + "+++ b/test.txt\n" - + "@@ -10,6 +10,12 @@\n" - + " Line 10\n" - + " Line 11\n" - + " Line 12\n" - + "+New Line A1\n" - + "+New Line A2\n" - + "+New Line A3\n" - + "+New Line A4\n" - + "+New Line A5\n" - + "+New Line A6\n" - + " Line 13\n" - + " Line 14\n" - + " Line 15\n" - + "@@ -118,6 +124,7 @@\n" - + " Line 118\n" - + " Line 119\n" - + " Line 120\n" - + "+New Line B\n" - + " Line 121\n" - + " Line 122\n" - + " Line 123\n"; + String patch = multiHunkPatch("\n"); ApplyResult result = applyPatch(patch); - assertTrue(result.getUpdatedFiles().size() > 0); + assertThat(result.getUpdatedFiles()).isNotEmpty(); String resultContent = readFile("test.txt"); - assertTrue(resultContent.contains("New Line A1")); - assertTrue(resultContent.contains("New Line B")); + assertThat(resultContent).contains("New Line A1", "New Line B"); } /** * Test applying a patch where the file has trailing whitespace on some - * context lines but the patch does not. The trailing-whitespace-tolerant - * comparison should handle this. + * context lines but the patch does not. */ @Test public void testMultiHunkPatchWithTrailingWhitespaceDifference() throws Exception { - // Create a file where some lines have trailing spaces StringBuilder fileContent = new StringBuilder(); for (int i = 1; i <= 150; i++) { if (i == 118 || i == 119 || i == 120) { @@ -216,38 +133,63 @@ public void testMultiHunkPatchWithTrailingWhitespaceDifference() } writeFileAndCommit("test.txt", fileContent.toString()); - // Patch context lines do NOT have trailing spaces - String patch = "diff --git a/test.txt b/test.txt\n" - + "--- a/test.txt\n" - + "+++ b/test.txt\n" - + "@@ -10,6 +10,12 @@\n" - + " Line 10\n" - + " Line 11\n" - + " Line 12\n" - + "+New Line A1\n" - + "+New Line A2\n" - + "+New Line A3\n" - + "+New Line A4\n" - + "+New Line A5\n" - + "+New Line A6\n" - + " Line 13\n" - + " Line 14\n" - + " Line 15\n" - + "@@ -118,6 +124,7 @@\n" - + " Line 118\n" - + " Line 119\n" - + " Line 120\n" - + "+New Line B\n" - + " Line 121\n" - + " Line 122\n" - + " Line 123\n"; + String patch = multiHunkPatch("\n"); ApplyResult result = applyPatch(patch); - assertTrue(result.getUpdatedFiles().size() > 0); + assertThat(result.getUpdatedFiles()).isNotEmpty(); String resultContent = readFile("test.txt"); - assertTrue(resultContent.contains("New Line A1")); - assertTrue(resultContent.contains("New Line B")); + assertThat(resultContent).contains("New Line A1", "New Line B"); + } + + /** + * Test that a patch with genuinely mismatched context lines is still + * rejected. + */ + @Test + public void testMultiHunkPatchWithMismatchedContextIsRejected() + throws Exception { + StringBuilder fileContent = new StringBuilder(); + for (int i = 1; i <= 150; i++) { + fileContent.append("Line ").append(i).append('\n'); + } + // Replace line 119 with completely different content + String content = fileContent.toString() + .replace("Line 119\n", "DIFFERENT CONTENT\n"); + writeFileAndCommit("test.txt", content); + + String patch = multiHunkPatch("\n"); + + assertThatThrownBy(() -> applyPatch(patch)) + .isInstanceOf(PatchApplyException.class) + .hasMessageContaining("hunk"); + } + + private String multiHunkPatch(String eol) { + return "diff --git a/test.txt b/test.txt" + eol + + "--- a/test.txt" + eol + + "+++ b/test.txt" + eol + + "@@ -10,6 +10,12 @@" + eol + + " Line 10" + eol + + " Line 11" + eol + + " Line 12" + eol + + "+New Line A1" + eol + + "+New Line A2" + eol + + "+New Line A3" + eol + + "+New Line A4" + eol + + "+New Line A5" + eol + + "+New Line A6" + eol + + " Line 13" + eol + + " Line 14" + eol + + " Line 15" + eol + + "@@ -118,6 +124,7 @@" + eol + + " Line 118" + eol + + " Line 119" + eol + + " Line 120" + eol + + "+New Line B" + eol + + " Line 121" + eol + + " Line 122" + eol + + " Line 123" + eol; } private void writeFileAndCommit(String name, String content)