Remove the PREVIEW rule selector (#7389)

The rule selector is not useful because `--select PREVIEW` only targets
Ruff developers and `--ignore PREVIEW` has no effect due to its low
specificity. We may restore it later if useful.
This commit is contained in:
Zanie Blue 2023-09-14 12:31:59 -05:00 committed by GitHub
parent d39eae2713
commit b9bb6bf780
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 65 deletions

View file

@ -15,10 +15,8 @@ use crate::settings::types::PreviewMode;
pub enum RuleSelector { pub enum RuleSelector {
/// Select all rules (includes rules in preview if enabled) /// Select all rules (includes rules in preview if enabled)
All, 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 /// 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, Nursery,
/// Legacy category to select both the `mccabe` and `flake8-comprehensions` linters /// Legacy category to select both the `mccabe` and `flake8-comprehensions` linters
/// via a single selector. /// via a single selector.
@ -54,7 +52,6 @@ impl FromStr for RuleSelector {
"ALL" => Ok(Self::All), "ALL" => Ok(Self::All),
#[allow(deprecated)] #[allow(deprecated)]
"NURSERY" => Ok(Self::Nursery), "NURSERY" => Ok(Self::Nursery),
"PREVIEW" => Ok(Self::Preview),
"C" => Ok(Self::C), "C" => Ok(Self::C),
"T" => Ok(Self::T), "T" => Ok(Self::T),
_ => { _ => {
@ -121,7 +118,6 @@ impl RuleSelector {
RuleSelector::All => ("", "ALL"), RuleSelector::All => ("", "ALL"),
#[allow(deprecated)] #[allow(deprecated)]
RuleSelector::Nursery => ("", "NURSERY"), RuleSelector::Nursery => ("", "NURSERY"),
RuleSelector::Preview => ("", "PREVIEW"),
RuleSelector::C => ("", "C"), RuleSelector::C => ("", "C"),
RuleSelector::T => ("", "T"), RuleSelector::T => ("", "T"),
RuleSelector::Prefix { prefix, .. } | RuleSelector::Rule { prefix, .. } => { RuleSelector::Prefix { prefix, .. } | RuleSelector::Rule { prefix, .. } => {
@ -185,9 +181,6 @@ impl RuleSelector {
RuleSelector::Nursery => { RuleSelector::Nursery => {
RuleSelectorIter::Nursery(Rule::iter().filter(Rule::is_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( RuleSelector::C => RuleSelectorIter::Chain(
Linter::Flake8Comprehensions Linter::Flake8Comprehensions
.rules() .rules()
@ -301,7 +294,6 @@ impl RuleSelector {
pub fn specificity(&self) -> Specificity { pub fn specificity(&self) -> Specificity {
match self { match self {
RuleSelector::All => Specificity::All, RuleSelector::All => Specificity::All,
RuleSelector::Preview => Specificity::All,
#[allow(deprecated)] #[allow(deprecated)]
RuleSelector::Nursery => Specificity::All, RuleSelector::Nursery => Specificity::All,
RuleSelector::T => Specificity::LinterGroup, RuleSelector::T => Specificity::LinterGroup,

View file

@ -341,7 +341,7 @@ fn nursery_group_selector_preview_enabled() {
Found 2 errors. Found 2 errors.
----- stderr ----- ----- 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] #[test]
fn preview_disabled_group_selector() { fn preview_group_selector() {
// `--select PREVIEW` should warn without the `--preview` flag // `--select PREVIEW` should error (selector was removed)
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"]; let args = ["--select", "PREVIEW", "--preview"];
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS) .args(STDIN_BASE_OPTIONS)
.args(args) .args(args)
.pass_stdin("I=42\n"), @r###" .pass_stdin("I=42\n"), @r###"
success: false success: false
exit_code: 1 exit_code: 2
----- stdout ----- ----- stdout -----
-:1:1: CPY001 Missing copyright notice at top of file
-:1:2: E225 Missing whitespace around operator
Found 2 errors.
----- stderr ----- ----- stderr -----
error: invalid value 'PREVIEW' for '--select <RULE_CODE>': Unknown rule selector: `PREVIEW`
For more information, try '--help'.
"###); "###);
} }
@ -483,13 +466,13 @@ fn preview_enabled_group_ignore() {
.args(args) .args(args)
.pass_stdin("I=42\n"), @r###" .pass_stdin("I=42\n"), @r###"
success: false success: false
exit_code: 1 exit_code: 2
----- stdout ----- ----- stdout -----
-:1:1: E741 Ambiguous variable name: `I`
-:1:2: E225 Missing whitespace around operator
Found 2 errors.
----- stderr ----- ----- stderr -----
error: invalid value 'PREVIEW' for '--ignore <RULE_CODE>': Unknown rule selector: `PREVIEW`
For more information, try '--help'.
"###); "###);
} }

View file

@ -588,11 +588,12 @@ impl Configuration {
#[allow(deprecated)] #[allow(deprecated)]
if matches!(selector, RuleSelector::Nursery) { if matches!(selector, RuleSelector::Nursery) {
let suggestion = if preview.is_disabled() { let suggestion = if preview.is_disabled() {
"Use the `--preview` flag instead." " Use the `--preview` flag instead."
} else { } 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() { if preview.is_disabled() {
@ -1092,6 +1093,27 @@ mod tests {
assert_eq!(actual, expected); assert_eq!(actual, expected);
} }
#[test]
fn select_all_preview() {
let actual = resolve_rules(
[RuleSelection {
select: Some(vec![RuleSelector::All]),
..RuleSelection::default()
}],
Some(PreviewMode::Disabled),
);
assert!(!actual.intersects(&RuleSet::from_rules(PREVIEW_RULES)));
let actual = resolve_rules(
[RuleSelection {
select: Some(vec![RuleSelector::All]),
..RuleSelection::default()
}],
Some(PreviewMode::Enabled),
);
assert!(actual.intersects(&RuleSet::from_rules(PREVIEW_RULES)));
}
#[test] #[test]
fn select_linter_preview() { fn select_linter_preview() {
let actual = resolve_rules( let actual = resolve_rules(
@ -1161,31 +1183,6 @@ mod tests {
assert_eq!(actual, expected); assert_eq!(actual, expected);
} }
#[test]
fn select_preview() {
let actual = resolve_rules(
[RuleSelection {
select: Some(vec![RuleSelector::Preview]),
..RuleSelection::default()
}],
Some(PreviewMode::Disabled),
);
let expected = RuleSet::empty();
assert_eq!(actual, expected);
let actual = resolve_rules(
[RuleSelection {
select: Some(vec![RuleSelector::Preview]),
..RuleSelection::default()
}],
Some(PreviewMode::Enabled),
);
let expected =
RuleSet::from_rules(NURSERY_RULES).union(&RuleSet::from_rules(PREVIEW_RULES));
assert_eq!(actual, expected);
}
#[test] #[test]
fn nursery_select_code() { fn nursery_select_code() {
// Backwards compatible behavior allows selection of nursery rules with their exact code // Backwards compatible behavior allows selection of nursery rules with their exact code