Ignore preview status for fixable and unfixable selectors (#9538)

## Summary

Right now, if you run with `explicit-preview-rules`, and use something
like `select = ["RUF017"]`, we won't actually enable fixing for that
rule, because `fixable = ["ALL"]` (the default) won't include `RUF017`
due to the `explicit-preview-rules`.

The framing in this PR is that `explicit-preview-rules` should only
affect the enablement selectors, whereas the fixable selectors should
just include all possible matching rules. I think this will lead to the
most intuitive behavior.

Closes https://github.com/astral-sh/ruff/issues/9282. (An alternative to
https://github.com/astral-sh/ruff/pull/9284.)
This commit is contained in:
Charlie Marsh 2024-01-15 21:48:41 -05:00 committed by GitHub
parent f9331c7683
commit 9a2f3e2cef
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 69 additions and 5 deletions

View file

@ -841,8 +841,9 @@ fn nursery_direct() {
success: false success: false
exit_code: 1 exit_code: 1
----- stdout ----- ----- stdout -----
-:1:2: E225 Missing whitespace around operator -:1:2: E225 [*] Missing whitespace around operator
Found 1 error. Found 1 error.
[*] 1 fixable with the `--fix` option.
----- stderr ----- ----- stderr -----
warning: Selection of nursery rule `E225` without the `--preview` flag is deprecated. warning: Selection of nursery rule `E225` without the `--preview` flag is deprecated.
@ -859,8 +860,9 @@ fn nursery_group_selector() {
exit_code: 1 exit_code: 1
----- stdout ----- ----- stdout -----
-:1:1: CPY001 Missing copyright notice at top of file -:1:1: CPY001 Missing copyright notice at top of file
-:1:2: E225 Missing whitespace around operator -:1:2: E225 [*] Missing whitespace around operator
Found 2 errors. Found 2 errors.
[*] 1 fixable with the `--fix` option.
----- stderr ----- ----- stderr -----
warning: The `NURSERY` selector has been deprecated. Use the `--preview` flag instead. warning: The `NURSERY` selector has been deprecated. Use the `--preview` flag instead.
@ -1656,3 +1658,65 @@ def log(x, base) -> float:
); );
Ok(()) Ok(())
} }
#[test]
fn fix_preview() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint]
preview = true
explicit-preview-rules = true
select = ["RUF017"]
"#,
)?;
let mut cmd = RuffCheck::default().config(&ruff_toml).build();
assert_cmd_snapshot!(cmd
.pass_stdin("x = [1, 2, 3]\ny = [4, 5, 6]\nsum([x, y], [])"),
@r###"
success: false
exit_code: 1
----- stdout -----
-:3:1: RUF017 Avoid quadratic list summation
Found 1 error.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).
----- stderr -----
"###);
Ok(())
}
#[test]
fn unfixable_preview() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint]
preview = true
explicit-preview-rules = true
select = ["RUF017"]
unfixable = ["RUF"]
"#,
)?;
let mut cmd = RuffCheck::default().config(&ruff_toml).build();
assert_cmd_snapshot!(cmd
.pass_stdin("x = [1, 2, 3]\ny = [4, 5, 6]\nsum([x, y], [])"),
@r###"
success: false
exit_code: 1
----- stdout -----
-:3:1: RUF017 Avoid quadratic list summation
Found 1 error.
----- stderr -----
"###);
Ok(())
}

View file

@ -714,7 +714,7 @@ impl LintConfiguration {
.collect(); .collect();
// The fixable set keeps track of which rules are fixable. // The fixable set keeps track of which rules are fixable.
let mut fixable_set: RuleSet = RuleSelector::All.rules(&preview).collect(); let mut fixable_set: RuleSet = RuleSelector::All.all_rules().collect();
// Ignores normally only subtract from the current set of selected // Ignores normally only subtract from the current set of selected
// rules. By that logic the ignore in `select = [], ignore = ["E501"]` // rules. By that logic the ignore in `select = [], ignore = ["E501"]`
@ -786,7 +786,7 @@ impl LintConfiguration {
.chain(selection.extend_fixable.iter()) .chain(selection.extend_fixable.iter())
.filter(|s| s.specificity() == spec) .filter(|s| s.specificity() == spec)
{ {
for rule in selector.rules(&preview) { for rule in selector.all_rules() {
fixable_map_updates.insert(rule, true); fixable_map_updates.insert(rule, true);
} }
} }
@ -796,7 +796,7 @@ impl LintConfiguration {
.chain(carriedover_unfixables.into_iter().flatten()) .chain(carriedover_unfixables.into_iter().flatten())
.filter(|s| s.specificity() == spec) .filter(|s| s.specificity() == spec)
{ {
for rule in selector.rules(&preview) { for rule in selector.all_rules() {
fixable_map_updates.insert(rule, false); fixable_map_updates.insert(rule, false);
} }
} }