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
This commit is contained in:
Zanie Blue 2024-01-29 13:33:46 -06:00 committed by GitHub
parent 05a2f52206
commit 4ccbacd44b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 89 additions and 69 deletions

View file

@ -878,15 +878,12 @@ fn nursery_group_selector_preview_enabled() {
assert_cmd_snapshot!(cmd assert_cmd_snapshot!(cmd
.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.
[*] 1 fixable with the `--fix` option.
----- stderr ----- ----- stderr -----
warning: The `NURSERY` selector has been deprecated. ruff failed
Cause: The `NURSERY` selector is deprecated and cannot be used with preview mode enabled.
"###); "###);
} }

View file

@ -239,7 +239,7 @@ impl Configuration {
}, },
linter: LinterSettings { 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())?, exclude: FilePatternSet::try_from_iter(lint.exclude.unwrap_or_default())?,
extension: self.extension.unwrap_or_default(), extension: self.extension.unwrap_or_default(),
preview: lint_preview, 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<RuleTable> {
let preview = PreviewOptions { let preview = PreviewOptions {
mode: preview, mode: preview,
require_explicit: self.explicit_preview_rules.unwrap_or_default(), require_explicit: self.explicit_preview_rules.unwrap_or_default(),
@ -860,13 +860,10 @@ impl LintConfiguration {
for (kind, selector) in selection.selectors_by_kind() { for (kind, selector) in selection.selectors_by_kind() {
#[allow(deprecated)] #[allow(deprecated)]
if matches!(selector, RuleSelector::Nursery) { if matches!(selector, RuleSelector::Nursery) {
let suggestion = if preview.mode.is_disabled() { if preview.mode.is_enabled() {
" Use the `--preview` flag instead." return Err(anyhow!("The `NURSERY` selector is deprecated and cannot be used with preview mode enabled."));
} else { }
// We have no suggested alternative since there is intentionally no "PREVIEW" selector warn_user_once!("The `NURSERY` selector has been deprecated. Use the `--preview` flag instead.");
""
};
warn_user_once!("The `NURSERY` selector has been deprecated.{suggestion}");
}; };
// Only warn for the following selectors if used to enable rules // Only warn for the following selectors if used to enable rules
@ -946,7 +943,7 @@ impl LintConfiguration {
} }
} }
rules Ok(rules)
} }
#[must_use] #[must_use]
@ -1136,6 +1133,7 @@ pub fn resolve_src(src: &[String], project_root: &Path) -> Result<Vec<PathBuf>>
mod tests { mod tests {
use crate::configuration::{LintConfiguration, RuleSelection}; use crate::configuration::{LintConfiguration, RuleSelection};
use crate::options::PydocstyleOptions; use crate::options::PydocstyleOptions;
use anyhow::Result;
use ruff_linter::codes::{Flake8Copyright, Pycodestyle, Refurb}; use ruff_linter::codes::{Flake8Copyright, Pycodestyle, Refurb};
use ruff_linter::registry::{Linter, Rule, RuleSet}; use ruff_linter::registry::{Linter, Rule, RuleSet};
use ruff_linter::rule_selector::PreviewOptions; use ruff_linter::rule_selector::PreviewOptions;
@ -1209,26 +1207,26 @@ mod tests {
fn resolve_rules( fn resolve_rules(
selections: impl IntoIterator<Item = RuleSelection>, selections: impl IntoIterator<Item = RuleSelection>,
preview: Option<PreviewOptions>, preview: Option<PreviewOptions>,
) -> RuleSet { ) -> Result<RuleSet> {
LintConfiguration { Ok(LintConfiguration {
rule_selections: selections.into_iter().collect(), rule_selections: selections.into_iter().collect(),
explicit_preview_rules: preview.as_ref().map(|preview| preview.require_explicit), explicit_preview_rules: preview.as_ref().map(|preview| preview.require_explicit),
..LintConfiguration::default() ..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() .iter_enabled()
.collect() .collect())
} }
#[test] #[test]
fn select_linter() { fn select_linter() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![Linter::Pycodestyle.into()]), select: Some(vec![Linter::Pycodestyle.into()]),
..RuleSelection::default() ..RuleSelection::default()
}], }],
None, None,
); )?;
let expected = RuleSet::from_rules(&[ let expected = RuleSet::from_rules(&[
Rule::MixedSpacesAndTabs, Rule::MixedSpacesAndTabs,
@ -1258,17 +1256,19 @@ mod tests {
Rule::InvalidEscapeSequence, Rule::InvalidEscapeSequence,
]); ]);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_one_char_prefix() { fn select_one_char_prefix() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![Pycodestyle::W.into()]), select: Some(vec![Pycodestyle::W.into()]),
..RuleSelection::default() ..RuleSelection::default()
}], }],
None, None,
); )?;
let expected = RuleSet::from_rules(&[ let expected = RuleSet::from_rules(&[
Rule::TrailingWhitespace, Rule::TrailingWhitespace,
@ -1279,23 +1279,25 @@ mod tests {
Rule::TabIndentation, Rule::TabIndentation,
]); ]);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_two_char_prefix() { fn select_two_char_prefix() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![Pycodestyle::W6.into()]), select: Some(vec![Pycodestyle::W6.into()]),
..RuleSelection::default() ..RuleSelection::default()
}], }],
None, None,
); )?;
let expected = RuleSet::from_rule(Rule::InvalidEscapeSequence); let expected = RuleSet::from_rule(Rule::InvalidEscapeSequence);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_prefix_ignore_code() { fn select_prefix_ignore_code() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![Pycodestyle::W.into()]), select: Some(vec![Pycodestyle::W.into()]),
@ -1303,7 +1305,7 @@ mod tests {
..RuleSelection::default() ..RuleSelection::default()
}], }],
None, None,
); )?;
let expected = RuleSet::from_rules(&[ let expected = RuleSet::from_rules(&[
Rule::TrailingWhitespace, Rule::TrailingWhitespace,
Rule::BlankLineWithWhitespace, Rule::BlankLineWithWhitespace,
@ -1312,10 +1314,11 @@ mod tests {
Rule::TabIndentation, Rule::TabIndentation,
]); ]);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_code_ignore_prefix() { fn select_code_ignore_prefix() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![Pycodestyle::W292.into()]), select: Some(vec![Pycodestyle::W292.into()]),
@ -1323,13 +1326,14 @@ mod tests {
..RuleSelection::default() ..RuleSelection::default()
}], }],
None, None,
); )?;
let expected = RuleSet::from_rule(Rule::MissingNewlineAtEndOfFile); let expected = RuleSet::from_rule(Rule::MissingNewlineAtEndOfFile);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_code_ignore_code() { fn select_code_ignore_code() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![Pycodestyle::W605.into()]), select: Some(vec![Pycodestyle::W605.into()]),
@ -1337,13 +1341,14 @@ mod tests {
..RuleSelection::default() ..RuleSelection::default()
}], }],
None, None,
); )?;
let expected = RuleSet::empty(); let expected = RuleSet::empty();
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_prefix_ignore_code_then_extend_select_code() { fn select_prefix_ignore_code_then_extend_select_code() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[ [
RuleSelection { RuleSelection {
@ -1357,7 +1362,7 @@ mod tests {
}, },
], ],
None, None,
); )?;
let expected = RuleSet::from_rules(&[ let expected = RuleSet::from_rules(&[
Rule::TrailingWhitespace, Rule::TrailingWhitespace,
Rule::MissingNewlineAtEndOfFile, Rule::MissingNewlineAtEndOfFile,
@ -1367,10 +1372,11 @@ mod tests {
Rule::TabIndentation, Rule::TabIndentation,
]); ]);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[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( let actual = resolve_rules(
[ [
RuleSelection { RuleSelection {
@ -1385,13 +1391,14 @@ mod tests {
}, },
], ],
None, None,
); )?;
let expected = RuleSet::from_rule(Rule::MissingNewlineAtEndOfFile); let expected = RuleSet::from_rule(Rule::MissingNewlineAtEndOfFile);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn ignore_code_then_select_prefix() { fn ignore_code_then_select_prefix() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[ [
RuleSelection { RuleSelection {
@ -1405,7 +1412,7 @@ mod tests {
}, },
], ],
None, None,
); )?;
let expected = RuleSet::from_rules(&[ let expected = RuleSet::from_rules(&[
Rule::TrailingWhitespace, Rule::TrailingWhitespace,
Rule::BlankLineWithWhitespace, Rule::BlankLineWithWhitespace,
@ -1414,10 +1421,11 @@ mod tests {
Rule::TabIndentation, Rule::TabIndentation,
]); ]);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn ignore_code_then_select_prefix_ignore_code() { fn ignore_code_then_select_prefix_ignore_code() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[ [
RuleSelection { RuleSelection {
@ -1432,7 +1440,7 @@ mod tests {
}, },
], ],
None, None,
); )?;
let expected = RuleSet::from_rules(&[ let expected = RuleSet::from_rules(&[
Rule::TrailingWhitespace, Rule::TrailingWhitespace,
Rule::BlankLineWithWhitespace, Rule::BlankLineWithWhitespace,
@ -1440,10 +1448,11 @@ mod tests {
Rule::TabIndentation, Rule::TabIndentation,
]); ]);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_all_preview() { fn select_all_preview() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![RuleSelector::All]), select: Some(vec![RuleSelector::All]),
@ -1453,7 +1462,7 @@ mod tests {
mode: PreviewMode::Disabled, mode: PreviewMode::Disabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
assert!(!actual.intersects(&RuleSet::from_rules(PREVIEW_RULES))); assert!(!actual.intersects(&RuleSet::from_rules(PREVIEW_RULES)));
let actual = resolve_rules( let actual = resolve_rules(
@ -1465,12 +1474,14 @@ mod tests {
mode: PreviewMode::Enabled, mode: PreviewMode::Enabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
assert!(actual.intersects(&RuleSet::from_rules(PREVIEW_RULES))); assert!(actual.intersects(&RuleSet::from_rules(PREVIEW_RULES)));
Ok(())
} }
#[test] #[test]
fn select_linter_preview() { fn select_linter_preview() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![Linter::Flake8Copyright.into()]), select: Some(vec![Linter::Flake8Copyright.into()]),
@ -1480,7 +1491,7 @@ mod tests {
mode: PreviewMode::Disabled, mode: PreviewMode::Disabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
let expected = RuleSet::empty(); let expected = RuleSet::empty();
assert_eq!(actual, expected); assert_eq!(actual, expected);
@ -1493,13 +1504,14 @@ mod tests {
mode: PreviewMode::Enabled, mode: PreviewMode::Enabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_prefix_preview() { fn select_prefix_preview() -> Result<()> {
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![Flake8Copyright::_0.into()]), select: Some(vec![Flake8Copyright::_0.into()]),
@ -1509,7 +1521,7 @@ mod tests {
mode: PreviewMode::Disabled, mode: PreviewMode::Disabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
let expected = RuleSet::empty(); let expected = RuleSet::empty();
assert_eq!(actual, expected); assert_eq!(actual, expected);
@ -1522,13 +1534,14 @@ mod tests {
mode: PreviewMode::Enabled, mode: PreviewMode::Enabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_rule_preview() { fn select_rule_preview() -> Result<()> {
// Test inclusion when toggling preview on and off // Test inclusion when toggling preview on and off
let actual = resolve_rules( let actual = resolve_rules(
[RuleSelection { [RuleSelection {
@ -1539,7 +1552,7 @@ mod tests {
mode: PreviewMode::Disabled, mode: PreviewMode::Disabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
let expected = RuleSet::empty(); let expected = RuleSet::empty();
assert_eq!(actual, expected); assert_eq!(actual, expected);
@ -1552,7 +1565,7 @@ mod tests {
mode: PreviewMode::Enabled, mode: PreviewMode::Enabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
let expected = RuleSet::from_rule(Rule::SliceCopy); let expected = RuleSet::from_rule(Rule::SliceCopy);
assert_eq!(actual, expected); assert_eq!(actual, expected);
@ -1566,13 +1579,14 @@ mod tests {
mode: PreviewMode::Enabled, mode: PreviewMode::Enabled,
require_explicit: true, require_explicit: true,
}), }),
); )?;
let expected = RuleSet::from_rule(Rule::SliceCopy); let expected = RuleSet::from_rule(Rule::SliceCopy);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn nursery_select_code() { fn nursery_select_code() -> Result<()> {
// Backwards compatible behavior allows selection of nursery rules with their exact code // Backwards compatible behavior allows selection of nursery rules with their exact code
// when preview is disabled // when preview is disabled
let actual = resolve_rules( let actual = resolve_rules(
@ -1584,7 +1598,7 @@ mod tests {
mode: PreviewMode::Disabled, mode: PreviewMode::Disabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice);
assert_eq!(actual, expected); assert_eq!(actual, expected);
@ -1597,14 +1611,15 @@ mod tests {
mode: PreviewMode::Enabled, mode: PreviewMode::Enabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice); let expected = RuleSet::from_rule(Rule::MissingCopyrightNotice);
assert_eq!(actual, expected); assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
#[allow(deprecated)] #[allow(deprecated)]
fn select_nursery() { fn select_nursery() -> Result<()> {
// Backwards compatible behavior allows selection of nursery rules with the nursery selector // Backwards compatible behavior allows selection of nursery rules with the nursery selector
// when preview is disabled // when preview is disabled
let actual = resolve_rules( let actual = resolve_rules(
@ -1616,11 +1631,12 @@ mod tests {
mode: PreviewMode::Disabled, mode: PreviewMode::Disabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )?;
let expected = RuleSet::from_rules(NURSERY_RULES); let expected = RuleSet::from_rules(NURSERY_RULES);
assert_eq!(actual, expected); assert_eq!(actual, expected);
let actual = resolve_rules( // When preview is enabled, use of NURSERY is banned
assert!(resolve_rules(
[RuleSelection { [RuleSelection {
select: Some(vec![RuleSelector::Nursery]), select: Some(vec![RuleSelector::Nursery]),
..RuleSelection::default() ..RuleSelection::default()
@ -1629,14 +1645,18 @@ mod tests {
mode: PreviewMode::Enabled, mode: PreviewMode::Enabled,
..PreviewOptions::default() ..PreviewOptions::default()
}), }),
); )
let expected = RuleSet::from_rules(NURSERY_RULES); .is_err());
assert_eq!(actual, expected);
Ok(())
} }
#[test] #[test]
fn select_docstring_convention_override() { fn select_docstring_convention_override() -> Result<()> {
fn assert_override(rule_selections: Vec<RuleSelection>, should_be_overridden: bool) { fn assert_override(
rule_selections: Vec<RuleSelection>,
should_be_overridden: bool,
) -> Result<()> {
use ruff_linter::rules::pydocstyle::settings::Convention; use ruff_linter::rules::pydocstyle::settings::Convention;
let config = LintConfiguration { let config = LintConfiguration {
@ -1661,11 +1681,12 @@ mod tests {
} }
assert_eq!( assert_eq!(
config config
.as_rule_table(PreviewMode::Disabled) .as_rule_table(PreviewMode::Disabled)?
.iter_enabled() .iter_enabled()
.collect::<RuleSet>(), .collect::<RuleSet>(),
expected, expected,
); );
Ok(())
} }
let d41 = RuleSelector::from_str("D41").unwrap(); let d41 = RuleSelector::from_str("D41").unwrap();
@ -1678,7 +1699,7 @@ mod tests {
..RuleSelection::default() ..RuleSelection::default()
}], }],
false, false,
); )?;
// ...but does appear when specified directly. // ...but does appear when specified directly.
assert_override( assert_override(
@ -1687,7 +1708,7 @@ mod tests {
..RuleSelection::default() ..RuleSelection::default()
}], }],
true, true,
); )?;
// ...but disappears if there's a subsequent `--select`. // ...but disappears if there's a subsequent `--select`.
assert_override( assert_override(
@ -1702,7 +1723,7 @@ mod tests {
}, },
], ],
false, false,
); )?;
// ...although an `--extend-select` is fine. // ...although an `--extend-select` is fine.
assert_override( assert_override(
@ -1717,6 +1738,8 @@ mod tests {
}, },
], ],
true, true,
); )?;
Ok(())
} }
} }