From eceffe74a06f0b1163491624c6c696ebdf3648e9 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 29 Feb 2024 14:50:01 +0100 Subject: [PATCH] Deprecate `ruff ` `ruff --explain`, `ruff --clean` and `ruff --generate-shell-completion` (#10169) --- crates/ruff/src/lib.rs | 5 ++ crates/ruff/src/main.rs | 51 +++++++++++-------- crates/ruff/tests/deprecation.rs | 7 +-- crates/ruff/tests/integration_test.rs | 11 ++-- crates/ruff/tests/lint.rs | 8 +-- ....snap => integration_test__rule_f401.snap} | 3 +- 6 files changed, 46 insertions(+), 39 deletions(-) rename crates/ruff/tests/snapshots/{integration_test__explain_status_codes_f401.snap => integration_test__rule_f401.snap} (98%) diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index f2414af7b9974..db7ff1de92aed 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -128,6 +128,7 @@ pub fn run( command, log_level_args, }: Args, + deprecated_alias_warning: Option<&'static str>, ) -> Result { { let default_panic_hook = std::panic::take_hook(); @@ -158,6 +159,10 @@ pub fn run( let log_level = LogLevel::from(&log_level_args); set_up_logging(&log_level)?; + if let Some(deprecated_alias_warning) = deprecated_alias_warning { + warn_user!("{}", deprecated_alias_warning); + } + match command { Command::Version { output_format } => { commands::version::version(output_format)?; diff --git a/crates/ruff/src/main.rs b/crates/ruff/src/main.rs index 8fbbe0e971919..2f5fe9cfab95b 100644 --- a/crates/ruff/src/main.rs +++ b/crates/ruff/src/main.rs @@ -27,26 +27,42 @@ pub fn main() -> ExitCode { let mut args = argfile::expand_args_from(args, argfile::parse_fromfile, argfile::PREFIX).unwrap(); - // Clap doesn't support default subcommands but we want to run `check` by - // default for convenience and backwards-compatibility, so we just - // preprocess the arguments accordingly before passing them to Clap. - if let Some(arg) = args.get(1) { - if arg - .to_str() - .is_some_and(|arg| !Command::has_subcommand(rewrite_legacy_subcommand(arg))) + // We can't use `warn_user` here because logging isn't set up at this point + // and we also don't know if the user runs ruff with quiet. + // Keep the message and pass it to `run` that is responsible for emitting the warning. + let deprecated_alias_warning = match args.get(1).and_then(|arg| arg.to_str()) { + // Deprecated aliases that are handled by clap + Some("--explain") => { + Some("`ruff --explain ` is deprecated. Use `ruff rule ` instead.") + } + Some("--clean") => { + Some("`ruff --clean` is deprecated. Use `ruff clean` instead.") + } + Some("--generate-shell-completion") => { + Some("`ruff --generate-shell-completion ` is deprecated. Use `ruff generate-shell-completion ` instead.") + } + // Deprecated `ruff` alias to `ruff check` + // Clap doesn't support default subcommands but we want to run `check` by + // default for convenience and backwards-compatibility, so we just + // preprocess the arguments accordingly before passing them to Clap. + Some(arg) if !Command::has_subcommand(arg) && arg != "-h" && arg != "--help" && arg != "-V" && arg != "--version" - && arg != "help" - { - args.insert(1, "check".into()); - } - } + && arg != "help" => { + + { + args.insert(1, "check".into()); + Some("`ruff ` is deprecated. Use `ruff check ` instead.") + } + }, + _ => None + }; let args = Args::parse_from(args); - match run(args) { + match run(args, deprecated_alias_warning) { Ok(code) => code.into(), Err(err) => { #[allow(clippy::print_stderr)] @@ -65,12 +81,3 @@ pub fn main() -> ExitCode { } } } - -fn rewrite_legacy_subcommand(cmd: &str) -> &str { - match cmd { - "--explain" => "rule", - "--clean" => "clean", - "--generate-shell-completion" => "generate-shell-completion", - cmd => cmd, - } -} diff --git a/crates/ruff/tests/deprecation.rs b/crates/ruff/tests/deprecation.rs index 0d3215d3bdeb3..339adc549dae7 100644 --- a/crates/ruff/tests/deprecation.rs +++ b/crates/ruff/tests/deprecation.rs @@ -12,9 +12,10 @@ const STDIN: &str = "l = 1"; fn ruff_check(show_source: Option, output_format: Option) -> Command { let mut cmd = Command::new(get_cargo_bin(BIN_NAME)); let output_format = output_format.unwrap_or(format!("{}", SerializationFormat::default(false))); - cmd.arg("--output-format"); - cmd.arg(output_format); - cmd.arg("--no-cache"); + cmd.arg("check") + .arg("--output-format") + .arg(output_format) + .arg("--no-cache"); match show_source { Some(true) => { cmd.arg("--show-source"); diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs index f9e9c221acdd5..fcb2d9b9ebf6d 100644 --- a/crates/ruff/tests/integration_test.rs +++ b/crates/ruff/tests/integration_test.rs @@ -71,6 +71,7 @@ impl<'a> RuffCheck<'a> { /// Generate a [`Command`] for the `ruff check` command. fn build(self) -> Command { let mut cmd = ruff_cmd(); + cmd.arg("check"); if let Some(output_format) = self.output_format { cmd.args(["--output-format", output_format]); } @@ -805,13 +806,13 @@ fn full_output_format() { } #[test] -fn explain_status_codes_f401() { - assert_cmd_snapshot!(ruff_cmd().args(["--explain", "F401"])); +fn rule_f401() { + assert_cmd_snapshot!(ruff_cmd().args(["rule", "F401"])); } #[test] -fn explain_status_codes_ruf404() { - assert_cmd_snapshot!(ruff_cmd().args(["--explain", "RUF404"]), @r###" +fn rule_invalid_rule_name() { + assert_cmd_snapshot!(ruff_cmd().args(["rule", "RUF404"]), @r###" success: false exit_code: 2 ----- stdout ----- @@ -1348,7 +1349,7 @@ fn unreadable_pyproject_toml() -> Result<()> { // Don't `--isolated` since the configuration discovery is where the error happens let args = Args::parse_from(["", "check", "--no-cache", tempdir.path().to_str().unwrap()]); - let err = run(args).err().context("Unexpected success")?; + let err = run(args, None).err().context("Unexpected success")?; assert_eq!( err.chain() .map(std::string::ToString::to_string) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 96e347845fef1..579d185d53f57 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -12,7 +12,7 @@ use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; use tempfile::TempDir; const BIN_NAME: &str = "ruff"; -const STDIN_BASE_OPTIONS: &[&str] = &["--no-cache", "--output-format", "concise"]; +const STDIN_BASE_OPTIONS: &[&str] = &["check", "--no-cache", "--output-format", "concise"]; fn tempdir_filter(tempdir: &TempDir) -> String { format!(r"{}\\?/?", escape(tempdir.path().to_str().unwrap())) @@ -246,7 +246,6 @@ OTHER = "OTHER" }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) - .arg("check") .args(STDIN_BASE_OPTIONS) .args(["--config", &ruff_toml.file_name().unwrap().to_string_lossy()]) // Explicitly pass test.py, should be linted regardless of it being excluded by lint.exclude @@ -293,7 +292,6 @@ inline-quotes = "single" }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) - .arg("check") .args(STDIN_BASE_OPTIONS) .args(["--config", &ruff_toml.file_name().unwrap().to_string_lossy()]) .args(["--stdin-filename", "generated.py"]) @@ -386,7 +384,6 @@ inline-quotes = "single" }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) - .arg("check") .args(STDIN_BASE_OPTIONS) .args(["--config", &ruff_toml.file_name().unwrap().to_string_lossy()]) .args(["--stdin-filename", "generated.py"]) @@ -435,7 +432,6 @@ inline-quotes = "single" }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) - .arg("check") .args(STDIN_BASE_OPTIONS) .args(["--config", &ruff_toml.file_name().unwrap().to_string_lossy()]) .args(["--stdin-filename", "generated.py"]) @@ -495,7 +491,6 @@ ignore = ["D203", "D212"] }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(sub_dir) - .arg("check") .args(STDIN_BASE_OPTIONS) , @r###" success: true @@ -921,7 +916,6 @@ include = ["*.ipy"] }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) - .arg("check") .args(STDIN_BASE_OPTIONS) .args(["--config", &ruff_toml.file_name().unwrap().to_string_lossy()]) .args(["--extension", "ipy:ipynb"]) diff --git a/crates/ruff/tests/snapshots/integration_test__explain_status_codes_f401.snap b/crates/ruff/tests/snapshots/integration_test__rule_f401.snap similarity index 98% rename from crates/ruff/tests/snapshots/integration_test__explain_status_codes_f401.snap rename to crates/ruff/tests/snapshots/integration_test__rule_f401.snap index fb91ebbaa5fa5..cad44c30080e6 100644 --- a/crates/ruff/tests/snapshots/integration_test__explain_status_codes_f401.snap +++ b/crates/ruff/tests/snapshots/integration_test__rule_f401.snap @@ -3,7 +3,7 @@ source: crates/ruff/tests/integration_test.rs info: program: ruff args: - - "--explain" + - rule - F401 --- success: true @@ -68,4 +68,3 @@ else: - [Typing documentation: interface conventions](https://typing.readthedocs.io/en/latest/source/libraries.html#library-interface-public-and-private-symbols) ----- stderr ----- -