From cd7ee80354297ce53dc505ebf6f0f1d033b4638b Mon Sep 17 00:00:00 2001 From: vitali87 Date: Sun, 30 Aug 2026 14:03:36 +0100 Subject: [PATCH] fix: reject mode-only changes instead of silently dropping them A chmod-only change produces a diff entry with no hunks. Like binary files, nothing in the plan can claim it, coverage validation passes, and the mode change is missing from every sub-PR. Reject it up front with the same kind of message as binary files. --- pr_split/exceptions.py | 4 ++++ pr_split/planner/validator.py | 13 +++++++++---- tests/test_validator.py | 26 ++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/pr_split/exceptions.py b/pr_split/exceptions.py index 7e9b242..7eb6529 100644 --- a/pr_split/exceptions.py +++ b/pr_split/exceptions.py @@ -31,6 +31,10 @@ class ErrorMsg(StrEnum): "Diff contains binary files, which cannot be split into hunks: {files}." " Commit them separately and re-run" ) + HUNKLESS_FILES_UNSUPPORTED = ( + "Diff contains files with no text hunks (mode-only or empty changes), which" + " cannot be assigned to a sub-PR: {files}. Commit them separately and re-run" + ) def __call__(self, **kwargs: object) -> str: return self.value.format(**kwargs) if kwargs else self.value diff --git a/pr_split/planner/validator.py b/pr_split/planner/validator.py index 9cbe0c2..1bc5e13 100644 --- a/pr_split/planner/validator.py +++ b/pr_split/planner/validator.py @@ -10,15 +10,20 @@ def validate_no_binary_files(parsed_diff: ParsedDiff) -> None: - """Refuse diffs with binary files. + """Refuse diffs with binary files or hunk-less entries. - Binary patches carry no hunks, so coverage validation would pass - without any group claiming them and the change would silently be - dropped from every sub-PR. + Binary patches and mode-only changes carry no hunks, so coverage + validation would pass without any group claiming them and the change + would silently be dropped from every sub-PR. """ binary = [pf.path for pf in parsed_diff.patch_set if pf.is_binary_file] if binary: raise PlanValidationError(ErrorMsg.BINARY_FILES_UNSUPPORTED(files=", ".join(binary))) + # Mode-only changes (chmod +x) and other hunk-less entries have the same + # failure mode: nothing for a group to claim, so the change vanishes. + hunkless = [pf.path for pf in parsed_diff.patch_set if len(pf) == 0] + if hunkless: + raise PlanValidationError(ErrorMsg.HUNKLESS_FILES_UNSUPPORTED(files=", ".join(hunkless))) def validate_coverage(groups: list[Group], parsed_diff: ParsedDiff) -> None: diff --git a/tests/test_validator.py b/tests/test_validator.py index 134dca0..bfb53a9 100644 --- a/tests/test_validator.py +++ b/tests/test_validator.py @@ -226,3 +226,29 @@ def test_whole_file_overlapping_same_file_partial_raises(self) -> None: ] with pytest.raises(PlanValidationError, match="multiple groups"): validate_coverage(groups, parsed) + + +MODE_ONLY_DIFF = """\ +diff --git a/run.sh b/run.sh +old mode 100644 +new mode 100755 +diff --git a/a.py b/a.py +--- a/a.py ++++ b/a.py +@@ -1 +1 @@ +-x ++y +""" + + +class TestValidateNoHunklessFiles: + def test_mode_only_change_raises(self) -> None: + parsed = parse_diff(MODE_ONLY_DIFF) + with pytest.raises(PlanValidationError, match=r"no text hunks.*run\.sh"): + validate_no_binary_files(parsed) + + def test_validate_plan_rejects_mode_only_even_when_text_hunks_are_covered(self) -> None: + parsed = parse_diff(MODE_ONLY_DIFF) + groups = [_make_group("g1", [_ga("a.py", WHOLE, [0])], 2)] + with pytest.raises(PlanValidationError, match="no text hunks"): + validate_plan(groups, parsed, PlanDAG(groups), max_loc=400)