Deprecate ruff <path> ruff --explain, ruff --clean and `ruff --…

…generate-shell-completion` (#10169)
astral-sh · Feb 29, 2024 · eceffe7 · eceffe7
1 parent c73c497
commit eceffe7
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 39 deletions.
diff --git 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<ExitStatus> {
     {
         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
@@ -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 <RULE>` is deprecated. Use `ruff rule <RULE>` instead.")
+        }
+        Some("--clean") => {
+            Some("`ruff --clean` is deprecated. Use `ruff clean` instead.")
+        }
+        Some("--generate-shell-completion") => {
+            Some("`ruff --generate-shell-completion <SHELL>` is deprecated. Use `ruff generate-shell-completion <SHELL>` 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 <path>` is deprecated. Use `ruff check <path>` 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
@@ -12,9 +12,10 @@ const STDIN: &str = "l = 1";
 fn ruff_check(show_source: Option<bool>, output_format: Option<String>) -> 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
@@ -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
@@ -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/...tion_test__explain_status_codes_f401.snap → ...napshots/integration_test__rule_f401.snap b/...tion_test__explain_status_codes_f401.snap → ...napshots/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 -----
-