Skip to content

Commit

Permalink
Support negated patterns in [extend-]per-file-ignores (astral-sh#10852)
Browse files Browse the repository at this point in the history
Fixes astral-sh#3172 

## Summary

Allow prefixing [extend-]per-file-ignores patterns with `!` to negate
the pattern; listed rules / prefixes will be ignored in all files that
don't match the pattern.

## Test Plan

Added tests for the feature.

Rendered docs and checked rendered output.
  • Loading branch information
carljm authored and Glyphack committed Apr 12, 2024
1 parent 2c4d529 commit 786430a
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 19 deletions.
80 changes: 80 additions & 0 deletions crates/ruff/tests/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1168,3 +1168,83 @@ def func():

Ok(())
}

/// Per-file selects via ! negation in per-file-ignores
#[test]
fn negated_per_file_ignores() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint.per-file-ignores]
"!selected.py" = ["RUF"]
"#,
)?;
let selected = tempdir.path().join("selected.py");
fs::write(selected, "")?;
let ignored = tempdir.path().join("ignored.py");
fs::write(ignored, "")?;

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 -----
selected.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(())
}

#[test]
fn negated_per_file_ignores_absolute() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint.per-file-ignores]
"!src/**.py" = ["RUF"]
"#,
)?;
let src_dir = tempdir.path().join("src");
fs::create_dir(&src_dir)?;
let selected = src_dir.join("selected.py");
fs::write(selected, "")?;
let ignored = tempdir.path().join("ignored.py");
fs::write(ignored, "")?;

insta::with_settings!({filters => vec![
// Replace windows paths
(r"\\", "/"),
]}, {
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 -----
src/selected.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(())
}
33 changes: 23 additions & 10 deletions crates/ruff_linter/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,37 @@ use crate::registry::RuleSet;
/// Create a set with codes matching the pattern/code pairs.
pub(crate) fn ignores_from_path(
path: &Path,
pattern_code_pairs: &[(GlobMatcher, GlobMatcher, RuleSet)],
pattern_code_pairs: &[(GlobMatcher, GlobMatcher, bool, RuleSet)],
) -> RuleSet {
let file_name = path.file_name().expect("Unable to parse filename");
pattern_code_pairs
.iter()
.filter_map(|(absolute, basename, rules)| {
.filter_map(|(absolute, basename, negated, rules)| {
if basename.is_match(file_name) {
debug!(
"Adding per-file ignores for {:?} due to basename match on {:?}: {:?}",
path,
basename.glob().regex(),
rules
);
Some(rules)
if *negated { None } else {
debug!(
"Adding per-file ignores for {:?} due to basename match on {:?}: {:?}",
path,
basename.glob().regex(),
rules
);
Some(rules)
}
} else if absolute.is_match(path) {
if *negated { None } else {
debug!(
"Adding per-file ignores for {:?} due to absolute match on {:?}: {:?}",
path,
absolute.glob().regex(),
rules
);
Some(rules)
}
} else if *negated {
debug!(
"Adding per-file ignores for {:?} due to absolute match on {:?}: {:?}",
"Adding per-file ignores for {:?} due to negated pattern matching neither {:?} nor {:?}: {:?}",
path,
basename.glob().regex(),
absolute.glob().regex(),
rules
);
Expand Down
27 changes: 21 additions & 6 deletions crates/ruff_linter/src/settings/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,22 @@ impl CacheKey for FilePatternSet {
pub struct PerFileIgnore {
pub(crate) basename: String,
pub(crate) absolute: PathBuf,
pub(crate) negated: bool,
pub(crate) rules: RuleSet,
}

impl PerFileIgnore {
pub fn new(pattern: String, prefixes: &[RuleSelector], project_root: Option<&Path>) -> Self {
pub fn new(
mut pattern: String,
prefixes: &[RuleSelector],
project_root: Option<&Path>,
) -> Self {
// Rules in preview are included here even if preview mode is disabled; it's safe to ignore disabled rules
let rules: RuleSet = prefixes.iter().flat_map(RuleSelector::all_rules).collect();
let negated = pattern.starts_with('!');
if negated {
pattern.drain(..1);
}
let path = Path::new(&pattern);
let absolute = match project_root {
Some(project_root) => fs::normalize_path_to(path, project_root),
Expand All @@ -312,6 +321,7 @@ impl PerFileIgnore {
Self {
basename: pattern,
absolute,
negated,
rules,
}
}
Expand Down Expand Up @@ -593,7 +603,7 @@ pub type IdentifierPattern = glob::Pattern;
#[derive(Debug, Clone, CacheKey, Default)]
pub struct PerFileIgnores {
// Ordered as (absolute path matcher, basename matcher, rules)
ignores: Vec<(GlobMatcher, GlobMatcher, RuleSet)>,
ignores: Vec<(GlobMatcher, GlobMatcher, bool, RuleSet)>,
}

impl PerFileIgnores {
Expand All @@ -609,7 +619,12 @@ impl PerFileIgnores {
// Construct basename matcher.
let basename = Glob::new(&per_file_ignore.basename)?.compile_matcher();

Ok((absolute, basename, per_file_ignore.rules))
Ok((
absolute,
basename,
per_file_ignore.negated,
per_file_ignore.rules,
))
})
.collect();
Ok(Self { ignores: ignores? })
Expand All @@ -622,10 +637,10 @@ impl Display for PerFileIgnores {
write!(f, "{{}}")?;
} else {
writeln!(f, "{{")?;
for (absolute, basename, rules) in &self.ignores {
for (absolute, basename, negated, rules) in &self.ignores {
writeln!(
f,
"\t{{ absolute = {absolute:#?}, basename = {basename:#?}, rules = {rules} }},"
"\t{{ absolute = {absolute:#?}, basename = {basename:#?}, negated = {negated:#?}, rules = {rules} }},"
)?;
}
write!(f, "}}")?;
Expand All @@ -635,7 +650,7 @@ impl Display for PerFileIgnores {
}

impl Deref for PerFileIgnores {
type Target = Vec<(GlobMatcher, GlobMatcher, RuleSet)>;
type Target = Vec<(GlobMatcher, GlobMatcher, bool, RuleSet)>;

fn deref(&self) -> &Self::Target {
&self.ignores
Expand Down
5 changes: 4 additions & 1 deletion crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,8 @@ 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.
/// exclude, when considering any matching files. An initial '!' negates
/// the file pattern.
#[option(
default = "{}",
value_type = "dict[str, list[RuleSelector]]",
Expand All @@ -914,6 +915,8 @@ 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" = ["F401"]
"#
)]
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 786430a

Please sign in to comment.