Skip to content

Commit 7d658ef

Browse files
zaniebkonstin
authored andcommitted
Only show warnings for empty preview selectors when enabling rules (#7842)
Closes #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`.
1 parent f64e10e commit 7d658ef

File tree

4 files changed

+87
-11
lines changed

4 files changed

+87
-11
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ruff_cli/tests/integration_test.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,42 @@ fn preview_disabled_prefix_empty() {
737737
"###);
738738
}
739739

740+
#[test]
741+
fn preview_disabled_does_not_warn_for_empty_ignore_selections() {
742+
// Does not warn that the selection is empty since the user is not trying to enable the rule
743+
let args = ["--ignore", "CPY"];
744+
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
745+
.args(STDIN_BASE_OPTIONS)
746+
.args(args)
747+
.pass_stdin("I=42\n"), @r###"
748+
success: false
749+
exit_code: 1
750+
----- stdout -----
751+
-:1:1: E741 Ambiguous variable name: `I`
752+
Found 1 error.
753+
754+
----- stderr -----
755+
"###);
756+
}
757+
758+
#[test]
759+
fn preview_disabled_does_not_warn_for_empty_fixable_selections() {
760+
// Does not warn that the selection is empty since the user is not trying to enable the rule
761+
let args = ["--fixable", "CPY"];
762+
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
763+
.args(STDIN_BASE_OPTIONS)
764+
.args(args)
765+
.pass_stdin("I=42\n"), @r###"
766+
success: false
767+
exit_code: 1
768+
----- stdout -----
769+
-:1:1: E741 Ambiguous variable name: `I`
770+
Found 1 error.
771+
772+
----- stderr -----
773+
"###);
774+
}
775+
740776
#[test]
741777
fn preview_group_selector() {
742778
// `--select PREVIEW` should error (selector was removed)

crates/ruff_workspace/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ anyhow = { workspace = true }
2525
colored = { workspace = true }
2626
dirs = { version = "5.0.0" }
2727
ignore = { workspace = true }
28+
is-macro = { workspace = true }
2829
itertools = { workspace = true }
2930
log = { workspace = true }
3031
glob = { workspace = true }

crates/ruff_workspace/src/configuration.rs

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,51 @@ pub struct RuleSelection {
5757
pub extend_fixable: Vec<RuleSelector>,
5858
}
5959

60+
#[derive(Debug, Eq, PartialEq, is_macro::Is)]
61+
pub enum RuleSelectorKind {
62+
/// Enables the selected rules
63+
Enable,
64+
/// Disables the selected rules
65+
Disable,
66+
/// Modifies the behavior of selected rules
67+
Modify,
68+
}
69+
70+
impl RuleSelection {
71+
pub fn selectors_by_kind(&self) -> impl Iterator<Item = (RuleSelectorKind, &RuleSelector)> {
72+
self.select
73+
.iter()
74+
.flatten()
75+
.map(|selector| (RuleSelectorKind::Enable, selector))
76+
.chain(
77+
self.fixable
78+
.iter()
79+
.flatten()
80+
.map(|selector| (RuleSelectorKind::Modify, selector)),
81+
)
82+
.chain(
83+
self.ignore
84+
.iter()
85+
.map(|selector| (RuleSelectorKind::Disable, selector)),
86+
)
87+
.chain(
88+
self.extend_select
89+
.iter()
90+
.map(|selector| (RuleSelectorKind::Enable, selector)),
91+
)
92+
.chain(
93+
self.unfixable
94+
.iter()
95+
.map(|selector| (RuleSelectorKind::Modify, selector)),
96+
)
97+
.chain(
98+
self.extend_fixable
99+
.iter()
100+
.map(|selector| (RuleSelectorKind::Modify, selector)),
101+
)
102+
}
103+
}
104+
60105
#[derive(Debug, Default)]
61106
pub struct Configuration {
62107
// Global options
@@ -704,16 +749,7 @@ impl LintConfiguration {
704749
}
705750

706751
// Check for selections that require a warning
707-
for selector in selection
708-
.select
709-
.iter()
710-
.chain(selection.fixable.iter())
711-
.flatten()
712-
.chain(selection.ignore.iter())
713-
.chain(selection.extend_select.iter())
714-
.chain(selection.unfixable.iter())
715-
.chain(selection.extend_fixable.iter())
716-
{
752+
for (kind, selector) in selection.selectors_by_kind() {
717753
#[allow(deprecated)]
718754
if matches!(selector, RuleSelector::Nursery) {
719755
let suggestion = if preview.mode.is_disabled() {
@@ -725,7 +761,9 @@ impl LintConfiguration {
725761
warn_user_once!("The `NURSERY` selector has been deprecated.{suggestion}");
726762
};
727763

728-
if preview.mode.is_disabled() {
764+
// Only warn for the following selectors if used to enable rules
765+
// e.g. use with `--ignore` or `--fixable` is okay
766+
if preview.mode.is_disabled() && kind.is_enable() {
729767
if let RuleSelector::Rule { prefix, .. } = selector {
730768
if prefix.rules().any(|rule| rule.is_nursery()) {
731769
deprecated_nursery_selectors.insert(selector);

0 commit comments

Comments
 (0)