Browse Source

git cl format: Fix return code when using both --dry-run and --diff

When running `git cl format` with both --dry-run and --diff flags, some
formatters weren't returning exit code 2 when files needed formatting.
This made it impossible to detect formatting issues when using both
flags together.

Modified the following formatters to consistently return exit code 2
when files need formatting under --dry-run --diff flags:
- java
- rustfmt
- swift-format
- yapf
- gn

Also fixed incorrect usage of --presubmit flag in rustfmt and
swift-format. This flag is only intended to filter out formatters that
have dedicated presubmit checks, not to control their return codes.
Since CheckPatchFormatted in presubmit_canned_checks.py always passes
both --dry-run and --presubmit flags, the presubmit behavior remains
unchanged.

Change-Id: I4c26f22fc197c700a5c08d42b96ea4fc535ce293
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6509717
Commit-Queue: Aleksei Khoroshilov <akhoroshilov@brave.com>
Auto-Submit: Aleksei Khoroshilov <akhoroshilov@brave.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Scott Lee <ddoman@chromium.org>
Aleksey Khoroshilov 3 months ago
parent
commit
a9cc320bf7
1 changed files with 7 additions and 7 deletions
  1. 7 7
      git_cl.py

+ 7 - 7
git_cl.py

@@ -6816,7 +6816,7 @@ def _RunGoogleJavaFormat(opts, paths, top_dir, upstream_commit):
             if stdout:
             if stdout:
                 if opts.diff:
                 if opts.diff:
                     sys.stdout.write('Requires formatting: ' + stdout)
                     sys.stdout.write('Requires formatting: ' + stdout)
-                else:
+                if opts.dry_run:
                     return_value = 2
                     return_value = 2
 
 
         return return_value
         return return_value
@@ -6842,7 +6842,7 @@ def _RunRustFmt(opts, rust_diff_files, top_dir, upstream_commit):
     cmd += rust_diff_files
     cmd += rust_diff_files
     rustfmt_exitcode = subprocess2.call(cmd)
     rustfmt_exitcode = subprocess2.call(cmd)
 
 
-    if opts.presubmit and rustfmt_exitcode != 0:
+    if opts.dry_run and rustfmt_exitcode != 0:
         return 2
         return 2
 
 
     return 0
     return 0
@@ -6867,7 +6867,7 @@ def _RunSwiftFormat(opts, swift_diff_files, top_dir, upstream_commit):
     cmd += swift_diff_files
     cmd += swift_diff_files
     swift_format_exitcode = subprocess2.call(cmd)
     swift_format_exitcode = subprocess2.call(cmd)
 
 
-    if opts.presubmit and swift_format_exitcode != 0:
+    if opts.dry_run and swift_format_exitcode != 0:
         return 2
         return 2
 
 
     return 0
     return 0
@@ -6929,7 +6929,7 @@ def _RunYapf(opts, paths, top_dir, upstream_commit):
                                 shell=sys.platform.startswith('win32'))
                                 shell=sys.platform.startswith('win32'))
             if opts.diff:
             if opts.diff:
                 sys.stdout.write(stdout)
                 sys.stdout.write(stdout)
-            elif len(stdout) > 0:
+            if opts.dry_run and len(stdout) > 0:
                 return_value = 2
                 return_value = 2
         else:
         else:
             cmd += ['-i']
             cmd += ['-i']
@@ -6946,11 +6946,11 @@ def _RunGnFormat(opts, paths, top_dir, upstream_commit):
         gn_ret = subprocess2.call(cmd + [path],
         gn_ret = subprocess2.call(cmd + [path],
                                   shell=sys.platform.startswith('win'),
                                   shell=sys.platform.startswith('win'),
                                   cwd=top_dir)
                                   cwd=top_dir)
-        if opts.dry_run and gn_ret == 2:
-            return_value = 2  # Not formatted.
-        elif opts.diff and gn_ret == 2:
+        if opts.diff and gn_ret == 2:
             # TODO this should compute and print the actual diff.
             # TODO this should compute and print the actual diff.
             print('This change has GN build file diff for ' + path)
             print('This change has GN build file diff for ' + path)
+        if opts.dry_run and gn_ret == 2:
+            return_value = 2  # Not formatted.
         elif gn_ret != 0:
         elif gn_ret != 0:
             # For non-dry run cases (and non-2 return values for dry-run), a
             # For non-dry run cases (and non-2 return values for dry-run), a
             # nonzero error code indicates a failure, probably because the
             # nonzero error code indicates a failure, probably because the