Skip to content

Commit

Permalink
Remove the PREVIEW rule selector (#7389)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@zanieb
zanieb authored Sep 14, 2023
1 parent d39eae2 commit b9bb6bf
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 60 deletions.
10 changes: 1 addition & 9 deletions crates/ruff/src/rule_selector.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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),
_ => {
Expand Down Expand Up @@ -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, .. } => {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand Down
39 changes: 11 additions & 28 deletions crates/ruff_cli/tests/integration_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"###);
}

Expand Down Expand Up @@ -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 <RULE_CODE>': Unknown rule selector: `PREVIEW`
For more information, try '--help'.
"###);
}

Expand All @@ -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 <RULE_CODE>': Unknown rule selector: `PREVIEW`
For more information, try '--help'.
"###);
}

Expand Down
43 changes: 20 additions & 23 deletions crates/ruff_workspace/src/configuration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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);
}

Expand Down

0 comments on commit b9bb6bf

Please sign in to comment.