From 6b222f42130deff0e4ae534cfe37486a72165fe8 Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 14 Sep 2023 11:56:35 -0500 Subject: [PATCH] Remove the `PREVIEW` rule selector --- crates/ruff/src/rule_selector.rs | 10 +---- crates/ruff_cli/tests/integration_test.rs | 39 ++++++-------------- crates/ruff_workspace/src/configuration.rs | 43 ++++++++++------------ 3 files changed, 32 insertions(+), 60 deletions(-) diff --git a/crates/ruff/src/rule_selector.rs b/crates/ruff/src/rule_selector.rs index 362dcb72b2129..fc3e3611af51d 100644 --- a/crates/ruff/src/rule_selector.rs +++ b/crates/ruff/src/rule_selector.rs @@ -15,10 +15,8 @@ use crate::settings::types::PreviewMode; pub enum RuleSelector { /// Select all rules (includes rules in preview if enabled) All, - /// Category to select all rules in preview (includes legacy nursery rules) - Preview, /// Legacy category to select all rules in the "nursery" which predated preview mode - #[deprecated(note = "Use `RuleSelector::Preview` for new rules instead")] + #[deprecated(note = "The nursery was replaced with 'preview mode' which has no selector")] Nursery, /// Legacy category to select both the `mccabe` and `flake8-comprehensions` linters /// via a single selector. @@ -54,7 +52,6 @@ impl FromStr for RuleSelector { "ALL" => Ok(Self::All), #[allow(deprecated)] "NURSERY" => Ok(Self::Nursery), - "PREVIEW" => Ok(Self::Preview), "C" => Ok(Self::C), "T" => Ok(Self::T), _ => { @@ -121,7 +118,6 @@ impl RuleSelector { RuleSelector::All => ("", "ALL"), #[allow(deprecated)] RuleSelector::Nursery => ("", "NURSERY"), - RuleSelector::Preview => ("", "PREVIEW"), RuleSelector::C => ("", "C"), RuleSelector::T => ("", "T"), RuleSelector::Prefix { prefix, .. } | RuleSelector::Rule { prefix, .. } => { @@ -185,9 +181,6 @@ impl RuleSelector { RuleSelector::Nursery => { RuleSelectorIter::Nursery(Rule::iter().filter(Rule::is_nursery)) } - RuleSelector::Preview => RuleSelectorIter::Nursery( - Rule::iter().filter(|rule| rule.is_preview() || rule.is_nursery()), - ), RuleSelector::C => RuleSelectorIter::Chain( Linter::Flake8Comprehensions .rules() @@ -301,7 +294,6 @@ impl RuleSelector { pub fn specificity(&self) -> Specificity { match self { RuleSelector::All => Specificity::All, - RuleSelector::Preview => Specificity::All, #[allow(deprecated)] RuleSelector::Nursery => Specificity::All, RuleSelector::T => Specificity::LinterGroup, diff --git a/crates/ruff_cli/tests/integration_test.rs b/crates/ruff_cli/tests/integration_test.rs index 4425a057a8a5b..4b2ff8d9ea873 100644 --- a/crates/ruff_cli/tests/integration_test.rs +++ b/crates/ruff_cli/tests/integration_test.rs @@ -341,7 +341,7 @@ fn nursery_group_selector_preview_enabled() { Found 2 errors. ----- stderr ----- - warning: The `NURSERY` selector has been deprecated. Use the `PREVIEW` selector instead. + warning: The `NURSERY` selector has been deprecated. "###); } @@ -439,38 +439,21 @@ fn preview_disabled_prefix_empty() { } #[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 +fn preview_group_selector() { + // `--select PREVIEW` should error (selector was removed) 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 + exit_code: 2 ----- stdout ----- - -:1:1: CPY001 Missing copyright notice at top of file - -:1:2: E225 Missing whitespace around operator - Found 2 errors. ----- stderr ----- + error: invalid value 'PREVIEW' for '--select ': Unknown rule selector: `PREVIEW` + + For more information, try '--help'. "###); } @@ -483,13 +466,13 @@ fn preview_enabled_group_ignore() { .args(args) .pass_stdin("I=42\n"), @r###" success: false - exit_code: 1 + exit_code: 2 ----- stdout ----- - -:1:1: E741 Ambiguous variable name: `I` - -:1:2: E225 Missing whitespace around operator - Found 2 errors. ----- stderr ----- + error: invalid value 'PREVIEW' for '--ignore ': Unknown rule selector: `PREVIEW` + + For more information, try '--help'. "###); } diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 04a92c0a83b58..8e98012327659 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -588,11 +588,12 @@ impl Configuration { #[allow(deprecated)] if matches!(selector, RuleSelector::Nursery) { let suggestion = if preview.is_disabled() { - "Use the `--preview` flag instead." + " Use the `--preview` flag instead." } else { - "Use the `PREVIEW` selector instead." + // We have no suggested alternative since there is intentionally no "PREVIEW" selector + "" }; - warn_user_once!("The `NURSERY` selector has been deprecated. {suggestion}"); + warn_user_once!("The `NURSERY` selector has been deprecated.{suggestion}"); } if preview.is_disabled() { @@ -1093,33 +1094,31 @@ mod tests { } #[test] - fn select_linter_preview() { + fn select_all_preview() { let actual = resolve_rules( [RuleSelection { - select: Some(vec![Linter::Flake8Copyright.into()]), + select: Some(vec![RuleSelector::All]), ..RuleSelection::default() }], Some(PreviewMode::Disabled), ); - let expected = RuleSet::empty(); - assert_eq!(actual, expected); + assert!(!actual.intersects(&RuleSet::from_rules(PREVIEW_RULES))); let actual = resolve_rules( [RuleSelection { - select: Some(vec![Linter::Flake8Copyright.into()]), + select: Some(vec![RuleSelector::All]), ..RuleSelection::default() }], Some(PreviewMode::Enabled), ); - let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); - assert_eq!(actual, expected); + assert!(actual.intersects(&RuleSet::from_rules(PREVIEW_RULES))); } #[test] - fn select_prefix_preview() { + fn select_linter_preview() { let actual = resolve_rules( [RuleSelection { - select: Some(vec![Flake8Copyright::_0.into()]), + select: Some(vec![Linter::Flake8Copyright.into()]), ..RuleSelection::default() }], Some(PreviewMode::Disabled), @@ -1129,7 +1128,7 @@ mod tests { let actual = resolve_rules( [RuleSelection { - select: Some(vec![Flake8Copyright::_0.into()]), + select: Some(vec![Linter::Flake8Copyright.into()]), ..RuleSelection::default() }], Some(PreviewMode::Enabled), @@ -1139,10 +1138,10 @@ mod tests { } #[test] - fn select_rule_preview() { + fn select_prefix_preview() { let actual = resolve_rules( [RuleSelection { - select: Some(vec![Refurb::_145.into()]), + select: Some(vec![Flake8Copyright::_0.into()]), ..RuleSelection::default() }], Some(PreviewMode::Disabled), @@ -1152,20 +1151,20 @@ mod tests { let actual = resolve_rules( [RuleSelection { - select: Some(vec![Refurb::_145.into()]), + select: Some(vec![Flake8Copyright::_0.into()]), ..RuleSelection::default() }], Some(PreviewMode::Enabled), ); - let expected = RuleSet::from_rule(Rule::SliceCopy); + let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); assert_eq!(actual, expected); } #[test] - fn select_preview() { + fn select_rule_preview() { let actual = resolve_rules( [RuleSelection { - select: Some(vec![RuleSelector::Preview]), + select: Some(vec![Refurb::_145.into()]), ..RuleSelection::default() }], Some(PreviewMode::Disabled), @@ -1175,14 +1174,12 @@ mod tests { let actual = resolve_rules( [RuleSelection { - select: Some(vec![RuleSelector::Preview]), + select: Some(vec![Refurb::_145.into()]), ..RuleSelection::default() }], Some(PreviewMode::Enabled), ); - - let expected = - RuleSet::from_rules(NURSERY_RULES).union(&RuleSet::from_rules(PREVIEW_RULES)); + let expected = RuleSet::from_rule(Rule::SliceCopy); assert_eq!(actual, expected); }