diff --git a/BREAKING_CHANGES.md b/BREAKING_CHANGES.md index 2e269c1f17..087c8fddbd 100644 --- a/BREAKING_CHANGES.md +++ b/BREAKING_CHANGES.md @@ -1,5 +1,16 @@ # Breaking Changes +## 0.0.265 + +### `--fix-only` now exits with a zero exit code, unless `--exit-non-zero-on-fix` is specified ([#4146](https://github.com/charliermarsh/ruff/pull/4146)) + +Previously, `--fix-only` would exit with a non-zero exit code if any fixes were applied. This +behavior was inconsistent with `--fix`, and further, meant that `--exit-non-zero-on-fix` was +effectively ignored when `--fix-only` was specified. + +Now, `--fix-only` will exit with a zero exit code, unless `--exit-non-zero-on-fix` is specified, +in which case it will exit with a non-zero exit code if any fixes were applied. + ## 0.0.260 ### Fixes are now represented as a list of edits ([#3709](https://github.com/charliermarsh/ruff/pull/3709)) diff --git a/crates/ruff_cli/src/lib.rs b/crates/ruff_cli/src/lib.rs index 2a0ef4ae42..dc7a1ae7a2 100644 --- a/crates/ruff_cli/src/lib.rs +++ b/crates/ruff_cli/src/lib.rs @@ -278,17 +278,33 @@ fn check(args: CheckArgs, log_level: LogLevel) -> Result { } if !cli.exit_zero { - if cli.diff || fix_only { + if cli.diff { + // If we're printing a diff, we always want to exit non-zero if there are + // any fixable violations (since we've printed the diff, but not applied the + // fixes). 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 we're only fixing, we want to exit zero (since we've fixed all fixable + // violations), unless we're explicitly asked to exit non-zero on fix. + if cli.exit_non_zero_on_fix { + if !diagnostics.fixed.is_empty() { + return Ok(ExitStatus::Failure); + } } } else { - if !diagnostics.messages.is_empty() { - return Ok(ExitStatus::Failure); + // If we're running the linter (not just fixing), we want to exit non-zero if + // there are any violations, unless we're explicitly asked to exit zero on + // fix. + if cli.exit_non_zero_on_fix { + if !diagnostics.fixed.is_empty() || !diagnostics.messages.is_empty() { + return Ok(ExitStatus::Failure); + } + } else { + if !diagnostics.messages.is_empty() { + return Ok(ExitStatus::Failure); + } } } }