bug fix for diff hunk parsing

The diff hunk can omit the number of lines after the line position.
For example, the following hunks are all valid
- @@ -1,2 +1,3 @@
- @@ -1 +1,3 @@
- @@ -1,2 +1 @@
- @@ -1 +1 @@

However, the existing regex assumes that the number of lines is always
given. It wasn't a problem before crrev.com/c/6952890, because -U3 was
applied always. However, since it changed the context lines to -U0,
git diff started generating hunks without the number of lines, if
a given changed section contains only a single line.

This CL fixes the regex.

Bug: 453641840
Change-Id: I67f680bc1d8fa046e373b065e8ccbb6ce652eb3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/7158599
Auto-Submit: Scott Lee <ddoman@chromium.org>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Scott Lee <ddoman@chromium.org>

presubmit_support.py

@@ -1248,7 +1248,8 @@ class AffectedFile(object):
         # The keeplinebreaks parameter to splitlines must be True or else the
         # CheckForWindowsLineEndings presubmit will be a NOP.
         for line in self.GenerateScmDiff().splitlines(keeplinebreaks):
-            m = re.match(r'^@@ [0-9\,\+\-]+ \+([0-9]+)\,[0-9]+ @@', line)
+            m = re.match(r'^@@ -[0-9]+(?:,[0-9]+)? \+([0-9]+)(?:,[0-9]+)? @@',
+                         line)
             if m:
                 line_num = int(m.groups(1)[0])
                 continue

tests/presubmit_support_test.py

@@ -99,6 +99,29 @@ class ProvidedDiffChangeTest(fake_repos.FakeReposTestBase):
         self._test('somewhere/else', ['not a top level file!'],
                    ['still not a top level file!'])
 
+    def test_changed_contents_of_modified_file(self):
+        diff = '\n'.join([
+            'diff --git a/foo b/foo',
+            'index d7ba659f..b7957f3 100644',
+            '--- a/foo',
+            '+++ b/foo',
+            '%s',
+            '-  foo1',
+            '+  foo2',
+        ])
+
+        def test(hunk):
+            change = self._create_change(diff % hunk)
+            afs = change.AffectedFiles()
+            self.assertEqual(len(afs), 1)
+            return afs[0].ChangedContents()
+
+        self.assertEqual(test('@@ -1,1 +1,1 @@'), [(1, '  foo2')])
+        self.assertEqual(test('@@ -290,1 +290 @@'), [(290, '  foo2')])
+        self.assertEqual(test('@@ -291 +291 @@'), [(291, '  foo2')])
+        self.assertEqual(test('@@ -292 +292,1 @@'), [(292, '  foo2')])
+        self.assertEqual(test('@@ -293,1 +293,1 @@'), [(293, '  foo2')])
+
     def test_unix_local_paths(self):
         if sys.platform == 'win32':
             self.assertIn(r'somewhere\else', self.change.LocalPaths())