From ebd1b296fdcaa439ab51ac43df963b2b4f1e1231 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 13 Sep 2023 15:29:58 -0500 Subject: [PATCH] Add warnings for nursery and preview rule selection (#7210) ## Summary Adds warnings for cases where: - A selector does not include any rules because preview is disabled - A nursery rule is selected without the preview flag ## Test plan Add integration tests --- crates/ruff_cli/tests/integration_test.rs | 189 +++++++++++++++++++++ crates/ruff_workspace/src/configuration.rs | 43 ++++- 2 files changed, 229 insertions(+), 3 deletions(-) diff --git a/crates/ruff_cli/tests/integration_test.rs b/crates/ruff_cli/tests/integration_test.rs index acdf802b9f083..4425a057a8a5b 100644 --- a/crates/ruff_cli/tests/integration_test.rs +++ b/crates/ruff_cli/tests/integration_test.rs @@ -300,6 +300,195 @@ fn nursery_direct() { -:1:2: E225 Missing whitespace around operator Found 1 error. + ----- stderr ----- + warning: Selection of nursery rule `E225` without the `--preview` flag is deprecated. + "###); +} + +#[test] +fn nursery_group_selector() { + // Only nursery rules should be detected e.g. E225 and a warning should be displayed + let args = ["--select", "NURSERY"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("I=42\n"), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:1: CPY001 Missing copyright notice at top of file + -:1:2: E225 Missing whitespace around operator + Found 2 errors. + + ----- stderr ----- + warning: The `NURSERY` selector has been deprecated. Use the `--preview` flag instead. + "###); +} + +#[test] +fn nursery_group_selector_preview_enabled() { + // Only nursery rules should be detected e.g. E225 and a warning should be displayed + let args = ["--select", "NURSERY", "--preview"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("I=42\n"), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:1: CPY001 Missing copyright notice at top of file + -:1:2: E225 Missing whitespace around operator + Found 2 errors. + + ----- stderr ----- + warning: The `NURSERY` selector has been deprecated. Use the `PREVIEW` selector instead. + "###); +} + +#[test] +fn preview_enabled_prefix() { + // E741 and E225 (preview) should both be detected + let args = ["--select", "E", "--preview"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("I=42\n"), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:1: E741 Ambiguous variable name: `I` + -:1:2: E225 Missing whitespace around operator + Found 2 errors. + + ----- stderr ----- + "###); +} + +#[test] +fn preview_enabled_all() { + let args = ["--select", "ALL", "--preview"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("I=42\n"), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:1: E741 Ambiguous variable name: `I` + -:1:1: D100 Missing docstring in public module + -:1:1: CPY001 Missing copyright notice at top of file + -:1:2: E225 Missing whitespace around operator + Found 4 errors. + + ----- stderr ----- + warning: `one-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible. Ignoring `one-blank-line-before-class`. + warning: `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible. Ignoring `multi-line-summary-second-line`. + "###); +} + +#[test] +fn preview_enabled_direct() { + // E225 should be detected without warning + let args = ["--select", "E225", "--preview"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("I=42\n"), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:2: E225 Missing whitespace around operator + Found 1 error. + + ----- stderr ----- + "###); +} + +#[test] +fn preview_disabled_direct() { + // FURB145 is preview not nursery so selecting should be empty + let args = ["--select", "FURB145"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("a = l[:]\n"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: Selection `FURB145` has no effect because the `--preview` flag was not included. + "###); +} + +#[test] +fn preview_disabled_prefix_empty() { + // Warns that the selection is empty since all of the CPY rules are in preview + let args = ["--select", "CPY"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("I=42\n"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: Selection `CPY` has no effect because the `--preview` flag was not included. + "###); +} + +#[test] +fn preview_disabled_group_selector() { + // `--select PREVIEW` should warn without the `--preview` flag + let args = ["--select", "PREVIEW"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("I=42\n"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: Selection `PREVIEW` has no effect because the `--preview` flag was not included. + "###); +} + +#[test] +fn preview_enabled_group_selector() { + // `--select PREVIEW` is okay with the `--preview` flag and shouldn't warn + let args = ["--select", "PREVIEW", "--preview"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("I=42\n"), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:1: CPY001 Missing copyright notice at top of file + -:1:2: E225 Missing whitespace around operator + Found 2 errors. + + ----- stderr ----- + "###); +} + +#[test] +fn preview_enabled_group_ignore() { + // `--select E --ignore PREVIEW` should detect E741 and E225, which is in preview but "E" is more specific. + let args = ["--select", "E", "--ignore", "PREVIEW", "--preview"]; + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .args(args) + .pass_stdin("I=42\n"), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:1:1: E741 Ambiguous variable name: `I` + -:1:2: E225 Missing whitespace around operator + Found 2 errors. + ----- stderr ----- "###); } diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 5fd1e8b659fda..105ef947fa184 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -27,7 +27,7 @@ use ruff::settings::types::{ Version, }; use ruff::settings::{defaults, resolve_per_file_ignores, AllSettings, CliSettings, Settings}; -use ruff::{fs, warn_user_once_by_id, RuleSelector, RUFF_PKG_VERSION}; +use ruff::{fs, warn_user, warn_user_once, warn_user_once_by_id, RuleSelector, RUFF_PKG_VERSION}; use ruff_cache::cache_dir; use rustc_hash::{FxHashMap, FxHashSet}; use shellexpand; @@ -460,7 +460,10 @@ impl Configuration { let mut carryover_ignores: Option<&[RuleSelector]> = None; let mut carryover_unfixables: Option<&[RuleSelector]> = None; + // Store selectors for displaying warnings let mut redirects = FxHashMap::default(); + let mut deprecated_nursery_selectors = FxHashSet::default(); + let mut ignored_preview_selectors = FxHashSet::default(); for selection in &self.rule_selections { // If a selection only specifies extend-select we cannot directly @@ -571,8 +574,7 @@ impl Configuration { } } - // We insert redirects into the hashmap so that we - // can warn the users about remapped rule codes. + // Check for selections that require a warning for selector in selection .select .iter() @@ -583,6 +585,29 @@ impl Configuration { .chain(selection.unfixable.iter()) .chain(selection.extend_fixable.iter()) { + #[allow(deprecated)] + if matches!(selector, RuleSelector::Nursery) { + let suggestion = if preview.is_disabled() { + "Use the `--preview` flag instead." + } else { + "Use the `PREVIEW` selector instead." + }; + warn_user_once!("The `NURSERY` selector has been deprecated. {suggestion}"); + } + + if preview.is_disabled() { + if let RuleSelector::Rule { prefix, .. } = selector { + if prefix.rules().any(|rule| rule.is_nursery()) { + deprecated_nursery_selectors.insert(selector); + } + } + + // Check if the selector is empty because preview mode is disabled + if selector.rules(PreviewMode::Disabled).next().is_none() { + ignored_preview_selectors.insert(selector); + } + } + if let RuleSelector::Prefix { prefix, redirected_from: Some(redirect_from), @@ -603,6 +628,18 @@ impl Configuration { ); } + for selection in deprecated_nursery_selectors { + let (prefix, code) = selection.prefix_and_code(); + warn_user!("Selection of nursery rule `{prefix}{code}` without the `--preview` flag is deprecated.",); + } + + for selection in ignored_preview_selectors { + let (prefix, code) = selection.prefix_and_code(); + warn_user!( + "Selection `{prefix}{code}` has no effect because the `--preview` flag was not included.", + ); + } + let mut rules = RuleTable::empty(); for rule in select_set {