Change `--fix-only` exit semantics to mirror `--fix` by charliermarsh · Pull Request #4146 · astral-sh/ruff

charliermarsh · 2023-04-29T04:08:31Z

Summary

After this change, running ruff --fix-only will exit with status code 0 unless the program hits an internal error, while ruff --fix-only --exit-non-zero-on-fix will exit with status code 1 if at least one diagnostic is fixed. This effectively mirrors ruff --fix, taking into account that non-fixable diagnostics are ignored. Previously, the --exit-non-zero-on-fix flag had no effect when combined with --fix-only.

Closes #4092.

github-actions · 2023-04-29T04:18:18Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     14.8±0.53ms     2.7 MB/sec    1.01     15.0±0.51ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.18ms     4.7 MB/sec    1.02      3.6±0.09ms     4.6 MB/sec
linter/all-rules/numpy/globals.py          1.06   459.8±14.85µs     6.4 MB/sec    1.00   432.0±21.92µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.9±0.37ms     4.3 MB/sec    1.02      6.1±0.25ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.00      7.2±0.49ms     5.7 MB/sec    1.04      7.4±0.20ms     5.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1614.2±50.54µs    10.3 MB/sec    1.00  1593.5±33.90µs    10.4 MB/sec
linter/default-rules/numpy/globals.py      1.00    177.2±4.62µs    16.7 MB/sec    1.00    178.1±5.58µs    16.6 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.3±0.07ms     7.8 MB/sec    1.05      3.5±0.09ms     7.4 MB/sec
parser/large/dataset.py                    1.00      5.6±0.31ms     7.3 MB/sec    1.05      5.8±0.16ms     7.0 MB/sec
parser/numpy/ctypeslib.py                  1.00  1122.1±28.38µs    14.8 MB/sec    1.03  1160.6±40.73µs    14.3 MB/sec
parser/numpy/globals.py                    1.01    117.4±4.59µs    25.1 MB/sec    1.00    115.8±4.28µs    25.5 MB/sec
parser/pydantic/types.py                   1.00      2.4±0.05ms    10.4 MB/sec    1.02      2.5±0.06ms    10.2 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.02     12.4±0.17ms     3.3 MB/sec    1.00     12.1±0.08ms     3.4 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.2±0.03ms     5.3 MB/sec    1.00      3.1±0.02ms     5.3 MB/sec
linter/all-rules/numpy/globals.py          1.00    321.0±4.98µs     9.2 MB/sec    1.02    328.2±6.32µs     9.0 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.3±0.05ms     4.9 MB/sec    1.00      5.3±0.08ms     4.8 MB/sec
linter/default-rules/large/dataset.py      1.02      6.3±0.05ms     6.5 MB/sec    1.00      6.2±0.04ms     6.6 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1329.1±12.29µs    12.5 MB/sec    1.00  1316.6±12.05µs    12.6 MB/sec
linter/default-rules/numpy/globals.py      1.00    138.9±1.78µs    21.2 MB/sec    1.02    141.6±2.50µs    20.8 MB/sec
linter/default-rules/pydantic/types.py     1.02      2.9±0.03ms     8.9 MB/sec    1.00      2.8±0.02ms     9.1 MB/sec
parser/large/dataset.py                    1.00      4.9±0.05ms     8.3 MB/sec    1.01      4.9±0.05ms     8.3 MB/sec
parser/numpy/ctypeslib.py                  1.00    926.4±7.05µs    18.0 MB/sec    1.00    928.9±8.82µs    17.9 MB/sec
parser/numpy/globals.py                    1.00     95.4±1.06µs    30.9 MB/sec    1.00     95.8±1.38µs    30.8 MB/sec
parser/pydantic/types.py                   1.00      2.1±0.02ms    12.3 MB/sec    1.00      2.1±0.02ms    12.3 MB/sec

MichaReiser · 2023-04-29T15:54:58Z

crates/ruff_cli/src/lib.rs

+            if cli.diff {
                if !diagnostics.fixed.is_empty() {
                    return Ok(ExitStatus::Failure);
                }
-            } else if cli.exit_non_zero_on_fix {
-                if !diagnostics.fixed.is_empty() || !diagnostics.messages.is_empty() {
-                    return Ok(ExitStatus::Failure);
+            } else if fix_only {
+                if cli.exit_non_zero_on_fix {
+                    if !diagnostics.fixed.is_empty() {
+                        return Ok(ExitStatus::Failure);
+                    }
+                } else {
+                    return Ok(ExitStatus::Success);
                }
            } else {


I'm a bit confused. Isn't diff --fix and fix_only --fix-only? If so, shouldn't the cli.diff and fix_only branches be identical?

No, those aren't the same. --fix, --fix-only, and --diff are all different options.

MichaReiser · 2023-04-29T15:55:13Z

crates/ruff_cli/src/lib.rs

+                } else {
+                    return Ok(ExitStatus::Success);
                }


Nit: Safe to fallthrough.

Suggested change

} else {

return Ok(ExitStatus::Success);

}

}

MichaReiser · 2023-05-02T07:18:29Z

Nit: It may be helpful to add some documentation explaining why/when we exit with a negative exit code for each option.

charliermarsh added breaking Breaking API change cli Related to the command-line interface labels Apr 29, 2023

MichaReiser reviewed Apr 29, 2023

View reviewed changes

MichaReiser approved these changes May 2, 2023

View reviewed changes

Change --fix-only exit semantics to mirror --fix

42e8d9c

charliermarsh force-pushed the charlie/fix-only branch from 12366ac to 42e8d9c Compare May 4, 2023 18:57

charliermarsh enabled auto-merge (squash) May 4, 2023 18:57

charliermarsh merged commit b71cc37 into main May 4, 2023

charliermarsh deleted the charlie/fix-only branch May 4, 2023 19:03

Tenzer mentioned this pull request May 5, 2023

ruff 0.0.265 Homebrew/homebrew-core#130273

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change `--fix-only` exit semantics to mirror `--fix`#4146

Change `--fix-only` exit semantics to mirror `--fix`#4146
charliermarsh merged 1 commit intomainfrom
charlie/fix-only

charliermarsh commented Apr 29, 2023

Uh oh!

github-actions bot commented Apr 29, 2023 •

edited

Loading

Uh oh!

MichaReiser Apr 29, 2023

Uh oh!

charliermarsh May 2, 2023

Uh oh!

MichaReiser Apr 29, 2023

Uh oh!

MichaReiser commented May 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

charliermarsh commented Apr 29, 2023

Summary

Uh oh!

github-actions bot commented Apr 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Check Results

Ecosystem

Benchmark

Linux

Windows

Uh oh!

MichaReiser Apr 29, 2023

Choose a reason for hiding this comment

Uh oh!

charliermarsh May 2, 2023

Choose a reason for hiding this comment

Uh oh!

MichaReiser Apr 29, 2023

Choose a reason for hiding this comment

Uh oh!

MichaReiser commented May 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Apr 29, 2023 •

edited

Loading