From 7aa60a4b466f8520089e587732a99f984b05c2ef Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 25 Jun 2024 10:11:43 +0200 Subject: [PATCH] Error instead of removing 'text' --- crates/ruff/src/args.rs | 15 +++++++-- crates/ruff/src/printer.rs | 11 ++++-- crates/ruff/tests/deprecation.rs | 37 ++++++++++++++++++++ crates/ruff_linter/src/settings/types.rs | 9 +++++ crates/ruff_workspace/src/configuration.rs | 11 ++++++ docs/configuration.md | 2 +- ruff.schema.json | 39 ++++++++++++++-------- 7 files changed, 105 insertions(+), 19 deletions(-) create mode 100644 crates/ruff/tests/deprecation.rs diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index 4dd84bb93bcd01..2222c3d656974a 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; -use anyhow::bail; +use anyhow::{anyhow, bail}; use clap::builder::{TypedValueParser, ValueParserFactory}; use clap::{command, Parser}; use colored::Colorize; @@ -693,7 +693,7 @@ impl CheckCommand { unsafe_fixes: resolve_bool_arg(self.unsafe_fixes, self.no_unsafe_fixes) .map(UnsafeFixes::from), force_exclude: resolve_bool_arg(self.force_exclude, self.no_force_exclude), - output_format: self.output_format, + output_format: resolve_output_format(self.output_format)?, show_fixes: resolve_bool_arg(self.show_fixes, self.no_show_fixes), extension: self.extension, }; @@ -921,6 +921,17 @@ The path `{value}` does not point to a configuration file" } } +#[allow(deprecated)] +fn resolve_output_format( + output_format: Option, +) -> anyhow::Result> { + if let Some(OutputFormat::Text) = output_format { + Err(anyhow!("`--output-format=text` is deprecated. Use `--output-format=full` or `--output-format=concise` instead.")) + } else { + Ok(output_format) + } +} + /// CLI settings that are distinct from configuration (commands, lists of files, /// etc.). #[allow(clippy::struct_excessive_bools)] diff --git a/crates/ruff/src/printer.rs b/crates/ruff/src/printer.rs index d15ba88c10086e..cef5596df8b4e6 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -240,9 +240,13 @@ impl Printer { } if !self.flags.intersects(Flags::SHOW_VIOLATIONS) { + #[allow(deprecated)] if matches!( self.format, - OutputFormat::Full | OutputFormat::Concise | OutputFormat::Grouped + OutputFormat::Text + | OutputFormat::Full + | OutputFormat::Concise + | OutputFormat::Grouped ) { if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) { if !diagnostics.fixed.is_empty() { @@ -320,6 +324,8 @@ impl Printer { OutputFormat::Sarif => { SarifEmitter.emit(writer, &diagnostics.messages, &context)?; } + #[allow(deprecated)] + OutputFormat::Text => unreachable!("Text is deprecated and should have been automatically converted to the default serialization format") } writer.flush()?; @@ -362,7 +368,8 @@ impl Printer { } match self.format { - OutputFormat::Full | OutputFormat::Concise => { + #[allow(deprecated)] + OutputFormat::Text | OutputFormat::Full | OutputFormat::Concise => { // Compute the maximum number of digits in the count and code, for all messages, // to enable pretty-printing. let count_width = num_digits( diff --git a/crates/ruff/tests/deprecation.rs b/crates/ruff/tests/deprecation.rs new file mode 100644 index 00000000000000..8a6d44c9c98a52 --- /dev/null +++ b/crates/ruff/tests/deprecation.rs @@ -0,0 +1,37 @@ +//! A test suite that ensures deprecated command line options have appropriate warnings / behaviors + +use ruff_linter::settings::types::OutputFormat; +use std::process::Command; + +use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; + +const BIN_NAME: &str = "ruff"; + +const STDIN: &str = "l = 1"; + +fn ruff_check(output_format: OutputFormat) -> Command { + let mut cmd = Command::new(get_cargo_bin(BIN_NAME)); + let output_format = output_format.to_string(); + cmd.arg("check") + .arg("--output-format") + .arg(output_format) + .arg("--no-cache"); + cmd.arg("-"); + + cmd +} + +#[test] +#[allow(deprecated)] +fn ensure_output_format_is_deprecated() { + assert_cmd_snapshot!(ruff_check(OutputFormat::Text).pass_stdin(STDIN), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:1: E741 Ambiguous variable name: `l` + Found 1 error. + + ----- stderr ----- + warning: `--output-format=text` is deprecated. Use `--output-format=full` or `--output-format=concise` instead. `text` will be treated as `concise`. + "###); +} diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index 704a3838b89e92..a5c2dae9c452eb 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -1,3 +1,5 @@ +#![allow(deprecated)] + use std::fmt::{Display, Formatter}; use std::hash::{Hash, Hasher}; use std::ops::Deref; @@ -505,6 +507,12 @@ impl FromIterator for ExtensionMapping { #[serde(rename_all = "kebab-case")] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub enum OutputFormat { + // Remove the module level `#![allow(deprecated)` when removing the text variant. + // Adding the `#[deprecated]` attribute to text creates clippy warnings about + // using a deprecated item in the derived code and there seems to be no way to suppress the clippy error + // other than disabling the warning for the entire module and/or moving `OutputFormat` to another module. + #[deprecated(note = "Use `concise` or `full` instead")] + Text, Concise, #[default] Full, @@ -523,6 +531,7 @@ pub enum OutputFormat { impl Display for OutputFormat { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { + Self::Text => write!(f, "text"), Self::Concise => write!(f, "concise"), Self::Full => write!(f, "full"), Self::Json => write!(f, "json"), diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index b7e3189e4254e3..004cbca7d07c51 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -427,6 +427,17 @@ impl Configuration { options.indent_width.or(options.tab_size) }; + #[allow(deprecated)] + if options.output_format == Some(OutputFormat::Text) { + let config_to_update = path.map_or_else( + || String::from("your `--config` CLI arguments"), + |path| format!("`{}`", fs::relativize_path(path)), + ); + return Err(anyhow!( + r#"The Setting `output_format=text` has been deprecated. Update {config_to_update} to use `output-format="concise"` or `output-format="full"` instead."# + )); + } + Ok(Self { builtins: options.builtins, cache_dir: options diff --git a/docs/configuration.md b/docs/configuration.md index 42c6bf3e040459..3749325b03fed3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -595,7 +595,7 @@ Options: --output-format Output serialization format for violations. The default serialization format is "concise". In preview mode, the default serialization - format is "full" [env: RUFF_OUTPUT_FORMAT=] [possible values: + format is "full" [env: RUFF_OUTPUT_FORMAT=] [possible values: text, concise, full, json, json-lines, junit, grouped, github, gitlab, pylint, rdjson, azure, sarif] -o, --output-file diff --git a/ruff.schema.json b/ruff.schema.json index 0dc93d5e540b38..62c6520c3ebf64 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2294,20 +2294,31 @@ "additionalProperties": false }, "OutputFormat": { - "type": "string", - "enum": [ - "concise", - "full", - "json", - "json-lines", - "junit", - "grouped", - "github", - "gitlab", - "pylint", - "rdjson", - "azure", - "sarif" + "oneOf": [ + { + "type": "string", + "enum": [ + "concise", + "full", + "json", + "json-lines", + "junit", + "grouped", + "github", + "gitlab", + "pylint", + "rdjson", + "azure", + "sarif" + ] + }, + { + "deprecated": true, + "type": "string", + "enum": [ + "text" + ] + } ] }, "ParametrizeNameType": {