Skip to content

Commit

Permalink
Error if nursery rules are selected without preview (#9683)
Browse files Browse the repository at this point in the history
Extends #9682 to error if the nursery selector is used or nursery rules
are selected without preview.

Part of #7992 — we will remove this in 0.3.0 instead so we can provide
nice errors in 0.2.0.
# Conflicts:
#	crates/ruff/tests/integration_test.rs
#	crates/ruff_workspace/src/configuration.rs
  • Loading branch information
zanieb committed Feb 1, 2024
1 parent 6aa6433 commit a0f32df
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 93 deletions.
27 changes: 13 additions & 14 deletions crates/ruff/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -857,45 +857,44 @@ fn nursery_all() {

#[test]
fn nursery_direct() {
// Should warn that the nursery rule is selected without preview flag but still
// include the diagnostic
// Should fail when a nursery rule is selected without the preview flag
// Before Ruff v0.2.0 this would warn
let mut cmd = RuffCheck::default()
.args(["--select", "RUF912", "--output-format=concise"])
.build();
assert_cmd_snapshot!(cmd, @r###"
success: false
exit_code: 1
exit_code: 2
----- stdout -----
-:1:1: RUF912 Hey this is a nursery test rule.
Found 1 error.
----- stderr -----
warning: Selection of nursery rule `RUF912` without the `--preview` flag is deprecated.
ruff failed
Cause: Selection of unstable rule `RUF912` without the `--preview` flag is not allowed.
"###);
}

#[test]
fn nursery_group_selector() {
// Only nursery rules should be detected e.g. RUF912
// The NURSERY selector is removed but parses in the CLI for a nicer error message
// Before Ruff v0.2.0 this would warn
let mut cmd = RuffCheck::default()
.args(["--select", "NURSERY", "--output-format=concise"])
.build();
assert_cmd_snapshot!(cmd, @r###"
success: false
exit_code: 1
exit_code: 2
----- stdout -----
-:1:1: CPY001 Missing copyright notice at top of file
-:1:1: RUF912 Hey this is a nursery test rule.
Found 2 errors.
----- stderr -----
warning: The `NURSERY` selector has been deprecated. Use the `--preview` flag instead.
ruff failed
Cause: The `NURSERY` selector was removed. Use the `--preview` flag instead.
"###);
}

#[test]
fn nursery_group_selector_preview_enabled() {
// A warning should be displayed due to deprecated selector usage
// When preview mode is enabled, we shouldn't suggest using the `--preview` flag.
// Before Ruff v0.2.0 this would warn
let mut cmd = RuffCheck::default()
.args(["--select", "NURSERY", "--preview"])
.build();
Expand All @@ -906,7 +905,7 @@ fn nursery_group_selector_preview_enabled() {
----- stderr -----
ruff failed
Cause: The `NURSERY` selector is deprecated and cannot be used with preview mode enabled.
Cause: The `NURSERY` selector was removed. Unstable rules should be selected individually or by their respective groups.
"###);
}

Expand Down
103 changes: 33 additions & 70 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -887,10 +887,12 @@ impl LintConfiguration {
for (kind, selector) in selection.selectors_by_kind() {
#[allow(deprecated)]
if matches!(selector, RuleSelector::Nursery) {
if preview.mode.is_enabled() {
return Err(anyhow!("The `NURSERY` selector is deprecated and cannot be used with preview mode enabled."));
}
warn_user_once!("The `NURSERY` selector has been deprecated. Use the `--preview` flag instead.");
let suggestion = if preview.mode.is_disabled() {
" Use the `--preview` flag instead."
} else {
" Unstable rules should be selected individually or by their respective groups."
};
return Err(anyhow!("The `NURSERY` selector was removed.{suggestion}"));
};

// Only warn for the following selectors if used to enable rules
Expand Down Expand Up @@ -928,9 +930,24 @@ impl LintConfiguration {
);
}

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.",);
let deprecated_nursery_selectors = deprecated_nursery_selectors.iter().collect::<Vec<_>>();
match deprecated_nursery_selectors.as_slice() {
[] => (),
[selection] => {
let (prefix, code) = selection.prefix_and_code();
return Err(anyhow!("Selection of unstable rule `{prefix}{code}` without the `--preview` flag is not allowed."));
}
[..] => {
let mut message = "Selection of unstable rules without the `--preview` flag is not allowed. Enable preview or remove selection of:".to_string();
for selection in deprecated_nursery_selectors {
let (prefix, code) = selection.prefix_and_code();
message.push_str("\n\t- ");
message.push_str(prefix);
message.push_str(code);
}
message.push('\n');
return Err(anyhow!(message));
}
}

for selection in ignored_preview_selectors {
Expand Down Expand Up @@ -1363,52 +1380,6 @@ mod tests {
use ruff_linter::RuleSelector;
use std::str::FromStr;

const NURSERY_RULES: &[Rule] = &[
Rule::MissingCopyrightNotice,
Rule::IndentationWithInvalidMultiple,
Rule::NoIndentedBlock,
Rule::UnexpectedIndentation,
Rule::IndentationWithInvalidMultipleComment,
Rule::NoIndentedBlockComment,
Rule::UnexpectedIndentationComment,
Rule::OverIndented,
Rule::WhitespaceAfterOpenBracket,
Rule::WhitespaceBeforeCloseBracket,
Rule::WhitespaceBeforePunctuation,
Rule::WhitespaceBeforeParameters,
Rule::MultipleSpacesBeforeOperator,
Rule::MultipleSpacesAfterOperator,
Rule::TabBeforeOperator,
Rule::TabAfterOperator,
Rule::MissingWhitespaceAroundOperator,
Rule::MissingWhitespaceAroundArithmeticOperator,
Rule::MissingWhitespaceAroundBitwiseOrShiftOperator,
Rule::MissingWhitespaceAroundModuloOperator,
Rule::MissingWhitespace,
Rule::MultipleSpacesAfterComma,
Rule::TabAfterComma,
Rule::UnexpectedSpacesAroundKeywordParameterEquals,
Rule::MissingWhitespaceAroundParameterEquals,
Rule::TooFewSpacesBeforeInlineComment,
Rule::NoSpaceAfterInlineComment,
Rule::NoSpaceAfterBlockComment,
Rule::MultipleLeadingHashesForBlockComment,
Rule::MultipleSpacesAfterKeyword,
Rule::MultipleSpacesBeforeKeyword,
Rule::TabAfterKeyword,
Rule::TabBeforeKeyword,
Rule::MissingWhitespaceAfterKeyword,
Rule::CompareToEmptyString,
Rule::NoSelfUse,
Rule::EqWithoutHash,
Rule::BadDunderMethodName,
Rule::RepeatedAppend,
Rule::DeleteFullSlice,
Rule::CheckAndRemoveFromSet,
Rule::QuadraticListSummation,
Rule::NurseryTestRule,
];

const PREVIEW_RULES: &[Rule] = &[
Rule::AndOrTernary,
Rule::AssignmentInAssert,
Expand Down Expand Up @@ -1811,9 +1782,8 @@ mod tests {

#[test]
fn nursery_select_code() -> Result<()> {
// Backwards compatible behavior allows selection of nursery rules with their exact code
// when preview is disabled
let actual = resolve_rules(
// We do not allow selection of nursery rules when preview is disabled
assert!(resolve_rules(
[RuleSelection {
select: Some(vec![Flake8Copyright::_001.into()]),
..RuleSelection::default()
Expand All @@ -1822,9 +1792,8 @@ mod tests {
mode: PreviewMode::Disabled,
..PreviewOptions::default()
}),
)?;
let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice);
assert_eq!(actual, expected);
)
.is_err());

let actual = resolve_rules(
[RuleSelection {
Expand All @@ -1843,10 +1812,9 @@ mod tests {

#[test]
#[allow(deprecated)]
fn select_nursery() -> Result<()> {
// Backwards compatible behavior allows selection of nursery rules with the nursery selector
// when preview is disabled
let actual = resolve_rules(
fn select_nursery() {
// We no longer allow use of the NURSERY selector and should error in both cases
assert!(resolve_rules(
[RuleSelection {
select: Some(vec![RuleSelector::Nursery]),
..RuleSelection::default()
Expand All @@ -1855,11 +1823,8 @@ mod tests {
mode: PreviewMode::Disabled,
..PreviewOptions::default()
}),
)?;
let expected = RuleSet::from_rules(NURSERY_RULES);
assert_eq!(actual, expected);

// When preview is enabled, use of NURSERY is banned
)
.is_err());
assert!(resolve_rules(
[RuleSelection {
select: Some(vec![RuleSelector::Nursery]),
Expand All @@ -1871,8 +1836,6 @@ mod tests {
}),
)
.is_err());

Ok(())
}

#[test]
Expand Down
9 changes: 0 additions & 9 deletions docs/preview.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,3 @@ In our previous example, `--select` with `ALL` `HYP`, `HYP0`, or `HYP00` would n
rule will need to be selected with its exact code, e.g. `--select ALL,HYP001`.

If preview mode is not enabled, this setting has no effect.

## Legacy behavior

Before the preview mode was introduced, new rules were added in a "nursery" category that required selection of
rules with their exact codes — similar to if `explicit-preview-rules` is enabled.

The nursery category has been deprecated and all rules in the nursery are now considered to be in preview.
For backwards compatibility, nursery rules are selectable with their exact codes without enabling preview mode.
However, this behavior will display a warning and support will be removed in a future release.

0 comments on commit a0f32df

Please sign in to comment.