Skip to content

Commit

Permalink
Struct not tuple for compiled per-file ignores (#10864)
Browse files Browse the repository at this point in the history
## Summary

Code cleanup for per-file ignores; use a struct instead of a tuple.

Named the structs for individual ignores and the list of ignores
`CompiledPerFileIgnore` and `CompiledPerFileIgnoreList`. Name choice is
because we already have a `PerFileIgnore` struct for a
pre-compiled-matchers form of the config. Name bikeshedding welcome.

## Test Plan

Refactor, should not change behavior; existing tests pass.

---------

Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
  • Loading branch information
carljm and AlexWaygood authored Apr 11, 2024
1 parent e7d1d43 commit 25f5a8b
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 52 deletions.
41 changes: 19 additions & 22 deletions crates/ruff_linter/src/fs.rs
Original file line number Diff line number Diff line change
@@ -1,49 +1,46 @@
use std::path::{Path, PathBuf};

use globset::GlobMatcher;
use log::debug;
use path_absolutize::Absolutize;

use crate::registry::RuleSet;
use crate::settings::types::CompiledPerFileIgnoreList;

/// Create a set with codes matching the pattern/code pairs.
pub(crate) fn ignores_from_path(
path: &Path,
pattern_code_pairs: &[(GlobMatcher, GlobMatcher, bool, RuleSet)],
) -> RuleSet {
pub(crate) fn ignores_from_path(path: &Path, ignore_list: &CompiledPerFileIgnoreList) -> RuleSet {
let file_name = path.file_name().expect("Unable to parse filename");
pattern_code_pairs
ignore_list
.iter()
.filter_map(|(absolute, basename, negated, rules)| {
if basename.is_match(file_name) {
if *negated { None } else {
.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,
basename.glob().regex(),
rules
entry.basename_matcher.glob().regex(),
entry.rules
);
Some(rules)
Some(&entry.rules)
}
} else if absolute.is_match(path) {
if *negated { None } else {
} 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,
absolute.glob().regex(),
rules
entry.absolute_matcher.glob().regex(),
entry.rules
);
Some(rules)
Some(&entry.rules)
}
} else if *negated {
} else if entry.negated {
debug!(
"Adding per-file ignores for {:?} due to negated pattern matching neither {:?} nor {:?}: {:?}",
path,
basename.glob().regex(),
absolute.glob().regex(),
rules
entry.basename_matcher.glob().regex(),
entry.absolute_matcher.glob().regex(),
entry.rules
);
Some(rules)
Some(&entry.rules)
} else {
None
}
Expand Down
8 changes: 5 additions & 3 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ mod tests {

use crate::pyproject_toml::lint_pyproject_toml;
use crate::registry::Rule;
use crate::settings::types::{PerFileIgnore, PerFileIgnores, PreviewMode, PythonVersion};
use crate::settings::types::{
CompiledPerFileIgnoreList, PerFileIgnore, PreviewMode, PythonVersion,
};
use crate::test::{test_path, test_resource_path};
use crate::{assert_messages, settings};

Expand Down Expand Up @@ -179,7 +181,7 @@ mod tests {
let mut settings =
settings::LinterSettings::for_rules(vec![Rule::UnusedNOQA, Rule::UnusedImport]);

settings.per_file_ignores = PerFileIgnores::resolve(vec![PerFileIgnore::new(
settings.per_file_ignores = CompiledPerFileIgnoreList::resolve(vec![PerFileIgnore::new(
"RUF100_2.py".to_string(),
&["F401".parse().unwrap()],
None,
Expand Down Expand Up @@ -236,7 +238,7 @@ mod tests {
let diagnostics = test_path(
Path::new("ruff/ruff_per_file_ignores.py"),
&settings::LinterSettings {
per_file_ignores: PerFileIgnores::resolve(vec![PerFileIgnore::new(
per_file_ignores: CompiledPerFileIgnoreList::resolve(vec![PerFileIgnore::new(
"ruff_per_file_ignores.py".to_string(),
&["F401".parse().unwrap(), "RUF100".parse().unwrap()],
None,
Expand Down
11 changes: 8 additions & 3 deletions crates/ruff_linter/src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ use crate::rules::{
flake8_self, flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe,
pep8_naming, pycodestyle, pydocstyle, pyflakes, pylint, pyupgrade,
};
use crate::settings::types::{ExtensionMapping, FilePatternSet, PerFileIgnores, PythonVersion};
use crate::settings::types::{
CompiledPerFileIgnoreList, ExtensionMapping, FilePatternSet, PythonVersion,
};
use crate::{codes, RuleSelector};

use super::line_width::IndentWidth;
Expand Down Expand Up @@ -129,6 +131,9 @@ macro_rules! display_settings {
(@field $fmt:ident, $prefix:ident, $settings:ident.$field:ident | quoted) => {
writeln!($fmt, "{}{} = \"{}\"", $prefix, stringify!($field), $settings.$field)?;
};
(@field $fmt:ident, $prefix:ident, $settings:ident.$field:ident | globmatcher) => {
writeln!($fmt, "{}{} = \"{}\"", $prefix, stringify!($field), $settings.$field.glob())?;
};
(@field $fmt:ident, $prefix:ident, $settings:ident.$field:ident | nested) => {
write!($fmt, "{}", $settings.$field)?;
};
Expand Down Expand Up @@ -213,7 +218,7 @@ pub struct LinterSettings {
pub project_root: PathBuf,

pub rules: RuleTable,
pub per_file_ignores: PerFileIgnores,
pub per_file_ignores: CompiledPerFileIgnoreList,
pub fix_safety: FixSafetyTable,

pub target_version: PythonVersion,
Expand Down Expand Up @@ -388,7 +393,7 @@ impl LinterSettings {
logger_objects: vec![],
namespace_packages: vec![],

per_file_ignores: PerFileIgnores::default(),
per_file_ignores: CompiledPerFileIgnoreList::default(),
fix_safety: FixSafetyTable::default(),

src: vec![path_dedot::CWD.clone()],
Expand Down
62 changes: 41 additions & 21 deletions crates/ruff_linter/src/settings/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,57 +600,77 @@ impl Display for RequiredVersion {
/// pattern matching.
pub type IdentifierPattern = glob::Pattern;

#[derive(Debug, Clone, CacheKey)]
pub struct CompiledPerFileIgnore {
pub absolute_matcher: GlobMatcher,
pub basename_matcher: GlobMatcher,
pub negated: bool,
pub rules: RuleSet,
}

impl Display for CompiledPerFileIgnore {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
display_settings! {
formatter = f,
fields = [
self.absolute_matcher | globmatcher,
self.basename_matcher | globmatcher,
self.negated,
self.rules,
]
}
Ok(())
}
}

#[derive(Debug, Clone, CacheKey, Default)]
pub struct PerFileIgnores {
pub struct CompiledPerFileIgnoreList {
// Ordered as (absolute path matcher, basename matcher, rules)
ignores: Vec<(GlobMatcher, GlobMatcher, bool, RuleSet)>,
ignores: Vec<CompiledPerFileIgnore>,
}

impl PerFileIgnores {
impl CompiledPerFileIgnoreList {
/// Given a list of patterns, create a `GlobSet`.
pub fn resolve(per_file_ignores: Vec<PerFileIgnore>) -> Result<Self> {
let ignores: Result<Vec<_>> = per_file_ignores
.into_iter()
.map(|per_file_ignore| {
// Construct absolute path matcher.
let absolute =
let absolute_matcher =
Glob::new(&per_file_ignore.absolute.to_string_lossy())?.compile_matcher();

// Construct basename matcher.
let basename = Glob::new(&per_file_ignore.basename)?.compile_matcher();

Ok((
absolute,
basename,
per_file_ignore.negated,
per_file_ignore.rules,
))
let basename_matcher = Glob::new(&per_file_ignore.basename)?.compile_matcher();

Ok(CompiledPerFileIgnore {
absolute_matcher,
basename_matcher,
negated: per_file_ignore.negated,
rules: per_file_ignore.rules,
})
})
.collect();
Ok(Self { ignores: ignores? })
}
}

impl Display for PerFileIgnores {
impl Display for CompiledPerFileIgnoreList {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
if self.is_empty() {
if self.ignores.is_empty() {
write!(f, "{{}}")?;
} else {
writeln!(f, "{{")?;
for (absolute, basename, negated, rules) in &self.ignores {
writeln!(
f,
"\t{{ absolute = {absolute:#?}, basename = {basename:#?}, negated = {negated:#?}, rules = {rules} }},"
)?;
for ignore in &self.ignores {
writeln!(f, "\t{ignore}")?;
}
write!(f, "}}")?;
}
Ok(())
}
}

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

fn deref(&self) -> &Self::Target {
&self.ignores
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use ruff_linter::rules::pycodestyle;
use ruff_linter::settings::fix_safety_table::FixSafetyTable;
use ruff_linter::settings::rule_table::RuleTable;
use ruff_linter::settings::types::{
ExtensionMapping, FilePattern, FilePatternSet, PerFileIgnore, PerFileIgnores, PreviewMode,
PythonVersion, RequiredVersion, SerializationFormat, UnsafeFixes,
CompiledPerFileIgnoreList, ExtensionMapping, FilePattern, FilePatternSet, PerFileIgnore,
PreviewMode, PythonVersion, RequiredVersion, SerializationFormat, UnsafeFixes,
};
use ruff_linter::settings::{LinterSettings, DEFAULT_SELECTORS, DUMMY_VARIABLE_RGX, TASK_TAGS};
use ruff_linter::{
Expand Down Expand Up @@ -259,7 +259,7 @@ impl Configuration {
line_length,
tab_size: self.indent_width.unwrap_or_default(),
namespace_packages: self.namespace_packages.unwrap_or_default(),
per_file_ignores: PerFileIgnores::resolve(
per_file_ignores: CompiledPerFileIgnoreList::resolve(
lint.per_file_ignores
.unwrap_or_default()
.into_iter()
Expand Down

0 comments on commit 25f5a8b

Please sign in to comment.