Struct not tuple for compiled per-file ignores (#10864)

## 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>
This commit is contained in:
Carl Meyer 2024-04-11 13:47:57 -06:00 committed by GitHub
parent e7d1d43f39
commit 25f5a8b201
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 77 additions and 53 deletions

View file

@ -1,49 +1,46 @@
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use globset::GlobMatcher;
use log::debug; use log::debug;
use path_absolutize::Absolutize; use path_absolutize::Absolutize;
use crate::registry::RuleSet; use crate::registry::RuleSet;
use crate::settings::types::CompiledPerFileIgnoreList;
/// Create a set with codes matching the pattern/code pairs. /// Create a set with codes matching the pattern/code pairs.
pub(crate) fn ignores_from_path( pub(crate) fn ignores_from_path(path: &Path, ignore_list: &CompiledPerFileIgnoreList) -> RuleSet {
path: &Path,
pattern_code_pairs: &[(GlobMatcher, GlobMatcher, bool, RuleSet)],
) -> RuleSet {
let file_name = path.file_name().expect("Unable to parse filename"); let file_name = path.file_name().expect("Unable to parse filename");
pattern_code_pairs ignore_list
.iter() .iter()
.filter_map(|(absolute, basename, negated, rules)| { .filter_map(|entry| {
if basename.is_match(file_name) { if entry.basename_matcher.is_match(file_name) {
if *negated { None } else { if entry.negated { None } else {
debug!( debug!(
"Adding per-file ignores for {:?} due to basename match on {:?}: {:?}", "Adding per-file ignores for {:?} due to basename match on {:?}: {:?}",
path, path,
basename.glob().regex(), entry.basename_matcher.glob().regex(),
rules entry.rules
); );
Some(rules) Some(&entry.rules)
} }
} else if absolute.is_match(path) { } else if entry.absolute_matcher.is_match(path) {
if *negated { None } else { if entry.negated { None } else {
debug!( debug!(
"Adding per-file ignores for {:?} due to absolute match on {:?}: {:?}", "Adding per-file ignores for {:?} due to absolute match on {:?}: {:?}",
path, path,
absolute.glob().regex(), entry.absolute_matcher.glob().regex(),
rules entry.rules
); );
Some(rules) Some(&entry.rules)
} }
} else if *negated { } else if entry.negated {
debug!( debug!(
"Adding per-file ignores for {:?} due to negated pattern matching neither {:?} nor {:?}: {:?}", "Adding per-file ignores for {:?} due to negated pattern matching neither {:?} nor {:?}: {:?}",
path, path,
basename.glob().regex(), entry.basename_matcher.glob().regex(),
absolute.glob().regex(), entry.absolute_matcher.glob().regex(),
rules entry.rules
); );
Some(rules) Some(&entry.rules)
} else { } else {
None None
} }

View file

@ -16,7 +16,9 @@ mod tests {
use crate::pyproject_toml::lint_pyproject_toml; use crate::pyproject_toml::lint_pyproject_toml;
use crate::registry::Rule; 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::test::{test_path, test_resource_path};
use crate::{assert_messages, settings}; use crate::{assert_messages, settings};
@ -179,7 +181,7 @@ mod tests {
let mut settings = let mut settings =
settings::LinterSettings::for_rules(vec![Rule::UnusedNOQA, Rule::UnusedImport]); 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(), "RUF100_2.py".to_string(),
&["F401".parse().unwrap()], &["F401".parse().unwrap()],
None, None,
@ -236,7 +238,7 @@ mod tests {
let diagnostics = test_path( let diagnostics = test_path(
Path::new("ruff/ruff_per_file_ignores.py"), Path::new("ruff/ruff_per_file_ignores.py"),
&settings::LinterSettings { &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(), "ruff_per_file_ignores.py".to_string(),
&["F401".parse().unwrap(), "RUF100".parse().unwrap()], &["F401".parse().unwrap(), "RUF100".parse().unwrap()],
None, None,

View file

@ -22,7 +22,9 @@ use crate::rules::{
flake8_self, flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, flake8_self, flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe,
pep8_naming, pycodestyle, pydocstyle, pyflakes, pylint, pyupgrade, 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 crate::{codes, RuleSelector};
use super::line_width::IndentWidth; use super::line_width::IndentWidth;
@ -129,6 +131,9 @@ macro_rules! display_settings {
(@field $fmt:ident, $prefix:ident, $settings:ident.$field:ident | quoted) => { (@field $fmt:ident, $prefix:ident, $settings:ident.$field:ident | quoted) => {
writeln!($fmt, "{}{} = \"{}\"", $prefix, stringify!($field), $settings.$field)?; 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) => { (@field $fmt:ident, $prefix:ident, $settings:ident.$field:ident | nested) => {
write!($fmt, "{}", $settings.$field)?; write!($fmt, "{}", $settings.$field)?;
}; };
@ -213,7 +218,7 @@ pub struct LinterSettings {
pub project_root: PathBuf, pub project_root: PathBuf,
pub rules: RuleTable, pub rules: RuleTable,
pub per_file_ignores: PerFileIgnores, pub per_file_ignores: CompiledPerFileIgnoreList,
pub fix_safety: FixSafetyTable, pub fix_safety: FixSafetyTable,
pub target_version: PythonVersion, pub target_version: PythonVersion,
@ -388,7 +393,7 @@ impl LinterSettings {
logger_objects: vec![], logger_objects: vec![],
namespace_packages: vec![], namespace_packages: vec![],
per_file_ignores: PerFileIgnores::default(), per_file_ignores: CompiledPerFileIgnoreList::default(),
fix_safety: FixSafetyTable::default(), fix_safety: FixSafetyTable::default(),
src: vec![path_dedot::CWD.clone()], src: vec![path_dedot::CWD.clone()],

View file

@ -600,48 +600,68 @@ impl Display for RequiredVersion {
/// pattern matching. /// pattern matching.
pub type IdentifierPattern = glob::Pattern; pub type IdentifierPattern = glob::Pattern;
#[derive(Debug, Clone, CacheKey, Default)] #[derive(Debug, Clone, CacheKey)]
pub struct PerFileIgnores { pub struct CompiledPerFileIgnore {
// Ordered as (absolute path matcher, basename matcher, rules) pub absolute_matcher: GlobMatcher,
ignores: Vec<(GlobMatcher, GlobMatcher, bool, RuleSet)>, pub basename_matcher: GlobMatcher,
pub negated: bool,
pub rules: RuleSet,
} }
impl PerFileIgnores { 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 CompiledPerFileIgnoreList {
// Ordered as (absolute path matcher, basename matcher, rules)
ignores: Vec<CompiledPerFileIgnore>,
}
impl CompiledPerFileIgnoreList {
/// Given a list of patterns, create a `GlobSet`. /// Given a list of patterns, create a `GlobSet`.
pub fn resolve(per_file_ignores: Vec<PerFileIgnore>) -> Result<Self> { pub fn resolve(per_file_ignores: Vec<PerFileIgnore>) -> Result<Self> {
let ignores: Result<Vec<_>> = per_file_ignores let ignores: Result<Vec<_>> = per_file_ignores
.into_iter() .into_iter()
.map(|per_file_ignore| { .map(|per_file_ignore| {
// Construct absolute path matcher. // Construct absolute path matcher.
let absolute = let absolute_matcher =
Glob::new(&per_file_ignore.absolute.to_string_lossy())?.compile_matcher(); Glob::new(&per_file_ignore.absolute.to_string_lossy())?.compile_matcher();
// Construct basename matcher. // Construct basename matcher.
let basename = Glob::new(&per_file_ignore.basename)?.compile_matcher(); let basename_matcher = Glob::new(&per_file_ignore.basename)?.compile_matcher();
Ok(( Ok(CompiledPerFileIgnore {
absolute, absolute_matcher,
basename, basename_matcher,
per_file_ignore.negated, negated: per_file_ignore.negated,
per_file_ignore.rules, rules: per_file_ignore.rules,
)) })
}) })
.collect(); .collect();
Ok(Self { ignores: ignores? }) Ok(Self { ignores: ignores? })
} }
} }
impl Display for PerFileIgnores { impl Display for CompiledPerFileIgnoreList {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
if self.is_empty() { if self.ignores.is_empty() {
write!(f, "{{}}")?; write!(f, "{{}}")?;
} else { } else {
writeln!(f, "{{")?; writeln!(f, "{{")?;
for (absolute, basename, negated, rules) in &self.ignores { for ignore in &self.ignores {
writeln!( writeln!(f, "\t{ignore}")?;
f,
"\t{{ absolute = {absolute:#?}, basename = {basename:#?}, negated = {negated:#?}, rules = {rules} }},"
)?;
} }
write!(f, "}}")?; write!(f, "}}")?;
} }
@ -649,8 +669,8 @@ impl Display for PerFileIgnores {
} }
} }
impl Deref for PerFileIgnores { impl Deref for CompiledPerFileIgnoreList {
type Target = Vec<(GlobMatcher, GlobMatcher, bool, RuleSet)>; type Target = Vec<CompiledPerFileIgnore>;
fn deref(&self) -> &Self::Target { fn deref(&self) -> &Self::Target {
&self.ignores &self.ignores

View file

@ -27,8 +27,8 @@ use ruff_linter::rules::pycodestyle;
use ruff_linter::settings::fix_safety_table::FixSafetyTable; use ruff_linter::settings::fix_safety_table::FixSafetyTable;
use ruff_linter::settings::rule_table::RuleTable; use ruff_linter::settings::rule_table::RuleTable;
use ruff_linter::settings::types::{ use ruff_linter::settings::types::{
ExtensionMapping, FilePattern, FilePatternSet, PerFileIgnore, PerFileIgnores, PreviewMode, CompiledPerFileIgnoreList, ExtensionMapping, FilePattern, FilePatternSet, PerFileIgnore,
PythonVersion, RequiredVersion, SerializationFormat, UnsafeFixes, PreviewMode, PythonVersion, RequiredVersion, SerializationFormat, UnsafeFixes,
}; };
use ruff_linter::settings::{LinterSettings, DEFAULT_SELECTORS, DUMMY_VARIABLE_RGX, TASK_TAGS}; use ruff_linter::settings::{LinterSettings, DEFAULT_SELECTORS, DUMMY_VARIABLE_RGX, TASK_TAGS};
use ruff_linter::{ use ruff_linter::{
@ -259,7 +259,7 @@ impl Configuration {
line_length, line_length,
tab_size: self.indent_width.unwrap_or_default(), tab_size: self.indent_width.unwrap_or_default(),
namespace_packages: self.namespace_packages.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 lint.per_file_ignores
.unwrap_or_default() .unwrap_or_default()
.into_iter() .into_iter()