Skip to content

Commit 3f8508b

Browse files
authored
Merge pull request #6188 from ClickHouse/fix/vale-changed-lines-full-range
fix(vale): track full line range per hunk in CI annotation parser
2 parents f0d6685 + 0769e41 commit 3f8508b

2 files changed

Lines changed: 62 additions & 4 deletions

File tree

scripts/vale/changed_lines_to_json.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,14 @@ def get_changed_lines(file_path, base_sha, head_sha):
5151
changed_lines = []
5252
for line in diff_output.splitlines():
5353
if line.startswith("@@"):
54-
# Extract line number from git diff header
55-
match = re.search(r"^@@ -[0-9]+(?:,[0-9]+)? \+([0-9]+)(?:,[0-9]+)? @@", line)
54+
# Extract the full added range from a unified=0 hunk header.
55+
# Format: @@ -A,B +C[,D] @@ — D is omitted when exactly 1 line
56+
# was added; D is 0 for pure deletions.
57+
match = re.search(r"^@@ -[0-9]+(?:,[0-9]+)? \+([0-9]+)(?:,([0-9]+))? @@", line)
5658
if match:
57-
line_number = int(match.group(1))
58-
changed_lines.append(line_number)
59+
start = int(match.group(1))
60+
count = int(match.group(2)) if match.group(2) is not None else 1
61+
changed_lines.extend(range(start, start + count))
5962

6063
return changed_lines
6164
except subprocess.CalledProcessError as e:
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Unit tests for changed_lines_to_json.get_changed_lines().
4+
5+
Run from the repo root:
6+
python3 scripts/vale/test/test_changed_lines_to_json.py
7+
"""
8+
import os
9+
import sys
10+
import unittest
11+
from unittest.mock import patch
12+
13+
sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
14+
15+
import changed_lines_to_json
16+
from changed_lines_to_json import get_changed_lines
17+
18+
19+
class GetChangedLinesTest(unittest.TestCase):
20+
def _run(self, diff_output):
21+
with patch.object(changed_lines_to_json.subprocess, "check_output", return_value=diff_output):
22+
return get_changed_lines("dummy.md", "BASE", "HEAD")
23+
24+
def test_single_line_addition_no_count(self):
25+
# `+26` (count omitted) means exactly one added line
26+
self.assertEqual(self._run("@@ -25,0 +26 @@"), [26])
27+
28+
def test_multi_line_addition(self):
29+
# `+43,26` means 26 lines starting at 43 — this is the case PR #6183 hit
30+
self.assertEqual(self._run("@@ -41,0 +43,26 @@"), list(range(43, 69)))
31+
32+
def test_pure_deletion_yields_no_lines(self):
33+
# `+9,0` is a pure deletion — no lines exist in the new file to lint
34+
self.assertEqual(self._run("@@ -10,5 +9,0 @@"), [])
35+
36+
def test_modified_hunk(self):
37+
self.assertEqual(self._run("@@ -10,3 +10,5 @@"), [10, 11, 12, 13, 14])
38+
39+
def test_multiple_hunks_concatenated(self):
40+
diff = "\n".join([
41+
"@@ -25,0 +26 @@",
42+
"@@ -41,0 +43,3 @@",
43+
])
44+
self.assertEqual(self._run(diff), [26, 43, 44, 45])
45+
46+
def test_hunk_header_with_trailing_context(self):
47+
# Real git output often appends a context line after the second @@
48+
self.assertEqual(
49+
self._run("@@ -41,0 +43,2 @@ Some context line"),
50+
[43, 44],
51+
)
52+
53+
54+
if __name__ == "__main__":
55+
unittest.main()

0 commit comments

Comments
 (0)