Only show warnings for empty preview selectors when enabling rules (#7842)

Closes https://github.com/astral-sh/ruff/issues/7491

Users found it confusing that warnings were displayed when ignoring a
preview rule (which has no effect without `--preview`). While we could
retain the warning with different messaging, I've opted to remove it for
now. With this pull request, we will only warn on `--select` and
`--extend-select` but not `--fixable`, `--unfixable`, `--ignore`, or
`--extend-fixable`.
This commit is contained in:
Zanie Blue 2023-10-08 11:14:37 -05:00 committed by GitHub
parent 2ba5677700
commit 2d6557a51b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 87 additions and 11 deletions

1
Cargo.lock generated
View file

@ -2494,6 +2494,7 @@ dependencies = [
"glob", "glob",
"globset", "globset",
"ignore", "ignore",
"is-macro",
"itertools 0.11.0", "itertools 0.11.0",
"log", "log",
"once_cell", "once_cell",

View file

@ -737,6 +737,42 @@ fn preview_disabled_prefix_empty() {
"###); "###);
} }
#[test]
fn preview_disabled_does_not_warn_for_empty_ignore_selections() {
// Does not warn that the selection is empty since the user is not trying to enable the rule
let args = ["--ignore", "CPY"];
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
----- stdout -----
-:1:1: E741 Ambiguous variable name: `I`
Found 1 error.
----- stderr -----
"###);
}
#[test]
fn preview_disabled_does_not_warn_for_empty_fixable_selections() {
// Does not warn that the selection is empty since the user is not trying to enable the rule
let args = ["--fixable", "CPY"];
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
----- stdout -----
-:1:1: E741 Ambiguous variable name: `I`
Found 1 error.
----- stderr -----
"###);
}
#[test] #[test]
fn preview_group_selector() { fn preview_group_selector() {
// `--select PREVIEW` should error (selector was removed) // `--select PREVIEW` should error (selector was removed)

View file

@ -25,6 +25,7 @@ anyhow = { workspace = true }
colored = { workspace = true } colored = { workspace = true }
dirs = { version = "5.0.0" } dirs = { version = "5.0.0" }
ignore = { workspace = true } ignore = { workspace = true }
is-macro = { workspace = true }
itertools = { workspace = true } itertools = { workspace = true }
log = { workspace = true } log = { workspace = true }
glob = { workspace = true } glob = { workspace = true }

View file

@ -57,6 +57,51 @@ pub struct RuleSelection {
pub extend_fixable: Vec<RuleSelector>, pub extend_fixable: Vec<RuleSelector>,
} }
#[derive(Debug, Eq, PartialEq, is_macro::Is)]
pub enum RuleSelectorKind {
/// Enables the selected rules
Enable,
/// Disables the selected rules
Disable,
/// Modifies the behavior of selected rules
Modify,
}
impl RuleSelection {
pub fn selectors_by_kind(&self) -> impl Iterator<Item = (RuleSelectorKind, &RuleSelector)> {
self.select
.iter()
.flatten()
.map(|selector| (RuleSelectorKind::Enable, selector))
.chain(
self.fixable
.iter()
.flatten()
.map(|selector| (RuleSelectorKind::Modify, selector)),
)
.chain(
self.ignore
.iter()
.map(|selector| (RuleSelectorKind::Disable, selector)),
)
.chain(
self.extend_select
.iter()
.map(|selector| (RuleSelectorKind::Enable, selector)),
)
.chain(
self.unfixable
.iter()
.map(|selector| (RuleSelectorKind::Modify, selector)),
)
.chain(
self.extend_fixable
.iter()
.map(|selector| (RuleSelectorKind::Modify, selector)),
)
}
}
#[derive(Debug, Default)] #[derive(Debug, Default)]
pub struct Configuration { pub struct Configuration {
// Global options // Global options
@ -704,16 +749,7 @@ impl LintConfiguration {
} }
// Check for selections that require a warning // Check for selections that require a warning
for selector in selection for (kind, selector) in selection.selectors_by_kind() {
.select
.iter()
.chain(selection.fixable.iter())
.flatten()
.chain(selection.ignore.iter())
.chain(selection.extend_select.iter())
.chain(selection.unfixable.iter())
.chain(selection.extend_fixable.iter())
{
#[allow(deprecated)] #[allow(deprecated)]
if matches!(selector, RuleSelector::Nursery) { if matches!(selector, RuleSelector::Nursery) {
let suggestion = if preview.mode.is_disabled() { let suggestion = if preview.mode.is_disabled() {
@ -725,7 +761,9 @@ impl LintConfiguration {
warn_user_once!("The `NURSERY` selector has been deprecated.{suggestion}"); warn_user_once!("The `NURSERY` selector has been deprecated.{suggestion}");
}; };
if preview.mode.is_disabled() { // Only warn for the following selectors if used to enable rules
// e.g. use with `--ignore` or `--fixable` is okay
if preview.mode.is_disabled() && kind.is_enable() {
if let RuleSelector::Rule { prefix, .. } = selector { if let RuleSelector::Rule { prefix, .. } = selector {
if prefix.rules().any(|rule| rule.is_nursery()) { if prefix.rules().any(|rule| rule.is_nursery()) {
deprecated_nursery_selectors.insert(selector); deprecated_nursery_selectors.insert(selector);