Skip to content

Commit

Permalink
Add settings for promoting and demoting fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Oct 6, 2023
1 parent 4f95df1 commit d7140c4
Show file tree
Hide file tree
Showing 9 changed files with 215 additions and 4 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

80 changes: 78 additions & 2 deletions crates/ruff_cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![cfg(not(target_family = "wasm"))]

#[cfg(unix)]
use std::fs;
#[cfg(unix)]
use std::fs::Permissions;
Expand All @@ -11,7 +10,6 @@ use std::path::Path;
use std::process::Command;
use std::str;

#[cfg(unix)]
use anyhow::{Context, Result};
#[cfg(unix)]
use clap::Parser;
Expand Down Expand Up @@ -1105,3 +1103,81 @@ fn diff_shows_unsafe_fixes_with_opt_in() {
"###
);
}

#[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(())
}
1 change: 1 addition & 0 deletions crates/ruff_diagnostics/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [] }
9 changes: 8 additions & 1 deletion crates/ruff_diagnostics/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
}
22 changes: 21 additions & 1 deletion crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -260,6 +260,26 @@ 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 {
// Check unsafe before safe so if someone puts a rule in both we are conservative
if settings
.extend_unsafe_fixes
.contains(diagnostic.kind.rule())
&& fix.applicability().is_always()
{
diagnostic.set_fix(fix.clone().with_applicability(Applicability::Sometimes));
} else if settings.extend_safe_fixes.contains(diagnostic.kind.rule())
&& fix.applicability().is_sometimes()
{
diagnostic.set_fix(fix.clone().with_applicability(Applicability::Always));
}
}
}
}

LinterResult::new((diagnostics, imports), error)
}

Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,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(),

Expand Down Expand Up @@ -449,6 +471,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>,
Expand Down Expand Up @@ -506,6 +532,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
Expand Down Expand Up @@ -809,6 +837,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
Expand Down
24 changes: 24 additions & 0 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,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
Expand Down
40 changes: 40 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d7140c4

Please sign in to comment.