From d008a181ecad9538ebd0146a1efe95210cda6b75 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 14 Sep 2022 22:21:17 -0400 Subject: [PATCH] Improve default exclusions and support extend-exclude (#188) --- Cargo.toml | 2 +- resources/test/fixtures/pyproject.toml | 2 +- src/fs.rs | 97 ++++++++++++++++++++++---- src/linter.rs | 40 +++++++++++ src/main.rs | 12 +++- src/pyproject.rs | 26 ++++--- src/settings.rs | 59 ++++++++++++---- 7 files changed, 193 insertions(+), 45 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a9bb4e3d5f..a7b119d746 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ common-path = { version = "1.0.0" } dirs = { version = "4.0.0" } fern = { version = "0.6.1" } filetime = { version = "0.2.17" } -glob = { version = "0.3.0" } +glob = "0.3.0" itertools = "0.10.3" log = { version = "0.4.17" } notify = { version = "4.0.17" } diff --git a/resources/test/fixtures/pyproject.toml b/resources/test/fixtures/pyproject.toml index 22e6c7e2a2..4a853c2e3c 100644 --- a/resources/test/fixtures/pyproject.toml +++ b/resources/test/fixtures/pyproject.toml @@ -1,6 +1,6 @@ [tool.ruff] line-length = 88 -exclude = ["excluded.py", "**/migrations"] +extend-exclude = ["excluded.py", "migrations"] select = [ "E402", "E501", diff --git a/src/fs.rs b/src/fs.rs index ef58d40435..b4d39185c9 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -4,34 +4,60 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use glob::Pattern; +use log::debug; use walkdir::{DirEntry, WalkDir}; -fn is_not_hidden(entry: &DirEntry) -> bool { - entry - .file_name() - .to_str() - .map(|s| (entry.depth() == 0 || !s.starts_with('.'))) - .unwrap_or(false) +fn is_excluded(path: &Path, exclude: &[Pattern]) -> bool { + if let Some(file_name) = path.file_name() { + if let Some(file_name) = file_name.to_str() { + for pattern in exclude { + if pattern.matches(file_name) { + return true; + } + } + false + } else { + false + } + } else { + false + } } -fn is_not_excluded(entry: &DirEntry, exclude: &[Pattern]) -> bool { - entry - .path() - .to_str() - .map(|s| !exclude.iter().any(|pattern| pattern.matches(s))) - .unwrap_or(false) +fn is_included(path: &Path) -> bool { + let file_name = path.to_string_lossy(); + file_name.ends_with(".py") || file_name.ends_with(".pyi") } pub fn iter_python_files<'a>( path: &'a PathBuf, exclude: &'a [Pattern], + extend_exclude: &'a [Pattern], ) -> impl Iterator + 'a { WalkDir::new(path) .follow_links(true) .into_iter() - .filter_entry(|entry| is_not_hidden(entry) && is_not_excluded(entry, exclude)) + .filter_entry(|entry| { + if exclude.is_empty() && extend_exclude.is_empty() { + return true; + } + + let path = entry.path(); + if is_excluded(path, exclude) { + debug!("Ignored path via `exclude`: {:?}", path); + false + } else if is_excluded(path, extend_exclude) { + debug!("Ignored path via `extend-exclude`: {:?}", path); + false + } else { + true + } + }) .filter_map(|entry| entry.ok()) - .filter(|entry| entry.path().to_string_lossy().ends_with(".py")) + .filter(|entry| { + let path = entry.path(); + is_included(path) + }) } pub fn read_file(path: &Path) -> Result { @@ -41,3 +67,46 @@ pub fn read_file(path: &Path) -> Result { buf_reader.read_to_string(&mut contents)?; Ok(contents) } + +#[cfg(test)] +mod tests { + use std::path::Path; + + use glob::Pattern; + + use crate::fs::{is_excluded, is_included}; + + #[test] + fn inclusions() { + let path = Path::new("foo/bar/baz.py"); + assert!(is_included(path)); + + let path = Path::new("foo/bar/baz.pyi"); + assert!(is_included(path)); + + let path = Path::new("foo/bar/baz.js"); + assert!(!is_included(path)); + + let path = Path::new("foo/bar/baz"); + assert!(!is_included(path)); + } + + #[test] + fn exclusions() { + let path = Path::new("foo"); + let exclude = vec![Pattern::new("foo").unwrap()]; + assert!(is_excluded(path, &exclude)); + + let path = Path::new("foo/bar"); + let exclude = vec![Pattern::new("bar").unwrap()]; + assert!(is_excluded(path, &exclude)); + + let path = Path::new("foo/bar/baz.py"); + let exclude = vec![Pattern::new("baz.py").unwrap()]; + assert!(is_excluded(path, &exclude)); + + let path = Path::new("foo/bar/baz.py"); + let exclude = vec![Pattern::new("baz").unwrap()]; + assert!(!is_excluded(path, &exclude)); + } +} diff --git a/src/linter.rs b/src/linter.rs index b5a8984be8..25751667bb 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -96,6 +96,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E402]), }, &fixer::Mode::Generate, @@ -112,6 +113,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E501]), }, &fixer::Mode::Generate, @@ -128,6 +130,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E711]), }, &fixer::Mode::Generate, @@ -144,6 +147,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E712]), }, &fixer::Mode::Generate, @@ -160,6 +164,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E713]), }, &fixer::Mode::Generate, @@ -176,6 +181,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E721]), }, &fixer::Mode::Generate, @@ -192,6 +198,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E722]), }, &fixer::Mode::Generate, @@ -208,6 +215,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E714]), }, &fixer::Mode::Generate, @@ -224,6 +232,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E731]), }, &fixer::Mode::Generate, @@ -240,6 +249,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E741]), }, &fixer::Mode::Generate, @@ -256,6 +266,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E742]), }, &fixer::Mode::Generate, @@ -272,6 +283,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::E743]), }, &fixer::Mode::Generate, @@ -288,6 +300,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F401]), }, &fixer::Mode::Generate, @@ -304,6 +317,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F403]), }, &fixer::Mode::Generate, @@ -320,6 +334,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F404]), }, &fixer::Mode::Generate, @@ -336,6 +351,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F406]), }, &fixer::Mode::Generate, @@ -352,6 +368,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F407]), }, &fixer::Mode::Generate, @@ -368,6 +385,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F541]), }, &fixer::Mode::Generate, @@ -384,6 +402,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F601]), }, &fixer::Mode::Generate, @@ -400,6 +419,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F602]), }, &fixer::Mode::Generate, @@ -416,6 +436,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F622]), }, &fixer::Mode::Generate, @@ -432,6 +453,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F631]), }, &fixer::Mode::Generate, @@ -448,6 +470,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F632]), }, &fixer::Mode::Generate, @@ -464,6 +487,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F633]), }, &fixer::Mode::Generate, @@ -480,6 +504,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F634]), }, &fixer::Mode::Generate, @@ -496,6 +521,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F701]), }, &fixer::Mode::Generate, @@ -512,6 +538,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F702]), }, &fixer::Mode::Generate, @@ -528,6 +555,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F704]), }, &fixer::Mode::Generate, @@ -544,6 +572,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F706]), }, &fixer::Mode::Generate, @@ -560,6 +589,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F707]), }, &fixer::Mode::Generate, @@ -576,6 +606,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F722]), }, &fixer::Mode::Generate, @@ -592,6 +623,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F821]), }, &fixer::Mode::Generate, @@ -608,6 +640,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F822]), }, &fixer::Mode::Generate, @@ -624,6 +657,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F823]), }, &fixer::Mode::Generate, @@ -640,6 +674,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F831]), }, &fixer::Mode::Generate, @@ -656,6 +691,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F841]), }, &fixer::Mode::Generate, @@ -672,6 +708,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F901]), }, &fixer::Mode::Generate, @@ -688,6 +725,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::R001]), }, &fixer::Mode::Generate, @@ -704,6 +742,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::R002]), }, &fixer::Mode::Generate, @@ -720,6 +759,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + extend_exclude: vec![], select: BTreeSet::from([CheckCode::F401, CheckCode::F821]), }, &fixer::Mode::Generate, diff --git a/src/main.rs b/src/main.rs index 1ef8f30db2..dfa94cd205 100644 --- a/src/main.rs +++ b/src/main.rs @@ -54,9 +54,12 @@ struct Cli { /// List of error codes to ignore. #[clap(long, multiple = true)] ignore: Vec, - /// List of file and/or directory patterns to exclude from checks. + /// List of paths, used to exclude files and/or directories from checks. #[clap(long, multiple = true)] exclude: Vec, + /// Like --exclude, but adds additional files and directories on top of the excluded ones. + #[clap(long, multiple = true)] + extend_exclude: Vec, } #[cfg(feature = "update-informer")] @@ -93,7 +96,7 @@ fn run_once( let start = Instant::now(); let paths: Vec = files .iter() - .flat_map(|path| iter_python_files(path, &settings.exclude)) + .flat_map(|path| iter_python_files(path, &settings.exclude, &settings.extend_exclude)) .collect(); let duration = start.elapsed(); debug!("Identified files to lint in: {:?}", duration); @@ -191,7 +194,10 @@ fn inner_main() -> Result { settings.ignore(&cli.ignore); } if !cli.exclude.is_empty() { - settings.exclude(cli.exclude); + settings.exclude = cli.exclude; + } + if !cli.extend_exclude.is_empty() { + settings.extend_exclude = cli.extend_exclude; } if cli.watch { diff --git a/src/pyproject.rs b/src/pyproject.rs index 48b09c71fa..25b1845f26 100644 --- a/src/pyproject.rs +++ b/src/pyproject.rs @@ -8,19 +8,16 @@ use serde::Deserialize; use crate::checks::CheckCode; use crate::fs; -pub fn load_config(paths: &[PathBuf]) -> (PathBuf, Config) { +pub fn load_config(paths: &[PathBuf]) -> Config { match find_project_root(paths) { Some(project_root) => match find_pyproject_toml(&project_root) { Some(path) => { - debug!("Found pyproject.toml at: {}", path.to_string_lossy()); + debug!("Found pyproject.toml at: {:?}", path); match parse_pyproject_toml(&path) { - Ok(pyproject) => { - let config = pyproject - .tool - .and_then(|tool| tool.ruff) - .unwrap_or_default(); - (project_root, config) - } + Ok(pyproject) => pyproject + .tool + .and_then(|tool| tool.ruff) + .unwrap_or_default(), Err(e) => { println!("Failed to load pyproject.toml: {:?}", e); println!("Falling back to default configuration..."); @@ -39,6 +36,7 @@ pub fn load_config(paths: &[PathBuf]) -> (PathBuf, Config) { pub struct Config { pub line_length: Option, pub exclude: Option>, + pub extend_exclude: Option>, pub select: Option>, pub ignore: Option>, } @@ -123,6 +121,7 @@ mod tests { ruff: Some(Config { line_length: None, exclude: None, + extend_exclude: None, select: None, ignore: None, }) @@ -142,6 +141,7 @@ line-length = 79 ruff: Some(Config { line_length: Some(79), exclude: None, + extend_exclude: None, select: None, ignore: None, }) @@ -161,6 +161,7 @@ exclude = ["foo.py"] ruff: Some(Config { line_length: None, exclude: Some(vec![Path::new("foo.py").to_path_buf()]), + extend_exclude: None, select: None, ignore: None, }) @@ -180,6 +181,7 @@ select = ["E501"] ruff: Some(Config { line_length: None, exclude: None, + extend_exclude: None, select: Some(vec![CheckCode::E501]), ignore: None, }) @@ -199,6 +201,7 @@ ignore = ["E501"] ruff: Some(Config { line_length: None, exclude: None, + extend_exclude: None, select: None, ignore: Some(vec![CheckCode::E501]), }) @@ -255,9 +258,10 @@ other-attribute = 1 config, Config { line_length: Some(88), - exclude: Some(vec![ + exclude: None, + extend_exclude: Some(vec![ Path::new("excluded.py").to_path_buf(), - Path::new("**/migrations").to_path_buf() + Path::new("migrations").to_path_buf() ]), select: Some(vec![ CheckCode::E402, diff --git a/src/settings.rs b/src/settings.rs index 502d5a84f5..ee565532d3 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -3,6 +3,7 @@ use std::hash::{Hash, Hasher}; use std::path::PathBuf; use glob::Pattern; +use once_cell::sync::Lazy; use crate::checks::CheckCode; use crate::pyproject::load_config; @@ -11,6 +12,7 @@ use crate::pyproject::load_config; pub struct Settings { pub line_length: usize, pub exclude: Vec, + pub extend_exclude: Vec, pub select: BTreeSet, } @@ -22,25 +24,56 @@ impl Hash for Settings { } } } +static DEFAULT_EXCLUDE: Lazy> = Lazy::new(|| { + vec![ + Pattern::new(".bzr").unwrap(), + Pattern::new(".direnv").unwrap(), + Pattern::new(".eggs").unwrap(), + Pattern::new(".git").unwrap(), + Pattern::new(".hg").unwrap(), + Pattern::new(".mypy_cache").unwrap(), + Pattern::new(".nox").unwrap(), + Pattern::new(".pants.d").unwrap(), + Pattern::new(".svn").unwrap(), + Pattern::new(".tox").unwrap(), + Pattern::new(".venv").unwrap(), + Pattern::new("__pypackages__").unwrap(), + Pattern::new("_build").unwrap(), + Pattern::new("buck-out").unwrap(), + Pattern::new("build").unwrap(), + Pattern::new("dist").unwrap(), + Pattern::new("node_modules").unwrap(), + Pattern::new("venv").unwrap(), + ] +}); impl Settings { pub fn from_paths(paths: &[PathBuf]) -> Self { - let (project_root, config) = load_config(paths); + let config = load_config(paths); let mut settings = Settings { line_length: config.line_length.unwrap_or(88), exclude: config .exclude - .unwrap_or_default() - .into_iter() - .map(|path| { - if path.is_relative() { - project_root.join(path) - } else { - path - } + .map(|paths| { + paths + .into_iter() + .map(|path| { + Pattern::new(&path.to_string_lossy()).expect("Invalid pattern.") + }) + .collect() }) - .map(|path| Pattern::new(&path.to_string_lossy()).expect("Invalid pattern.")) - .collect(), + .unwrap_or_else(|| DEFAULT_EXCLUDE.clone()), + extend_exclude: config + .extend_exclude + .map(|paths| { + paths + .into_iter() + .map(|path| { + Pattern::new(&path.to_string_lossy()).expect("Invalid pattern.") + }) + .collect() + }) + .unwrap_or_default(), select: BTreeSet::from_iter(config.select.unwrap_or_else(|| { vec![ CheckCode::E402, @@ -105,8 +138,4 @@ impl Settings { self.select.remove(code); } } - - pub fn exclude(&mut self, exclude: Vec) { - self.exclude = exclude; - } }