From 4ccbacd44b79ad7277b69b253cad3fb689f3307f Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Mon, 29 Jan 2024 13:33:46 -0600 Subject: [PATCH] Error if the NURSERY selector is used with preview (#9682) Changes our warning for combined use of `--preview` and `--select NURSERY` to a hard error. This should go out _before_ #9680 where we will ban use of `NURSERY` outside of preview as well (see #9683). Part of https://github.com/astral-sh/ruff/issues/7992 --- crates/ruff/tests/integration_test.rs | 9 +- crates/ruff_workspace/src/configuration.rs | 149 ++++++++++++--------- 2 files changed, 89 insertions(+), 69 deletions(-) diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs index 8c4c346d5b189..7aed76fee2df3 100644 --- a/crates/ruff/tests/integration_test.rs +++ b/crates/ruff/tests/integration_test.rs @@ -878,15 +878,12 @@ fn nursery_group_selector_preview_enabled() { assert_cmd_snapshot!(cmd .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. - [*] 1 fixable with the `--fix` option. ----- stderr ----- - warning: The `NURSERY` selector has been deprecated. + ruff failed + Cause: The `NURSERY` selector is deprecated and cannot be used with preview mode enabled. "###); } diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 18e7551f556de..f759febfccb7a 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -239,7 +239,7 @@ impl Configuration { }, linter: LinterSettings { - rules: lint.as_rule_table(lint_preview), + rules: lint.as_rule_table(lint_preview)?, exclude: FilePatternSet::try_from_iter(lint.exclude.unwrap_or_default())?, extension: self.extension.unwrap_or_default(), preview: lint_preview, @@ -701,7 +701,7 @@ impl LintConfiguration { }) } - fn as_rule_table(&self, preview: PreviewMode) -> RuleTable { + fn as_rule_table(&self, preview: PreviewMode) -> Result { let preview = PreviewOptions { mode: preview, require_explicit: self.explicit_preview_rules.unwrap_or_default(), @@ -860,13 +860,10 @@ impl LintConfiguration { for (kind, selector) in selection.selectors_by_kind() { #[allow(deprecated)] if matches!(selector, RuleSelector::Nursery) { - let suggestion = if preview.mode.is_disabled() { - " Use the `--preview` flag instead." - } else { - // We have no suggested alternative since there is intentionally no "PREVIEW" selector - "" - }; - warn_user_once!("The `NURSERY` selector has been deprecated.{suggestion}"); + 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."); }; // Only warn for the following selectors if used to enable rules @@ -946,7 +943,7 @@ impl LintConfiguration { } } - rules + Ok(rules) } #[must_use] @@ -1136,6 +1133,7 @@ pub fn resolve_src(src: &[String], project_root: &Path) -> Result> mod tests { use crate::configuration::{LintConfiguration, RuleSelection}; use crate::options::PydocstyleOptions; + use anyhow::Result; use ruff_linter::codes::{Flake8Copyright, Pycodestyle, Refurb}; use ruff_linter::registry::{Linter, Rule, RuleSet}; use ruff_linter::rule_selector::PreviewOptions; @@ -1209,26 +1207,26 @@ mod tests { fn resolve_rules( selections: impl IntoIterator, preview: Option, - ) -> RuleSet { - LintConfiguration { + ) -> Result { + Ok(LintConfiguration { rule_selections: selections.into_iter().collect(), explicit_preview_rules: preview.as_ref().map(|preview| preview.require_explicit), ..LintConfiguration::default() } - .as_rule_table(preview.map(|preview| preview.mode).unwrap_or_default()) + .as_rule_table(preview.map(|preview| preview.mode).unwrap_or_default())? .iter_enabled() - .collect() + .collect()) } #[test] - fn select_linter() { + fn select_linter() -> Result<()> { let actual = resolve_rules( [RuleSelection { select: Some(vec![Linter::Pycodestyle.into()]), ..RuleSelection::default() }], None, - ); + )?; let expected = RuleSet::from_rules(&[ Rule::MixedSpacesAndTabs, @@ -1258,17 +1256,19 @@ mod tests { Rule::InvalidEscapeSequence, ]); assert_eq!(actual, expected); + + Ok(()) } #[test] - fn select_one_char_prefix() { + fn select_one_char_prefix() -> Result<()> { let actual = resolve_rules( [RuleSelection { select: Some(vec![Pycodestyle::W.into()]), ..RuleSelection::default() }], None, - ); + )?; let expected = RuleSet::from_rules(&[ Rule::TrailingWhitespace, @@ -1279,23 +1279,25 @@ mod tests { Rule::TabIndentation, ]); assert_eq!(actual, expected); + Ok(()) } #[test] - fn select_two_char_prefix() { + fn select_two_char_prefix() -> Result<()> { let actual = resolve_rules( [RuleSelection { select: Some(vec![Pycodestyle::W6.into()]), ..RuleSelection::default() }], None, - ); + )?; let expected = RuleSet::from_rule(Rule::InvalidEscapeSequence); assert_eq!(actual, expected); + Ok(()) } #[test] - fn select_prefix_ignore_code() { + fn select_prefix_ignore_code() -> Result<()> { let actual = resolve_rules( [RuleSelection { select: Some(vec![Pycodestyle::W.into()]), @@ -1303,7 +1305,7 @@ mod tests { ..RuleSelection::default() }], None, - ); + )?; let expected = RuleSet::from_rules(&[ Rule::TrailingWhitespace, Rule::BlankLineWithWhitespace, @@ -1312,10 +1314,11 @@ mod tests { Rule::TabIndentation, ]); assert_eq!(actual, expected); + Ok(()) } #[test] - fn select_code_ignore_prefix() { + fn select_code_ignore_prefix() -> Result<()> { let actual = resolve_rules( [RuleSelection { select: Some(vec![Pycodestyle::W292.into()]), @@ -1323,13 +1326,14 @@ mod tests { ..RuleSelection::default() }], None, - ); + )?; let expected = RuleSet::from_rule(Rule::MissingNewlineAtEndOfFile); assert_eq!(actual, expected); + Ok(()) } #[test] - fn select_code_ignore_code() { + fn select_code_ignore_code() -> Result<()> { let actual = resolve_rules( [RuleSelection { select: Some(vec![Pycodestyle::W605.into()]), @@ -1337,13 +1341,14 @@ mod tests { ..RuleSelection::default() }], None, - ); + )?; let expected = RuleSet::empty(); assert_eq!(actual, expected); + Ok(()) } #[test] - fn select_prefix_ignore_code_then_extend_select_code() { + fn select_prefix_ignore_code_then_extend_select_code() -> Result<()> { let actual = resolve_rules( [ RuleSelection { @@ -1357,7 +1362,7 @@ mod tests { }, ], None, - ); + )?; let expected = RuleSet::from_rules(&[ Rule::TrailingWhitespace, Rule::MissingNewlineAtEndOfFile, @@ -1367,10 +1372,11 @@ mod tests { Rule::TabIndentation, ]); assert_eq!(actual, expected); + Ok(()) } #[test] - fn select_prefix_ignore_code_then_extend_select_code_ignore_prefix() { + fn select_prefix_ignore_code_then_extend_select_code_ignore_prefix() -> Result<()> { let actual = resolve_rules( [ RuleSelection { @@ -1385,13 +1391,14 @@ mod tests { }, ], None, - ); + )?; let expected = RuleSet::from_rule(Rule::MissingNewlineAtEndOfFile); assert_eq!(actual, expected); + Ok(()) } #[test] - fn ignore_code_then_select_prefix() { + fn ignore_code_then_select_prefix() -> Result<()> { let actual = resolve_rules( [ RuleSelection { @@ -1405,7 +1412,7 @@ mod tests { }, ], None, - ); + )?; let expected = RuleSet::from_rules(&[ Rule::TrailingWhitespace, Rule::BlankLineWithWhitespace, @@ -1414,10 +1421,11 @@ mod tests { Rule::TabIndentation, ]); assert_eq!(actual, expected); + Ok(()) } #[test] - fn ignore_code_then_select_prefix_ignore_code() { + fn ignore_code_then_select_prefix_ignore_code() -> Result<()> { let actual = resolve_rules( [ RuleSelection { @@ -1432,7 +1440,7 @@ mod tests { }, ], None, - ); + )?; let expected = RuleSet::from_rules(&[ Rule::TrailingWhitespace, Rule::BlankLineWithWhitespace, @@ -1440,10 +1448,11 @@ mod tests { Rule::TabIndentation, ]); assert_eq!(actual, expected); + Ok(()) } #[test] - fn select_all_preview() { + fn select_all_preview() -> Result<()> { let actual = resolve_rules( [RuleSelection { select: Some(vec![RuleSelector::All]), @@ -1453,7 +1462,7 @@ mod tests { mode: PreviewMode::Disabled, ..PreviewOptions::default() }), - ); + )?; assert!(!actual.intersects(&RuleSet::from_rules(PREVIEW_RULES))); let actual = resolve_rules( @@ -1465,12 +1474,14 @@ mod tests { mode: PreviewMode::Enabled, ..PreviewOptions::default() }), - ); + )?; assert!(actual.intersects(&RuleSet::from_rules(PREVIEW_RULES))); + + Ok(()) } #[test] - fn select_linter_preview() { + fn select_linter_preview() -> Result<()> { let actual = resolve_rules( [RuleSelection { select: Some(vec![Linter::Flake8Copyright.into()]), @@ -1480,7 +1491,7 @@ mod tests { mode: PreviewMode::Disabled, ..PreviewOptions::default() }), - ); + )?; let expected = RuleSet::empty(); assert_eq!(actual, expected); @@ -1493,13 +1504,14 @@ mod tests { mode: PreviewMode::Enabled, ..PreviewOptions::default() }), - ); + )?; let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); assert_eq!(actual, expected); + Ok(()) } #[test] - fn select_prefix_preview() { + fn select_prefix_preview() -> Result<()> { let actual = resolve_rules( [RuleSelection { select: Some(vec![Flake8Copyright::_0.into()]), @@ -1509,7 +1521,7 @@ mod tests { mode: PreviewMode::Disabled, ..PreviewOptions::default() }), - ); + )?; let expected = RuleSet::empty(); assert_eq!(actual, expected); @@ -1522,13 +1534,14 @@ mod tests { mode: PreviewMode::Enabled, ..PreviewOptions::default() }), - ); + )?; let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); assert_eq!(actual, expected); + Ok(()) } #[test] - fn select_rule_preview() { + fn select_rule_preview() -> Result<()> { // Test inclusion when toggling preview on and off let actual = resolve_rules( [RuleSelection { @@ -1539,7 +1552,7 @@ mod tests { mode: PreviewMode::Disabled, ..PreviewOptions::default() }), - ); + )?; let expected = RuleSet::empty(); assert_eq!(actual, expected); @@ -1552,7 +1565,7 @@ mod tests { mode: PreviewMode::Enabled, ..PreviewOptions::default() }), - ); + )?; let expected = RuleSet::from_rule(Rule::SliceCopy); assert_eq!(actual, expected); @@ -1566,13 +1579,14 @@ mod tests { mode: PreviewMode::Enabled, require_explicit: true, }), - ); + )?; let expected = RuleSet::from_rule(Rule::SliceCopy); assert_eq!(actual, expected); + Ok(()) } #[test] - fn nursery_select_code() { + 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( @@ -1584,7 +1598,7 @@ mod tests { mode: PreviewMode::Disabled, ..PreviewOptions::default() }), - ); + )?; let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); assert_eq!(actual, expected); @@ -1597,14 +1611,15 @@ mod tests { mode: PreviewMode::Enabled, ..PreviewOptions::default() }), - ); + )?; let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); assert_eq!(actual, expected); + Ok(()) } #[test] #[allow(deprecated)] - fn select_nursery() { + fn select_nursery() -> Result<()> { // Backwards compatible behavior allows selection of nursery rules with the nursery selector // when preview is disabled let actual = resolve_rules( @@ -1616,11 +1631,12 @@ mod tests { mode: PreviewMode::Disabled, ..PreviewOptions::default() }), - ); + )?; let expected = RuleSet::from_rules(NURSERY_RULES); assert_eq!(actual, expected); - let actual = resolve_rules( + // When preview is enabled, use of NURSERY is banned + assert!(resolve_rules( [RuleSelection { select: Some(vec![RuleSelector::Nursery]), ..RuleSelection::default() @@ -1629,14 +1645,18 @@ mod tests { mode: PreviewMode::Enabled, ..PreviewOptions::default() }), - ); - let expected = RuleSet::from_rules(NURSERY_RULES); - assert_eq!(actual, expected); + ) + .is_err()); + + Ok(()) } #[test] - fn select_docstring_convention_override() { - fn assert_override(rule_selections: Vec, should_be_overridden: bool) { + fn select_docstring_convention_override() -> Result<()> { + fn assert_override( + rule_selections: Vec, + should_be_overridden: bool, + ) -> Result<()> { use ruff_linter::rules::pydocstyle::settings::Convention; let config = LintConfiguration { @@ -1661,11 +1681,12 @@ mod tests { } assert_eq!( config - .as_rule_table(PreviewMode::Disabled) + .as_rule_table(PreviewMode::Disabled)? .iter_enabled() .collect::(), expected, ); + Ok(()) } let d41 = RuleSelector::from_str("D41").unwrap(); @@ -1678,7 +1699,7 @@ mod tests { ..RuleSelection::default() }], false, - ); + )?; // ...but does appear when specified directly. assert_override( @@ -1687,7 +1708,7 @@ mod tests { ..RuleSelection::default() }], true, - ); + )?; // ...but disappears if there's a subsequent `--select`. assert_override( @@ -1702,7 +1723,7 @@ mod tests { }, ], false, - ); + )?; // ...although an `--extend-select` is fine. assert_override( @@ -1717,6 +1738,8 @@ mod tests { }, ], true, - ); + )?; + + Ok(()) } }