Skip to content

Fix hunk apply failure caused by trailing whitespace mismatch#10

Open
bmuschko wants to merge 1 commit intomainfrom
bmuschko/fix-hunk-header-apply
Open

Fix hunk apply failure caused by trailing whitespace mismatch#10
bmuschko wants to merge 1 commit intomainfrom
bmuschko/fix-hunk-header-apply

Conversation

@bmuschko
Copy link

@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

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: Ready to Review

Development

Successfully merging this pull request may close these issues.

2 participants