From a0f32dfa55ef5e64f549f59a916176b122a68204 Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 1 Feb 2024 08:55:55 -0600 Subject: [PATCH] Error if nursery rules are selected without preview (#9683) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- crates/ruff/tests/integration_test.rs | 27 +++--- crates/ruff_workspace/src/configuration.rs | 103 +++++++-------------- docs/preview.md | 9 -- 3 files changed, 46 insertions(+), 93 deletions(-) diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs index 9fab27356b..18e4a66584 100644 --- a/crates/ruff/tests/integration_test.rs +++ b/crates/ruff/tests/integration_test.rs @@ -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(); @@ -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. "###); } diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 4abb3f0c49..6a9a8093cd 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -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 @@ -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::>(); + 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 { @@ -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, @@ -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() @@ -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 { @@ -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() @@ -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]), @@ -1871,8 +1836,6 @@ mod tests { }), ) .is_err()); - - Ok(()) } #[test] diff --git a/docs/preview.md b/docs/preview.md index 1bea9e451d..048fcbe9b6 100644 --- a/docs/preview.md +++ b/docs/preview.md @@ -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.