Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --exit-non-zero-on-format #16009

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add --exit-non-zero-on-format #16009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
05ff08b
Add --exit-non-zero-on-format
thejcannon Feb 7, 2025
d4ff7b6
regen config markdown
thejcannon Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions crates/ruff/src/args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,10 @@ pub struct FormatCommand {
/// The option can only be used when formatting a single file. Range formatting of notebooks is unsupported.
#[clap(long, help_heading = "Editor options", verbatim_doc_comment)]
pub range: Option<FormatRange>,

/// Exit with a non-zero status code if any files were modified via format, even if all files were formatted successfully.
#[arg(long, help_heading = "Miscellaneous")]
pub exit_non_zero_on_format: bool,
Comment on lines +532 to +533
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you find ruff format --exit-non-zero-on-format awkward? Should it be ruff format --exit-non-zero-on-change or --exit-non-zero-on-write? It might be nice to switch to an agnostic name.

Should we retain the on_fix language we use for ruff check? I see the motivation for not, but at the very least we probably want an alias for parity with ruff check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current name was meant to "reflect" "exit non zero on (action)", to mirror "on fix".

As for the ergonomics? I really don't have an opinion. I can't imagine anyone using this flag outside of some kind of config file or script file of some sort. In which case the name is really only as important as "can be discovered" and "name reflects what happens". So I think the bike shed on the name (past probably "exit non zero on ...") doesn't bother me. I'm happy to use whatever yall like most!

}

#[derive(Copy, Clone, Debug, clap::Parser)]
Expand Down Expand Up @@ -762,6 +766,7 @@ impl FormatCommand {
no_cache: self.no_cache,
stdin_filename: self.stdin_filename,
range: self.range,
exit_non_zero_on_format: self.exit_non_zero_on_format,
};

let cli_overrides = ExplicitConfigOverrides {
Expand Down Expand Up @@ -1046,6 +1051,7 @@ pub struct FormatArguments {
pub files: Vec<PathBuf>,
pub stdin_filename: Option<PathBuf>,
pub range: Option<FormatRange>,
pub exit_non_zero_on_format: bool,
}

/// A text range specified by line and column numbers.
Expand Down
6 changes: 5 additions & 1 deletion crates/ruff/src/commands/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,11 @@ pub(crate) fn format(
match mode {
FormatMode::Write => {
if errors.is_empty() {
Ok(ExitStatus::Success)
if cli.exit_non_zero_on_format && results.any_formatted() {
Ok(ExitStatus::Failure)
} else {
Ok(ExitStatus::Success)
}
} else {
Ok(ExitStatus::Error)
}
Expand Down
43 changes: 43 additions & 0 deletions crates/ruff/tests/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,49 @@ if __name__ == "__main__":
Ok(())
}

#[test]
fn exit_non_zero_on_format() -> Result<()> {
let tempdir = TempDir::new()?;

fs::write(
tempdir.path().join("main.py"),
r#"
from test import say_hy

if __name__ == "__main__":
say_hy("dear Ruff contributor")
"#,
)?;

// First format should exit with code 1 since the file needed formatting
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.current_dir(tempdir.path())
.args(["format", "--no-cache", "--isolated", "--exit-non-zero-on-format"])
.arg("main.py"), @r"
success: false
exit_code: 1
----- stdout -----
1 file reformatted

----- stderr -----
");

// Second format should exit with code 0 since no files needed formatting
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.current_dir(tempdir.path())
.args(["format", "--no-cache", "--isolated", "--exit-non-zero-on-format"])
.arg("main.py"), @r"
success: true
exit_code: 0
----- stdout -----
1 file left unchanged

----- stderr -----
");

Ok(())
}

#[test]
fn force_exclude() -> Result<()> {
let tempdir = TempDir::new()?;
Expand Down
3 changes: 3 additions & 0 deletions docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,9 @@ Miscellaneous:
Path to the cache directory [env: RUFF_CACHE_DIR=]
--stdin-filename <STDIN_FILENAME>
The name of the file when passing it through stdin
--exit-non-zero-on-format
Exit with a non-zero status code if any files were modified via
format, even if all files were formatted successfully

File selection:
--respect-gitignore
Expand Down
Loading