From 25f5a8b2019260eba3889d9800a60950a810a43e Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 11 Apr 2024 13:47:57 -0600 Subject: [PATCH] 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 --- crates/ruff_linter/src/fs.rs | 41 +++++++------- crates/ruff_linter/src/rules/ruff/mod.rs | 8 +-- crates/ruff_linter/src/settings/mod.rs | 11 ++-- crates/ruff_linter/src/settings/types.rs | 62 ++++++++++++++-------- crates/ruff_workspace/src/configuration.rs | 6 +-- 5 files changed, 76 insertions(+), 52 deletions(-) diff --git a/crates/ruff_linter/src/fs.rs b/crates/ruff_linter/src/fs.rs index aa230ab28788e..39a09b2ed115e 100644 --- a/crates/ruff_linter/src/fs.rs +++ b/crates/ruff_linter/src/fs.rs @@ -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 } diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 67acf7442b020..6d2d76f544fbe 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -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}; @@ -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, @@ -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, diff --git a/crates/ruff_linter/src/settings/mod.rs b/crates/ruff_linter/src/settings/mod.rs index 42a4ab88b8c19..05f3edbfce540 100644 --- a/crates/ruff_linter/src/settings/mod.rs +++ b/crates/ruff_linter/src/settings/mod.rs @@ -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; @@ -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)?; }; @@ -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, @@ -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()], diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index a06bb2ecaeb54..77f9c5ed28d0e 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -600,48 +600,68 @@ 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, } -impl PerFileIgnores { +impl CompiledPerFileIgnoreList { /// Given a list of patterns, create a `GlobSet`. pub fn resolve(per_file_ignores: Vec) -> Result { let ignores: Result> = 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, "}}")?; } @@ -649,8 +669,8 @@ impl Display for PerFileIgnores { } } -impl Deref for PerFileIgnores { - type Target = Vec<(GlobMatcher, GlobMatcher, bool, RuleSet)>; +impl Deref for CompiledPerFileIgnoreList { + type Target = Vec; fn deref(&self) -> &Self::Target { &self.ignores diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 0745eb3d70b93..7d175bf673d81 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -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::{ @@ -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()