Add settings for promoting and demoting fixes (#7841)

Adds two configuration-file only settings `extend-safe-fixes` and
`extend-unsafe-fixes` which can be used to promote and demote the
applicability of fixes for rules.

Fixes with `Never` applicability cannot be promoted.
This commit is contained in:
Zanie Blue 2023-10-10 15:04:21 -05:00 committed by GitHub
parent 090c1a4a19
commit 739a8aa10e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 261 additions and 5 deletions

1
Cargo.lock generated
View file

@ -2143,6 +2143,7 @@ name = "ruff_diagnostics"
version = "0.0.0"
dependencies = [
"anyhow",
"is-macro",
"log",
"ruff_text_size",
"serde",

View file

@ -1,6 +1,5 @@
#![cfg(not(target_family = "wasm"))]
#[cfg(unix)]
use std::fs;
#[cfg(unix)]
use std::fs::Permissions;
@ -12,13 +11,13 @@ use std::process::Command;
use std::str;
#[cfg(unix)]
use anyhow::{Context, Result};
use anyhow::Context;
use anyhow::Result;
#[cfg(unix)]
use clap::Parser;
use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};
#[cfg(unix)]
use path_absolutize::path_dedot;
#[cfg(unix)]
use tempfile::TempDir;
#[cfg(unix)]
@ -1208,3 +1207,122 @@ fn diff_only_unsafe_fixes_available() {
"###
);
}
#[test]
fn check_extend_unsafe_fixes() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint]
extend-unsafe-fixes = ["UP034"]
"#,
)?;
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "--config"])
.arg(&ruff_toml)
.arg("-")
.args([
"--output-format",
"text",
"--no-cache",
"--select",
"F601,UP034",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
-:1:14: F601 Dictionary key literal `'a'` repeated
-:2:7: UP034 Avoid extraneous parentheses
Found 2 errors.
2 hidden fixes can be enabled with the `--unsafe-fixes` option.
----- stderr -----
"###);
Ok(())
}
#[test]
fn check_extend_safe_fixes() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint]
extend-safe-fixes = ["F601"]
"#,
)?;
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "--config"])
.arg(&ruff_toml)
.arg("-")
.args([
"--output-format",
"text",
"--no-cache",
"--select",
"F601,UP034",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
-:1:14: F601 [*] Dictionary key literal `'a'` repeated
-:2:7: UP034 [*] Avoid extraneous parentheses
Found 2 errors.
[*] 2 fixable with the `--fix` option.
----- stderr -----
"###);
Ok(())
}
#[test]
fn check_extend_unsafe_fixes_conflict_with_extend_safe_fixes() -> Result<()> {
// Adding a rule to both options should result in it being treated as unsafe
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint]
extend-unsafe-fixes = ["UP034"]
extend-safe-fixes = ["UP034"]
"#,
)?;
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "--config"])
.arg(&ruff_toml)
.arg("-")
.args([
"--output-format",
"text",
"--no-cache",
"--select",
"F601,UP034",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
-:1:14: F601 Dictionary key literal `'a'` repeated
-:2:7: UP034 Avoid extraneous parentheses
Found 2 errors.
2 hidden fixes can be enabled with the `--unsafe-fixes` option.
----- stderr -----
"###);
Ok(())
}

View file

@ -17,4 +17,5 @@ ruff_text_size = { path = "../ruff_text_size" }
anyhow = { workspace = true }
log = { workspace = true }
is-macro = { workspace = true }
serde = { workspace = true, optional = true, features = [] }

View file

@ -6,7 +6,7 @@ use ruff_text_size::{Ranged, TextSize};
use crate::edit::Edit;
/// Indicates if a fix can be applied.
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, is_macro::Is)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "serde", serde(rename_all = "lowercase"))]
pub enum Applicability {
@ -138,4 +138,11 @@ impl Fix {
pub fn applies(&self, applicability: Applicability) -> bool {
self.applicability >= applicability
}
/// Create a new [`Fix`] with the given [`Applicability`].
#[must_use]
pub fn with_applicability(mut self, applicability: Applicability) -> Self {
self.applicability = applicability;
self
}
}

View file

@ -8,7 +8,7 @@ use itertools::Itertools;
use log::error;
use rustc_hash::FxHashMap;
use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::{Applicability, Diagnostic};
use ruff_python_ast::imports::ImportMap;
use ruff_python_ast::PySourceType;
use ruff_python_codegen::Stylist;
@ -260,6 +260,29 @@ pub fn check_path(
}
}
// Update fix applicability to account for overrides
if !settings.extend_safe_fixes.is_empty() || !settings.extend_unsafe_fixes.is_empty() {
for diagnostic in &mut diagnostics {
if let Some(fix) = diagnostic.fix.take() {
// Enforce demotions over promotions so if someone puts a rule in both we are conservative
if fix.applicability().is_safe()
&& settings
.extend_unsafe_fixes
.contains(diagnostic.kind.rule())
{
diagnostic.set_fix(fix.with_applicability(Applicability::Unsafe));
} else if fix.applicability().is_unsafe()
&& settings.extend_safe_fixes.contains(diagnostic.kind.rule())
{
diagnostic.set_fix(fix.with_applicability(Applicability::Safe));
} else {
// Retain the existing fix (will be dropped from `.take()` otherwise)
diagnostic.set_fix(fix);
}
}
}
}
LinterResult::new((diagnostics, imports), error)
}

View file

@ -42,6 +42,8 @@ pub struct LinterSettings {
pub rules: RuleTable,
pub per_file_ignores: Vec<(GlobMatcher, GlobMatcher, RuleSet)>,
pub extend_unsafe_fixes: RuleSet,
pub extend_safe_fixes: RuleSet,
pub target_version: PythonVersion,
pub preview: PreviewMode,
@ -139,6 +141,8 @@ impl LinterSettings {
namespace_packages: vec![],
per_file_ignores: vec![],
extend_safe_fixes: RuleSet::empty(),
extend_unsafe_fixes: RuleSet::empty(),
src: vec![path_dedot::CWD.clone()],
// Needs duplicating

View file

@ -226,6 +226,28 @@ impl Configuration {
.chain(lint.extend_per_file_ignores)
.collect(),
)?,
extend_safe_fixes: lint
.extend_safe_fixes
.iter()
.flat_map(|selector| {
selector.rules(&PreviewOptions {
mode: preview,
require_explicit: false,
})
})
.collect(),
extend_unsafe_fixes: lint
.extend_unsafe_fixes
.iter()
.flat_map(|selector| {
selector.rules(&PreviewOptions {
mode: preview,
require_explicit: false,
})
})
.collect(),
src: self.src.unwrap_or_else(|| vec![project_root.to_path_buf()]),
explicit_preview_rules: lint.explicit_preview_rules.unwrap_or_default(),
@ -494,6 +516,10 @@ pub struct LintConfiguration {
pub rule_selections: Vec<RuleSelection>,
pub explicit_preview_rules: Option<bool>,
// Fix configuration
pub extend_unsafe_fixes: Vec<RuleSelector>,
pub extend_safe_fixes: Vec<RuleSelector>,
// Global lint settings
pub allowed_confusables: Option<Vec<char>>,
pub dummy_variable_rgx: Option<Regex>,
@ -551,6 +577,8 @@ impl LintConfiguration {
.collect(),
extend_fixable: options.extend_fixable.unwrap_or_default(),
}],
extend_safe_fixes: options.extend_safe_fixes.unwrap_or_default(),
extend_unsafe_fixes: options.extend_unsafe_fixes.unwrap_or_default(),
allowed_confusables: options.allowed_confusables,
dummy_variable_rgx: options
.dummy_variable_rgx
@ -847,6 +875,16 @@ impl LintConfiguration {
.into_iter()
.chain(self.rule_selections)
.collect(),
extend_safe_fixes: config
.extend_safe_fixes
.into_iter()
.chain(self.extend_safe_fixes)
.collect(),
extend_unsafe_fixes: config
.extend_unsafe_fixes
.into_iter()
.chain(self.extend_unsafe_fixes)
.collect(),
allowed_confusables: self.allowed_confusables.or(config.allowed_confusables),
dummy_variable_rgx: self.dummy_variable_rgx.or(config.dummy_variable_rgx),
extend_per_file_ignores: config

View file

@ -523,6 +523,30 @@ pub struct LintOptions {
)]
pub ignore: Option<Vec<RuleSelector>>,
/// A list of rule codes or prefixes for which unsafe fixes should be considered
/// safe.
#[option(
default = "[]",
value_type = "list[RuleSelector]",
example = r#"
# Allow applying all unsafe fixes in the `E` rules and `F401` without the `--unsafe-fixes` flag
extend_safe_fixes = ["E", "F401"]
"#
)]
pub extend_safe_fixes: Option<Vec<RuleSelector>>,
/// A list of rule codes or prefixes for which safe fixes should be considered
/// unsafe.
#[option(
default = "[]",
value_type = "list[RuleSelector]",
example = r#"
# Require the `--unsafe-fixes` flag when fixing the `E` rules and `F401`
extend_unsafe_fixes = ["E", "F401"]
"#
)]
pub extend_unsafe_fixes: Option<Vec<RuleSelector>>,
/// Avoid automatically removing unused imports in `__init__.py` files. Such
/// imports will still be flagged, but with a dedicated message suggesting
/// that the import is either added to the module's `__all__` symbol, or

40
ruff.schema.json generated
View file

@ -107,6 +107,16 @@
}
}
},
"extend-safe-fixes": {
"description": "A list of rule codes or prefixes for which unsafe fixes should be considered safe.",
"type": [
"array",
"null"
],
"items": {
"$ref": "#/definitions/RuleSelector"
}
},
"extend-select": {
"description": "A list of rule codes or prefixes to enable, in addition to those specified by `select`.",
"type": [
@ -117,6 +127,16 @@
"$ref": "#/definitions/RuleSelector"
}
},
"extend-unsafe-fixes": {
"description": "A list of rule codes or prefixes for which safe fixes should be considered unsafe.",
"type": [
"array",
"null"
],
"items": {
"$ref": "#/definitions/RuleSelector"
}
},
"external": {
"description": "A list of rule codes that are unsupported by Ruff, but should be preserved when (e.g.) validating `# noqa` directives. Useful for retaining `# noqa` directives that cover plugins not yet implemented by Ruff.",
"type": [
@ -1612,6 +1632,16 @@
}
}
},
"extend-safe-fixes": {
"description": "A list of rule codes or prefixes for which unsafe fixes should be considered safe.",
"type": [
"array",
"null"
],
"items": {
"$ref": "#/definitions/RuleSelector"
}
},
"extend-select": {
"description": "A list of rule codes or prefixes to enable, in addition to those specified by `select`.",
"type": [
@ -1622,6 +1652,16 @@
"$ref": "#/definitions/RuleSelector"
}
},
"extend-unsafe-fixes": {
"description": "A list of rule codes or prefixes for which safe fixes should be considered unsafe.",
"type": [
"array",
"null"
],
"items": {
"$ref": "#/definitions/RuleSelector"
}
},
"external": {
"description": "A list of rule codes that are unsupported by Ruff, but should be preserved when (e.g.) validating `# noqa` directives. Useful for retaining `# noqa` directives that cover plugins not yet implemented by Ruff.",
"type": [