Skip to content

Fix hunk apply failure caused by trailing whitespace mismatch#10

Merged
timtebeek merged 2 commits intomainfrom
bmuschko/fix-hunk-header-apply
Mar 11, 2026
Merged

Fix hunk apply failure caused by trailing whitespace mismatch#10
timtebeek merged 2 commits intomainfrom
bmuschko/fix-hunk-header-apply

Conversation

@bmuschko
Copy link
Contributor

@bmuschko bmuschko commented Mar 7, 2026

Summary

Fixes PatchApplyException when applying multi-hunk patches where the source file has trailing whitespace on context lines that the patch does not (or vice versa).

Before: Context lines in canApplyAt() were compared using strict ByteBuffer.equals():

|| !newLines.get(pos).equals(slice(hunkLine, 1)))

This required an exact byte-for-byte match, so "Line 118 " (with trailing spaces) in the file would not match "Line 118" in the patch context, causing the hunk to be rejected with:

PatchApplyException: Error applying patch in test.txt, hunk HunkHeader[118,6->124,7]: hunk does not apply to file content
    at org.openrewrite.jgit.api.ApplyCommand.applyText(ApplyCommand.java:676)

After: Context lines are compared using equalsIgnoringTrailingWhitespace():

|| !equalsIgnoringTrailingWhitespace(newLines.get(pos), slice(hunkLine, 1)))

This strips trailing spaces, tabs, and \r from both the file line and the hunk context line before comparing. Since context lines are only used for positioning the hunk (they're not modified), requiring an exact trailing-whitespace match is unnecessarily strict and causes valid patches to be rejected.

Additionally fixes FileHeader.parseName() to strip trailing \r from parsed file paths when patches have CRLF line endings.

@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Mar 7, 2026
@bmuschko bmuschko added the bug Something isn't working label Mar 7, 2026
Copy link
Contributor

@pstreef pstreef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. The core change in canApplyAt() is well-motivated — context lines are only used for positioning, so requiring exact trailing-whitespace matches is unnecessarily strict.

Verified that RawText.getRawString() strips \n but leaves \r from CRLF lines, which is exactly what equalsIgnoringTrailingWhitespace() handles. The FileHeader.parseName() fix for CRLF patch paths is clean too.

Minor nits (non-blocking):

  • Tests use JUnit assertTrue — AssertJ (assertThat(...).contains(...)) would give better failure messages
  • A negative test for a genuinely mismatched context line would add confidence the comparison still rejects real mismatches

@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Mar 10, 2026
Copy link
Contributor

@pstreef pstreef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops duplicate

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.
@timtebeek timtebeek merged commit 37244d5 into main Mar 11, 2026
1 check passed
@timtebeek timtebeek deleted the bmuschko/fix-hunk-header-apply branch March 11, 2026 21:32
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants