From 06ad687efd079f8d39c87d2be617898a89298808 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 1 Feb 2024 15:10:24 -0800 Subject: [PATCH] Deduplicate deprecation warnings for v0.2.0 release (#9764) ## Summary Adds an additional warning macro (we should consolidate these later) that shows a warning once based on the content of the warning itself. This is less efficient than `warn_user_once!` and `warn_user_by_id!`, but this is so expensive that it doesn't matter at all. Applies this macro to the various warnings for the v0.2.0 release, and also includes the filename in said warnings, so the FastAPI case is now: ```text warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in /Users/crmarsh/workspace/fastapi/pyproject.toml: - 'ignore' -> 'lint.ignore' - 'select' -> 'lint.select' - 'isort' -> 'lint.isort' - 'pyupgrade' -> 'lint.pyupgrade' - 'per-file-ignores' -> 'lint.per-file-ignores' ``` --------- Co-authored-by: Zanie --- crates/ruff/tests/format.rs | 8 +- crates/ruff/tests/lint.rs | 130 ++++++++++++++------- crates/ruff/tests/resolve_files.rs | 4 +- crates/ruff_linter/src/logging.rs | 25 +++- crates/ruff_wasm/src/lib.rs | 4 +- crates/ruff_workspace/src/configuration.rs | 26 +++-- crates/ruff_workspace/src/resolver.rs | 2 +- 7 files changed, 133 insertions(+), 66 deletions(-) diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index b834014146c64b..491711dee940cd 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -508,11 +508,9 @@ if __name__ == '__main__': say_hy("dear Ruff contributor") ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `ruff.toml`: - 'extend-select' -> 'lint.extend-select' - 'ignore' -> 'lint.ignore' - - "###); Ok(()) } @@ -551,11 +549,9 @@ if __name__ == '__main__': say_hy("dear Ruff contributor") ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `ruff.toml`: - 'extend-select' -> 'lint.extend-select' - 'ignore' -> 'lint.ignore' - - "###); Ok(()) } diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 7cc55eab8ccd9b..7bafd0b129c9c9 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -2,6 +2,7 @@ #![cfg(not(target_family = "wasm"))] +use regex::escape; use std::fs; use std::process::Command; use std::str; @@ -13,6 +14,10 @@ use tempfile::TempDir; const BIN_NAME: &str = "ruff"; const STDIN_BASE_OPTIONS: &[&str] = &["--no-cache", "--output-format", "concise"]; +fn tempdir_filter(tempdir: &TempDir) -> String { + format!(r"{}\\?/?", escape(tempdir.path().to_str().unwrap())) +} + #[test] fn top_level_options() -> Result<()> { let tempdir = TempDir::new()?; @@ -27,29 +32,32 @@ inline-quotes = "single" "#, )?; - assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) - .args(STDIN_BASE_OPTIONS) - .arg("--config") - .arg(&ruff_toml) - .args(["--stdin-filename", "test.py"]) - .arg("-") - .pass_stdin(r#"a = "abcba".strip("aba")"#), @r###" - success: false - exit_code: 1 - ----- stdout ----- - test.py:1:5: Q000 [*] Double quotes found but single quotes preferred - test.py:1:5: B005 Using `.strip()` with multi-character strings is misleading - test.py:1:19: Q000 [*] Double quotes found but single quotes preferred - Found 3 errors. - [*] 2 fixable with the `--fix` option. - - ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: - - 'extend-select' -> 'lint.extend-select' - - 'flake8-quotes' -> 'lint.flake8-quotes' - + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .arg("--config") + .arg(&ruff_toml) + .args(["--stdin-filename", "test.py"]) + .arg("-") + .pass_stdin(r#"a = "abcba".strip("aba")"#), @r###" + success: false + exit_code: 1 + ----- stdout ----- + test.py:1:5: Q000 [*] Double quotes found but single quotes preferred + test.py:1:5: B005 Using `.strip()` with multi-character strings is misleading + test.py:1:19: Q000 [*] Double quotes found but single quotes preferred + Found 3 errors. + [*] 2 fixable with the `--fix` option. + + ----- stderr ----- + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `[TMP]/ruff.toml`: + - 'extend-select' -> 'lint.extend-select' + - 'flake8-quotes' -> 'lint.flake8-quotes' + "###); + }); - "###); Ok(()) } @@ -68,6 +76,9 @@ inline-quotes = "single" "#, )?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .args(STDIN_BASE_OPTIONS) .arg("--config") @@ -85,6 +96,8 @@ inline-quotes = "single" ----- stderr ----- "###); + }); + Ok(()) } @@ -103,6 +116,9 @@ inline-quotes = "single" "#, )?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .args(STDIN_BASE_OPTIONS) .arg("--config") @@ -119,11 +135,11 @@ inline-quotes = "single" [*] 2 fixable with the `--fix` option. ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `[TMP]/ruff.toml`: - 'extend-select' -> 'lint.extend-select' - - "###); + }); + Ok(()) } @@ -146,6 +162,9 @@ inline-quotes = "single" "#, )?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .args(STDIN_BASE_OPTIONS) .arg("--config") @@ -162,11 +181,11 @@ inline-quotes = "single" [*] 2 fixable with the `--fix` option. ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `[TMP]/ruff.toml`: - 'flake8-quotes' -> 'lint.flake8-quotes' - - "###); + }); + Ok(()) } @@ -222,6 +241,9 @@ OTHER = "OTHER" fs::write(out_dir.join("a.py"), r#"a = "a""#)?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) .arg("check") @@ -241,11 +263,11 @@ OTHER = "OTHER" [*] 3 fixable with the `--fix` option. ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `ruff.toml`: - 'extend-select' -> 'lint.extend-select' - - "###); + }); + Ok(()) } @@ -266,6 +288,9 @@ inline-quotes = "single" "#, )?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) .arg("check") @@ -288,11 +313,11 @@ if __name__ == "__main__": [*] 2 fixable with the `--fix` option. ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `ruff.toml`: - 'extend-select' -> 'lint.extend-select' - - "###); + }); + Ok(()) } @@ -311,6 +336,9 @@ max-line-length = 100 "#, )?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .args(STDIN_BASE_OPTIONS) .arg("--config") @@ -330,12 +358,12 @@ _ = "--------------------------------------------------------------------------- Found 1 error. ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `[TMP]/ruff.toml`: - 'select' -> 'lint.select' - 'pycodestyle' -> 'lint.pycodestyle' - - "###); + }); + Ok(()) } @@ -353,6 +381,9 @@ inline-quotes = "single" "#, )?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) .arg("check") @@ -377,11 +408,11 @@ if __name__ == "__main__": [*] 1 fixable with the `--fix` option. ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `ruff.toml`: - 'extend-select' -> 'lint.extend-select' - - "###); + }); + Ok(()) } @@ -399,6 +430,9 @@ inline-quotes = "single" "#, )?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) .arg("check") @@ -423,11 +457,11 @@ if __name__ == "__main__": [*] 1 fixable with the `--fix` option. ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `ruff.toml`: - 'extend-select' -> 'lint.extend-select' - - "###); + }); + Ok(()) } @@ -456,6 +490,9 @@ ignore = ["D203", "D212"] "#, )?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(sub_dir) .arg("check") @@ -468,6 +505,8 @@ ignore = ["D203", "D212"] ----- stderr ----- warning: No Python files found under the given path(s) "###); + }); + Ok(()) } @@ -524,6 +563,9 @@ include = ["*.ipy"] "#, )?; + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .current_dir(tempdir.path()) .arg("check") @@ -540,5 +582,7 @@ include = ["*.ipy"] ----- stderr ----- "###); + }); + Ok(()) } diff --git a/crates/ruff/tests/resolve_files.rs b/crates/ruff/tests/resolve_files.rs index 6b8ea0044cbfda..4f647489552878 100644 --- a/crates/ruff/tests/resolve_files.rs +++ b/crates/ruff/tests/resolve_files.rs @@ -39,10 +39,8 @@ fn check_project_include_defaults() { [BASEPATH]/include-test/subdirectory/c.py ----- stderr ----- - warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration: + warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `nested-project/pyproject.toml`: - 'select' -> 'lint.select' - - "###); }); } diff --git a/crates/ruff_linter/src/logging.rs b/crates/ruff_linter/src/logging.rs index eb3a6584ae53f6..b97193a717bf50 100644 --- a/crates/ruff_linter/src/logging.rs +++ b/crates/ruff_linter/src/logging.rs @@ -8,6 +8,7 @@ use fern; use log::Level; use once_cell::sync::Lazy; use ruff_python_parser::{ParseError, ParseErrorType}; +use rustc_hash::FxHashSet; use ruff_source_file::{LineIndex, OneIndexed, SourceCode, SourceLocation}; @@ -15,7 +16,7 @@ use crate::fs; use crate::source_kind::SourceKind; use ruff_notebook::Notebook; -pub static WARNINGS: Lazy>> = Lazy::new(Mutex::default); +pub static IDENTIFIERS: Lazy>> = Lazy::new(Mutex::default); /// Warn a user once, with uniqueness determined by the given ID. #[macro_export] @@ -24,7 +25,7 @@ macro_rules! warn_user_once_by_id { use colored::Colorize; use log::warn; - if let Ok(mut states) = $crate::logging::WARNINGS.lock() { + if let Ok(mut states) = $crate::logging::IDENTIFIERS.lock() { if !states.contains(&$id) { let message = format!("{}", format_args!($($arg)*)); warn!("{}", message.bold()); @@ -34,6 +35,26 @@ macro_rules! warn_user_once_by_id { }; } +pub static MESSAGES: Lazy>> = Lazy::new(Mutex::default); + +/// Warn a user once, if warnings are enabled, with uniqueness determined by the content of the +/// message. +#[macro_export] +macro_rules! warn_user_once_by_message { + ($($arg:tt)*) => { + use colored::Colorize; + use log::warn; + + if let Ok(mut states) = $crate::logging::MESSAGES.lock() { + let message = format!("{}", format_args!($($arg)*)); + if !states.contains(&message) { + warn!("{}", message.bold()); + states.insert(message); + } + } + }; +} + /// Warn a user once, with uniqueness determined by the calling location itself. #[macro_export] macro_rules! warn_user_once { diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index de168f63a2c56c..cb18b337b31d8b 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -108,8 +108,8 @@ impl Workspace { #[wasm_bindgen(constructor)] pub fn new(options: JsValue) -> Result { let options: Options = serde_wasm_bindgen::from_value(options).map_err(into_error)?; - let configuration = - Configuration::from_options(options, Path::new(".")).map_err(into_error)?; + let configuration = Configuration::from_options(options, Path::new("."), Path::new(".")) + .map_err(into_error)?; let settings = configuration .into_settings(Path::new(".")) .map_err(into_error)?; diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 0c21393509e9c3..0cd2a8f14017f9 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -31,7 +31,8 @@ use ruff_linter::settings::types::{ }; use ruff_linter::settings::{LinterSettings, DEFAULT_SELECTORS, DUMMY_VARIABLE_RGX, TASK_TAGS}; use ruff_linter::{ - fs, warn_user, warn_user_once, warn_user_once_by_id, RuleSelector, RUFF_PKG_VERSION, + fs, warn_user_once, warn_user_once_by_id, warn_user_once_by_message, RuleSelector, + RUFF_PKG_VERSION, }; use ruff_python_formatter::{ DocstringCode, DocstringCodeLineWidth, MagicTrailingComma, QuoteStyle, @@ -395,8 +396,9 @@ impl Configuration { }) } - pub fn from_options(options: Options, project_root: &Path) -> Result { - warn_about_deprecated_top_level_lint_options(&options.lint_top_level.0); + /// Convert the [`Options`] read from the given [`Path`] into a [`Configuration`]. + pub fn from_options(options: Options, path: &Path, project_root: &Path) -> Result { + warn_about_deprecated_top_level_lint_options(&options.lint_top_level.0, path); let lint = if let Some(mut lint) = options.lint { lint.common = lint.common.combine(options.lint_top_level.0); @@ -429,7 +431,7 @@ impl Configuration { .output_format .map(|format| match format { SerializationFormat::Text => { - warn_user!(r#"Setting `output_format` to "text" is deprecated. Use "full" or "concise" instead. "text" will be treated as "{}"."#, SerializationFormat::default(options.preview.unwrap_or_default())); + warn_user_once!(r#"Setting `output_format` to "text" is deprecated. Use "full" or "concise" instead. "text" will be treated as "{}"."#, SerializationFormat::default(options.preview.unwrap_or_default())); SerializationFormat::default(options.preview.unwrap_or_default()) }, other => other @@ -1010,7 +1012,7 @@ impl LintConfiguration { if preview.mode.is_disabled() { for selection in deprecated_selectors.iter().sorted() { let (prefix, code) = selection.prefix_and_code(); - warn_user!( + warn_user_once_by_message!( "Rule `{prefix}{code}` is deprecated and will be removed in a future release.", ); } @@ -1038,7 +1040,9 @@ impl LintConfiguration { for selection in ignored_preview_selectors.iter().sorted() { let (prefix, code) = selection.prefix_and_code(); - warn_user!("Selection `{prefix}{code}` has no effect because preview is not enabled.",); + warn_user_once_by_message!( + "Selection `{prefix}{code}` has no effect because preview is not enabled.", + ); } let mut rules = RuleTable::empty(); @@ -1257,7 +1261,10 @@ pub fn resolve_src(src: &[String], project_root: &Path) -> Result> Ok(paths) } -fn warn_about_deprecated_top_level_lint_options(top_level_options: &LintCommonOptions) { +fn warn_about_deprecated_top_level_lint_options( + top_level_options: &LintCommonOptions, + path: &Path, +) { let mut used_options = Vec::new(); if top_level_options.allowed_confusables.is_some() { @@ -1447,8 +1454,9 @@ fn warn_about_deprecated_top_level_lint_options(top_level_options: &LintCommonOp .map(|option| format!("- '{option}' -> 'lint.{option}'")) .join("\n "); - warn_user!( - "The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in your configuration:\n {options_mapping}\n\n", + warn_user_once_by_message!( + "The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `{}`:\n {options_mapping}", + fs::relativize_path(path), ); } diff --git a/crates/ruff_workspace/src/resolver.rs b/crates/ruff_workspace/src/resolver.rs index e8603226238e1a..b1623461c9d960 100644 --- a/crates/ruff_workspace/src/resolver.rs +++ b/crates/ruff_workspace/src/resolver.rs @@ -264,7 +264,7 @@ fn resolve_configuration( let options = pyproject::load_options(&path)?; let project_root = relativity.resolve(&path); - let configuration = Configuration::from_options(options, &project_root)?; + let configuration = Configuration::from_options(options, &path, &project_root)?; // If extending, continue to collect. next = configuration.extend.as_ref().map(|extend| {