Skip to content

Commit

Permalink
allow negative patterns to un-ignore rules
Browse files Browse the repository at this point in the history
Co-authored-by: Alex Waygood <Alex.Waygood@gmail.com>
  • Loading branch information
carljm and AlexWaygood committed Apr 11, 2024
1 parent bc56cda commit 1bc4610
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 41 deletions.
51 changes: 46 additions & 5 deletions crates/ruff/tests/lint.rs
Expand Up @@ -1249,9 +1249,9 @@ fn negated_per_file_ignores_absolute() -> Result<()> {
Ok(())
}

/// patterns are additive, can't use negative patterns to "un-ignore"
/// negated patterns can be used to un-ignore previously-ignored rules
#[test]
fn negated_per_file_ignores_overlap() -> Result<()> {
fn negated_per_file_ignores_can_unignore() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
Expand All @@ -1275,10 +1275,51 @@ fn negated_per_file_ignores_overlap() -> Result<()> {
.arg("RUF901")
.current_dir(&tempdir)
, @r###"
success: true
exit_code: 0
success: false
exit_code: 1
----- stdout -----
All checks passed!
foo.py:1:1: RUF901 [*] Hey this is a stable test rule with a safe fix.
Found 1 error.
[*] 1 fixable with the `--fix` option.
----- stderr -----
"###);
Ok(())
}

/// extend-per-file-ignores can un-ignore from per-file-ignores
#[test]
fn negated_extend_per_file_ignores_can_unignore() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint.per-file-ignores]
"*.py" = ["RUF"]
[lint.extend-per-file-ignores]
"!foo.py" = ["RUF"]
"#,
)?;
let foo_file = tempdir.path().join("foo.py");
fs::write(foo_file, "")?;
let bar_file = tempdir.path().join("bar.py");
fs::write(bar_file, "")?;

assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.arg("--config")
.arg(&ruff_toml)
.arg("--select")
.arg("RUF901")
.current_dir(&tempdir)
, @r###"
success: false
exit_code: 1
----- stdout -----
foo.py:1:1: RUF901 [*] Hey this is a stable test rule with a safe fix.
Found 1 error.
[*] 1 fixable with the `--fix` option.
----- stderr -----
"###);
Expand Down
76 changes: 45 additions & 31 deletions crates/ruff_linter/src/fs.rs
Expand Up @@ -9,44 +9,58 @@ use crate::settings::types::CompiledPerFileIgnoreList;
/// Create a set with codes matching the pattern/code pairs.
pub(crate) fn ignores_from_path(path: &Path, ignore_list: &CompiledPerFileIgnoreList) -> RuleSet {
let file_name = path.file_name().expect("Unable to parse filename");
ignore_list
.iter()
.filter_map(|entry| {
if entry.basename_matcher.is_match(file_name) {
if entry.negated { None } else {
debug!(
"Adding per-file ignores for {:?} due to basename match on {:?}: {:?}",
path,
entry.basename_matcher.glob().regex(),
entry.rules
);
Some(&entry.rules)
}
} else if entry.absolute_matcher.is_match(path) {
if entry.negated { None } else {
debug!(
"Adding per-file ignores for {:?} due to absolute match on {:?}: {:?}",
path,
entry.absolute_matcher.glob().regex(),
entry.rules
);
Some(&entry.rules)
}
} else if entry.negated {
let mut ruleset = RuleSet::empty();
for entry in ignore_list.iter() {
ruleset = if entry.basename_matcher.is_match(file_name) {
if entry.negated {
debug!(
"Adding per-file ignores for {:?} due to negated pattern matching neither {:?} nor {:?}: {:?}",
"Removing per-file ignores for {:?} due to negative basename match on {:?}: {:?}",
path,
entry.basename_matcher.glob().regex(),
entry.absolute_matcher.glob().regex(),
entry.rules
);
Some(&entry.rules)
ruleset.subtract(&entry.rules)
} else {
None
debug!(
"Adding per-file ignores for {:?} due to positive basename match on {:?}: {:?}",
path,
entry.basename_matcher.glob().regex(),
entry.rules
);
ruleset.union(&entry.rules)
}
} else if entry.absolute_matcher.is_match(path) {
if entry.negated {
debug!(
"Removing per-file ignores for {:?} due to negative absolute match on {:?}: {:?}",
path,
entry.basename_matcher.glob().regex(),
entry.rules
);
ruleset.subtract(&entry.rules)
} else {
debug!(
"Adding per-file ignores for {:?} due to positive absolute match on {:?}: {:?}",
path,
entry.basename_matcher.glob().regex(),
entry.rules
);
ruleset.union(&entry.rules)
}
})
.flatten()
.collect()
} else if entry.negated {
debug!(
"Adding per-file ignores for {:?} due to negated pattern matching neither {:?} nor {:?}: {:?}",
path,
entry.basename_matcher.glob().regex(),
entry.absolute_matcher.glob().regex(),
entry.rules
);
ruleset.union(&entry.rules)
} else {
ruleset
};
}
ruleset
}

/// Convert any path to an absolute path (based on the current working
Expand Down
10 changes: 7 additions & 3 deletions crates/ruff_workspace/src/options.rs
Expand Up @@ -906,7 +906,9 @@ pub struct LintCommonOptions {
// Tables are required to go last.
/// A list of mappings from file pattern to rule codes or prefixes to
/// exclude, when considering any matching files. An initial '!' negates
/// the file pattern.
/// the file pattern, meaning the listed rule(s) will be ignored in all files not matching the
/// pattern, and will not be ignored in files matching the pattern (possibly overriding
/// previous ignore rules.)
#[option(
default = "{}",
value_type = "dict[str, list[RuleSelector]]",
Expand All @@ -915,8 +917,10 @@ pub struct LintCommonOptions {
# Ignore `E402` (import violations) in all `__init__.py` files, and in `path/to/file.py`.
"__init__.py" = ["E402"]
"path/to/file.py" = ["E402"]
# Ignore `D` rules everywhere except for the `src/` directory.
"!src/**.py" = ["D"]
# Ignore `E402` rules everywhere except for the `src/` directory.
# This will mean `E402` is not ignored in `__init__.py` files in `src/`; to change this,
# list the ignore rule for `__init__.py` after this rule instead of before it.
"!src/**.py" = ["E402"]
"#
)]
pub per_file_ignores: Option<FxHashMap<String, Vec<RuleSelector>>>,
Expand Down
4 changes: 2 additions & 2 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 1bc4610

Please sign in to comment.