git_cl: add --input_diff_file for git cl format

If the option is given, git cl format uses the input diff file to
determine the files and sections to format instead of git-diff.

The intended use case is to remove git dependencies in case
presubmit_support.py is executed with a diff file and it calls
presubmit_canned_checks.CheckPatchFormatted().

Bug: 386840832
Change-Id: Ia1eacd1bdb81731e07753df19ebf1d2470c9f1aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6912693
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Scott Lee <ddoman@chromium.org>

git_cl.py

@@ -7082,17 +7082,67 @@ def _RunLUCICfgFormat(opts, paths, top_dir, upstream_commit):
 FormatterFunction = Callable[[Any, list[str], str, str], int]
 
 
+def _SplitDiffsByFile(diff_string: str) -> Dict[str, str]:
+    """Split a given diff string into per-file patches.
+
+    This function expects that the diff_string was generated with a prefix.
+    In other words, if it was generated by git-diff, don't add `--no-prefix`.
+    `diff` always generates one with prefixes.
+
+    Deleted files will be skipped.
+
+    Args:
+        diff_string: Unified diff
+    Returns:
+        a dict of file_path -> patch
+    """
+    if not diff_string:
+        return {}
+
+    file_diffs = re.split(r'^(?=diff --git)', diff_string, flags=re.MULTILINE)
+    file_diffs = [d for d in file_diffs if d.strip()]
+    ret = {}
+
+    for file_diff in file_diffs:
+        # Look for the line with `+++ ${prefix}/path/file.cc`
+        # For file deletion, the line would be omitted or with `+++ /dev/null`,
+        # which wouldn't match the regex.
+        match = re.search(r'^\+\+\+ [^\s/]+/(.+)',
+                          file_diff,
+                          flags=re.MULTILINE)
+        if match:
+            ret[match.group(1)] = file_diff
+
+    return ret
+
+
+def _FindFilesToFormat(
+        opts: optparse.Values, files: list[str] | None,
+        upstream_commit: str | None) -> Tuple[List[str], Dict[str, str] | None]:
+    """Returns a list of files to format and the diffs.
+
+    If opts.full, returns None for the diffs.
+    Deleted files are always excluded in the return.
+    """
+    if opts.input_diff_file:
+        with open(opts.input_diff_file) as f:
+            diffs = _SplitDiffsByFile(f.read())
+            return diffs.keys(), diffs
+
+    if opts.full:
+        files = RunGitDiffCmd(['--name-only', '--diff-filter=d'],
+                              upstream_commit, files).splitlines()
+        return files.keys(), None
+
+    diffs = _SplitDiffsByFile(
+        RunGitDiffCmd(['-U0'], upstream_commit, files, allow_prefix=True))
+    return diffs.keys(), diffs
+
+
 @subcommand.usage('[files or directories to diff]')
 @metrics.collector.collect_metrics('git cl format')
 def CMDformat(parser: optparse.OptionParser, args: list[str]):
     """Runs auto-formatting tools (clang-format etc.) on the diff."""
-    if gclient_utils.IsEnvCog():
-        print(
-            'format command is not supported in non-git environment. Please '
-            'use the "Format Modified Lines in All Files (git cl format)" '
-            'functionality in the command palette in the editor instead.',
-            file=sys.stderr)
-        return 1
     clang_exts = ['.cc', '.cpp', '.h', '.m', '.mm', '.proto']
     GN_EXTS = ['.gn', '.gni', '.typemap']
     parser.add_option('--full',
@@ -7154,26 +7204,46 @@ def CMDformat(parser: optparse.OptionParser, args: list[str]):
         dest='use_swift_format',
         action='store_false',
         help='Disables formatting of Swift file types using swift-format.')
-
+    parser.add_option('--input_diff_file',
+                      help='File to read input change diff from. '
+                      'If given, the added blocks of the files in the diff '
+                      'will be formatted.')
     parser.add_option('--mojom',
                       action='store_true',
                       help='Enables formatting of .mojom files.')
-
     parser.add_option('--no-java',
                       action='store_true',
                       help='Disable auto-formatting of .java')
-
     parser.add_option('--lucicfg',
                       action='store_true',
                       help='Enables formatting of .star files.')
 
     opts, files = parser.parse_args(args)
-    opts.full = opts.full or settings.GetFormatFullByDefault()
-
-    # Normalize files against the current path, so paths relative to the
-    # current directory are still resolved as expected.
-    files = [os.path.join(os.getcwd(), file) for file in files]
+    upstream_commit: str | None = None
+    upstream_branch: str | None = opts.upstream
+    top_dir: str | None = None
 
+    if opts.input_diff_file:
+        if opts.full:
+            print('--full and --input_diff_file cannot be used together.',
+                  file=sys.stderr)
+            return 1
+        if files:
+            print(
+                'No file paths to format are allowed '
+                'if --input_diff_file is given.',
+                file=sys.stderr)
+            return 1
+    elif gclient_utils.IsEnvCog():
+        print(
+            'Use --input_diff_file to run the format command in non-git '
+            'environments. In CiderG, please use the '
+            '"Format Modified Lines in All Files (git cl format)" '
+            'functionality in the command palette instead.',
+            file=sys.stderr)
+        return 1
+    else:
+        opts.full = opts.full or settings.GetFormatFullByDefault()
         # git diff generates paths against the root of the repository.  Change
         # to that directory so clang-format can find files even within subdirs.
         rel_base_path = settings.GetRelativeRoot()
@@ -7184,23 +7254,20 @@ def CMDformat(parser: optparse.OptionParser, args: list[str]):
         # branch when it was created or the last time it was rebased. This is
         # to cover the case where the user may have called "git fetch origin",
         # moving the origin branch to a newer commit, but hasn't rebased yet.
-    upstream_commit: str | None = None
-    upstream_branch: str | None = opts.upstream
         if not upstream_branch:
             cl = Changelist()
             upstream_branch = cl.GetUpstreamBranch()
         if upstream_branch:
             upstream_commit = RunGit(['merge-base', 'HEAD',
                                       upstream_branch]).strip()
-
         if not upstream_commit:
             DieWithError('Could not find base commit for this branch. '
                          'Are you in detached state?')
 
-    # Filter out deleted files
-    diff_output = RunGitDiffCmd(['--name-only', '--diff-filter=d'],
-                                upstream_commit, files)
-    diff_files = diff_output.splitlines()
+    # Normalize files against the current path, so paths relative to the
+    # current directory are still resolved as expected.
+    files = [os.path.join(os.getcwd(), file) for file in files]
+    diff_files, _ = _FindFilesToFormat(opts, files, upstream_commit)
 
     if opts.js:
         clang_exts.extend(['.js', '.ts'])
@@ -7224,7 +7291,7 @@ def CMDformat(parser: optparse.OptionParser, args: list[str]):
     if opts.lucicfg:
         formatters.append((['.star'], _RunLUCICfgFormat))
 
-    top_dir = settings.GetRoot()
+    top_dir = os.getcwd() if opts.presubmit else settings.GetRoot()
     return_value = 0
     for file_types, format_func in formatters:
         paths = [p for p in diff_files if p.lower().endswith(tuple(file_types))]

tests/git_cl_test.py

@@ -4880,6 +4880,65 @@ class MakeRequestsHelperTestCase(unittest.TestCase):
                          {'category': 'my-special-category'})
 
 
+test_format_input_diff = ("""
+diff --git a/README.md b/README.md
+--- a/README.md
++++ b/README.md
+@@ -6,11 +6,8 @@
+ The project's web site is https://www.chromium.org.
+
+ To check out the source code locally, don't use `git clone`! Instead,
+-follow [the instructions on how to get the code](docs/get_the_code.md).
++follow [the instructionource is rooted in [docs/README.md](docs/README.md).
+
+-Documentation in the source is rooted in [docs/README.md](docs/README.md).
+-
+-Learn how to [Get Around the Chromium Source Code Directory
+ Structure](https://www.chromium.org/developers/how-tos/getting-around-the-chrome-source-code).
+
+ For historical reasons, there are some small top level directories. Now the
+diff --git a/net/base/net_error_details.h b/net/base/net_error_details.h
+--- a/net/base/net_error_details.h
++++ b/net/base/net_error_details.h
+@@ -13,13 +13,12 @@
+ namespace net {
+
+ // A record of net errors with granular error specification generated by
+-// net stack.
+ struct NET_EXPORT NetErrorDetails {
+   NetErrorDetails()
+       : quic_broken(false), quic_connection_error(quic::QUIC_NO_ERROR) {}
+
+   NetErrorDetails(bool quic_broken, quic::QuicErrorCode quic_connection_error)
+-      : quic_broken(quic_broken),
++           : quic_broken(quic_broken),
+         quic_connection_error(quic_connection_error) {}
+
+   // True if all QUIC alternative services are marked broken for the origin.
+diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
+--- a/net/base/net_error_list.h
++++ b/net/base/net_error_list.h
+@@ -1063,7 +1063,6 @@
+ // were no outstanding references (no one is waiting to read) to keep the
+ // blob alive.
+ NET_ERROR(BLOB_DEREFERENCED_WHILE_BUILDING, -904)
+-
+ // A blob that we referenced during construction is broken, or a browser-side
+ // builder tries to build a blob with a blob reference that isn't finished
+ // constructing.
+""")
+
+test_format_input_diff_windows = "\r\n".join([
+    'diff --git "a/C:\\\\path\\\\to\\\\src\\\\file.cc" "b/C:\\\\path\\\\to\\\\dst\\\\file.cc"',
+    'index ce013625..dd7e1c6f 100644',
+    '--- "a/C:\\\\path\\\\to\\\\src\\\\file.cc',
+    '+++ "b/C:\\\\path\\\\to\\\\dst\\\\file.cc',
+    '@@ -1 +1 @@',
+    '-random',
+    '+content',
+])
+
+
 class CMDFormatTestCase(unittest.TestCase):
     def setUp(self):
         super(CMDFormatTestCase, self).setUp()
@@ -5169,6 +5228,71 @@ class CMDFormatTestCase(unittest.TestCase):
         mock_call.return_value = 255
         self.assertEqual(run(mock_opts, files, self._top_dir, 'HEAD'), 2)
 
+    @mock.patch('sys.stderr', io.StringIO())
+    def testInputDiffFileIsMutallyExclusiveWithFull(self):
+        """Verifies that opts.input_diff_file cannot be set with opts.full."""
+        ret = git_cl.main([
+            'format',
+            "--input_diff_file",
+            "/tmp/patch.diff",
+            "--full",
+        ])
+        self.assertEqual(1, ret)
+        self.assertEqual(
+            sys.stderr.getvalue().strip(),
+            '--full and --input_diff_file cannot be used together.',
+        )
+
+    @mock.patch('sys.stderr', io.StringIO())
+    def testInputDiffFileIsMutallyExclusiveWithPositionalFilePaths(self):
+        """Verifies that opts.input_diff_file cannot be set if file given."""
+        ret = git_cl.main([
+            'format',
+            "--input_diff_file",
+            "/tmp/patch.diff",
+            "common.cc",
+        ])
+        self.assertEqual(1, ret)
+        self.assertEqual(
+            sys.stderr.getvalue().strip(),
+            'No file paths to format are allowed '
+            'if --input_diff_file is given.',
+        )
+
+    @mock.patch('git_cl._RunClangFormatDiff', return_value=0)
+    def testInputDiffFile(self, clang_formatter):
+        """Tests git cl format with --input_diff_file."""
+        # Windows doesn't allow a file to be reopened while it's open by
+        # another handler.
+        with tempfile.NamedTemporaryFile(mode="w+", delete=False) as input_diff:
+            input_diff.write(test_format_input_diff)
+
+        try:
+            ret = git_cl.main(['format', "--input_diff_file", input_diff.name])
+            self.assertEqual(0, ret)
+            clang_formatter.assert_called_with(
+                mock.ANY,
+                ['net/base/net_error_details.h', 'net/base/net_error_list.h'],
+                mock.ANY, mock.ANY)
+        finally:
+            os.remove(input_diff.name)
+
+    @mock.patch('git_cl._RunClangFormatDiff', return_value=0)
+    def testInputDiffFileWithWindowsPatch(self, clang_formatter):
+        # Windows doesn't allow a file to be reopened while it's open by
+        # another handler.
+        with tempfile.NamedTemporaryFile(mode="w+", delete=False) as input_diff:
+            input_diff.write(test_format_input_diff_windows)
+
+        try:
+            ret = git_cl.main(['format', "--input_diff_file", input_diff.name])
+            self.assertEqual(0, ret)
+            clang_formatter.assert_called_with(
+                mock.ANY, ['C:\\\\path\\\\to\\\\dst\\\\file.cc'], mock.ANY,
+                mock.ANY)
+        finally:
+            os.remove(input_diff.name)
+
 
 @unittest.skipIf(gclient_utils.IsEnvCog(),
                 'not supported in non-git environment')