From 3ac5a9aa31c439096d10e7a64c85baa2efe1cbce Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 22 Dec 2022 16:40:15 -0500 Subject: [PATCH] Respect --force-exclude for files passed via stdin (#1342) --- resources/test/project/README.md | 33 ++++---- src/commands.rs | 19 +++-- src/main.rs | 8 +- src/resolver.rs | 126 +++++++++++++++++++++---------- 4 files changed, 124 insertions(+), 62 deletions(-) diff --git a/resources/test/project/README.md b/resources/test/project/README.md index a521e34cb6a64..978d3d2db6b5e 100644 --- a/resources/test/project/README.md +++ b/resources/test/project/README.md @@ -9,14 +9,14 @@ Running from the repo root should pick up and enforce the appropriate settings f ``` ∴ cargo run resources/test/project/ -Found 7 error(s). resources/test/project/examples/.dotfiles/script.py:1:1: I001 Import block is un-sorted or un-formatted resources/test/project/examples/.dotfiles/script.py:1:8: F401 `numpy` imported but unused resources/test/project/examples/.dotfiles/script.py:2:17: F401 `app.app_file` imported but unused resources/test/project/examples/docs/docs/file.py:1:1: I001 Import block is un-sorted or un-formatted resources/test/project/examples/docs/docs/file.py:8:5: F841 Local variable `x` is assigned to but never used -resources/test/project/src/file.py:1:8: F401 `os` imported but unused -resources/test/project/src/import_file.py:1:1: I001 Import block is un-sorted or un-formatted +resources/test/project/project/file.py:1:8: F401 `os` imported but unused +resources/test/project/project/import_file.py:1:1: I001 Import block is un-sorted or un-formatted +Found 7 error(s). 6 potentially fixable with the --fix option. ``` @@ -24,14 +24,14 @@ Running from the project directory itself should exhibit the same behavior: ``` ∴ (cd resources/test/project/ && cargo run .) -Found 7 error(s). examples/.dotfiles/script.py:1:1: I001 Import block is un-sorted or un-formatted examples/.dotfiles/script.py:1:8: F401 `numpy` imported but unused examples/.dotfiles/script.py:2:17: F401 `app.app_file` imported but unused examples/docs/docs/file.py:1:1: I001 Import block is un-sorted or un-formatted examples/docs/docs/file.py:8:5: F841 Local variable `x` is assigned to but never used -src/file.py:1:8: F401 `os` imported but unused -src/import_file.py:1:1: I001 Import block is un-sorted or un-formatted +project/file.py:1:8: F401 `os` imported but unused +project/import_file.py:1:1: I001 Import block is un-sorted or un-formatted +Found 7 error(s). 6 potentially fixable with the --fix option. ``` @@ -40,9 +40,9 @@ files: ``` ∴ (cd resources/test/project/examples/docs && cargo run .) -Found 2 error(s). docs/file.py:1:1: I001 Import block is un-sorted or un-formatted docs/file.py:8:5: F841 Local variable `x` is assigned to but never used +Found 2 error(s). 1 potentially fixable with the --fix option. ``` @@ -51,8 +51,6 @@ file paths from the current working directory: ``` ∴ (cargo run -- --config=resources/test/project/pyproject.toml resources/test/project/) -Found 11 error(s). -resources/test/project/examples/.dotfiles/script.py:1:1: I001 Import block is un-sorted or un-formatted resources/test/project/examples/.dotfiles/script.py:1:8: F401 `numpy` imported but unused resources/test/project/examples/.dotfiles/script.py:2:17: F401 `app.app_file` imported but unused resources/test/project/examples/docs/docs/concepts/file.py:1:8: F401 `os` imported but unused @@ -61,9 +59,9 @@ resources/test/project/examples/docs/docs/file.py:1:8: F401 `os` imported but un resources/test/project/examples/docs/docs/file.py:3:8: F401 `numpy` imported but unused resources/test/project/examples/docs/docs/file.py:4:27: F401 `docs.concepts.file` imported but unused resources/test/project/examples/excluded/script.py:1:8: F401 `os` imported but unused -resources/test/project/src/file.py:1:8: F401 `os` imported but unused -resources/test/project/src/import_file.py:1:1: I001 Import block is un-sorted or un-formatted -11 potentially fixable with the --fix option. +resources/test/project/project/file.py:1:8: F401 `os` imported but unused +Found 9 error(s). +9 potentially fixable with the --fix option. ``` Running from a parent directory should this "ignore" the `exclude` (hence, `concepts/file.py` gets @@ -71,11 +69,11 @@ included in the output): ``` ∴ (cd resources/test/project/examples && cargo run -- --config=docs/pyproject.toml .) -Found 4 error(s). docs/docs/concepts/file.py:5:5: F841 Local variable `x` is assigned to but never used docs/docs/file.py:1:1: I001 Import block is un-sorted or un-formatted docs/docs/file.py:8:5: F841 Local variable `x` is assigned to but never used excluded/script.py:5:5: F841 Local variable `x` is assigned to but never used +Found 4 error(s). 1 potentially fixable with the --fix option. ``` @@ -83,7 +81,14 @@ Passing an excluded directory directly should report errors in the contained fil ``` ∴ cargo run resources/test/project/examples/excluded/ -Found 1 error(s). resources/test/project/examples/excluded/script.py:1:8: F401 `os` imported but unused +Found 1 error(s). 1 potentially fixable with the --fix option. ``` + +Unless we `--force-exclude`: + +``` +∴ cargo run resources/test/project/examples/excluded/ --force-exclude +∴ cargo run resources/test/project/examples/excluded/script.py --force-exclude +``` diff --git a/src/commands.rs b/src/commands.rs index 25cf801c68c71..d06e2ecb6f9b6 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -115,17 +115,24 @@ fn read_from_stdin() -> Result { /// Run the linter over a single file, read from `stdin`. pub fn run_stdin( filename: Option<&Path>, - strategy: &PyprojectDiscovery, + pyproject_strategy: &PyprojectDiscovery, + file_strategy: &FileDiscovery, + overrides: &Overrides, autofix: fixer::Mode, ) -> Result { - let package_root = filename - .and_then(std::path::Path::parent) - .and_then(packages::detect_package_root); - let stdin = read_from_stdin()?; - let settings = match strategy { + if let Some(filename) = filename { + if !resolver::python_file_at_path(filename, pyproject_strategy, file_strategy, overrides)? { + return Ok(Diagnostics::default()); + } + } + let settings = match pyproject_strategy { PyprojectDiscovery::Fixed(settings) => settings, PyprojectDiscovery::Hierarchical(settings) => settings, }; + let package_root = filename + .and_then(Path::parent) + .and_then(packages::detect_package_root); + let stdin = read_from_stdin()?; let mut diagnostics = lint_stdin(filename, package_root, &stdin, settings, autofix)?; diagnostics.messages.sort_unstable(); Ok(diagnostics) diff --git a/src/main.rs b/src/main.rs index 75bb08edaf89b..fa933e91dffbd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -224,7 +224,13 @@ fn inner_main() -> Result { // Generate lint violations. let diagnostics = if is_stdin { - commands::run_stdin(cli.stdin_filename.as_deref(), &pyproject_strategy, autofix)? + commands::run_stdin( + cli.stdin_filename.as_deref(), + &pyproject_strategy, + &file_strategy, + &overrides, + autofix, + )? } else { commands::run( &cli.files, diff --git a/src/resolver.rs b/src/resolver.rs index 95ad0f609281d..bf103c6b6e921 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -164,8 +164,8 @@ pub fn resolve_settings( /// Return `true` if the given file should be ignored based on the exclusion /// criteria. -fn is_excluded(file_path: &str, file_basename: &str, exclude: &globset::GlobSet) -> bool { - exclude.is_match(file_path) || exclude.is_match(file_basename) +fn match_exclusion(file_path: &str, file_basename: &str, exclusion: &globset::GlobSet) -> bool { + exclusion.is_match(file_path) || exclusion.is_match(file_basename) } /// Return `true` if the `Path` appears to be that of a Python file. @@ -207,35 +207,9 @@ pub fn python_files_in_path( } } - // Omit any paths that are nested within excluded ancestors. This ensures that - // if we pass `subdir/file.py`, and `subdir` is excluded, then we don't - // "miss" that exclusion when walking from `subdir/file.py` below. + // Check if the paths themselves are excluded. if file_strategy.force_exclude { - paths.retain(|path| { - for path in path.ancestors() { - let settings = resolver.resolve(path, pyproject_strategy); - match fs::extract_path_names(path) { - Ok((file_path, file_basename)) => { - if !settings.exclude.is_empty() - && is_excluded(file_path, file_basename, &settings.exclude) - { - debug!("Ignored path via `exclude`: {:?}", path); - return false; - } else if !settings.extend_exclude.is_empty() - && is_excluded(file_path, file_basename, &settings.extend_exclude) - { - debug!("Ignored path via `extend-exclude`: {:?}", path); - return false; - } - } - Err(err) => { - debug!("Ignored path due to error in parsing: {:?}: {}", path, err); - return false; - } - } - } - true - }); + paths.retain(|path| !is_file_excluded(path, &resolver, pyproject_strategy)); if paths.is_empty() { return Ok((vec![], resolver)); } @@ -298,19 +272,23 @@ pub fn python_files_in_path( // Respect our own exclusion behavior. if let Ok(entry) = &result { - if file_strategy.force_exclude || entry.depth() > 0 { + if entry.depth() > 0 { let path = entry.path(); let resolver = resolver.read().unwrap(); let settings = resolver.resolve(path, pyproject_strategy); match fs::extract_path_names(path) { Ok((file_path, file_basename)) => { if !settings.exclude.is_empty() - && is_excluded(file_path, file_basename, &settings.exclude) + && match_exclusion(file_path, file_basename, &settings.exclude) { debug!("Ignored path via `exclude`: {:?}", path); return WalkState::Skip; } else if !settings.extend_exclude.is_empty() - && is_excluded(file_path, file_basename, &settings.extend_exclude) + && match_exclusion( + file_path, + file_basename, + &settings.extend_exclude, + ) { debug!("Ignored path via `extend-exclude`: {:?}", path); return WalkState::Skip; @@ -337,6 +315,72 @@ pub fn python_files_in_path( Ok((files.into_inner().unwrap(), resolver.into_inner().unwrap())) } +/// Return `true` if the Python file at `Path` is _not_ excluded. +pub fn python_file_at_path( + path: &Path, + pyproject_strategy: &PyprojectDiscovery, + file_strategy: &FileDiscovery, + overrides: &Overrides, +) -> Result { + if !file_strategy.force_exclude { + return Ok(true); + } + + // Normalize the path (e.g., convert from relative to absolute). + let path = fs::normalize_path(path); + + // Search for `pyproject.toml` files in all parent directories. + let mut resolver = Resolver::default(); + for ancestor in path.ancestors() { + let pyproject = ancestor.join("pyproject.toml"); + if pyproject.is_file() { + if has_ruff_section(&pyproject)? { + let (root, settings) = + resolve_scoped_settings(&pyproject, &Relativity::Parent, Some(overrides))?; + resolver.add(root, settings); + } + } + } + + // Check exclusions. + Ok(!is_file_excluded(&path, &resolver, pyproject_strategy)) +} + +/// Return `true` if the given top-level `Path` should be excluded. +fn is_file_excluded( + path: &Path, + resolver: &Resolver, + pyproject_strategy: &PyprojectDiscovery, +) -> bool { + // TODO(charlie): Respect gitignore. + for path in path.ancestors() { + if path.file_name().is_none() { + break; + } + let settings = resolver.resolve(path, pyproject_strategy); + match fs::extract_path_names(path) { + Ok((file_path, file_basename)) => { + if !settings.exclude.is_empty() + && match_exclusion(file_path, file_basename, &settings.exclude) + { + debug!("Ignored path via `exclude`: {:?}", path); + return true; + } else if !settings.extend_exclude.is_empty() + && match_exclusion(file_path, file_basename, &settings.extend_exclude) + { + debug!("Ignored path via `extend-exclude`: {:?}", path); + return true; + } + } + Err(err) => { + debug!("Ignored path due to error in parsing: {:?}: {}", path, err); + return true; + } + } + } + false +} + #[cfg(test)] mod tests { use std::path::Path; @@ -346,7 +390,7 @@ mod tests { use path_absolutize::Absolutize; use crate::fs; - use crate::resolver::{is_excluded, is_python_path}; + use crate::resolver::{is_python_path, match_exclusion}; use crate::settings::types::FilePattern; #[test] @@ -383,7 +427,7 @@ mod tests { .to_path_buf(), ); let (file_path, file_basename) = fs::extract_path_names(&path)?; - assert!(is_excluded( + assert!(match_exclusion( file_path, file_basename, &make_exclusion(exclude,) @@ -398,7 +442,7 @@ mod tests { .to_path_buf(), ); let (file_path, file_basename) = fs::extract_path_names(&path)?; - assert!(is_excluded( + assert!(match_exclusion( file_path, file_basename, &make_exclusion(exclude,) @@ -415,7 +459,7 @@ mod tests { .to_path_buf(), ); let (file_path, file_basename) = fs::extract_path_names(&path)?; - assert!(is_excluded( + assert!(match_exclusion( file_path, file_basename, &make_exclusion(exclude,) @@ -430,7 +474,7 @@ mod tests { .to_path_buf(), ); let (file_path, file_basename) = fs::extract_path_names(&path)?; - assert!(is_excluded( + assert!(match_exclusion( file_path, file_basename, &make_exclusion(exclude,) @@ -447,7 +491,7 @@ mod tests { .to_path_buf(), ); let (file_path, file_basename) = fs::extract_path_names(&path)?; - assert!(is_excluded( + assert!(match_exclusion( file_path, file_basename, &make_exclusion(exclude,) @@ -464,7 +508,7 @@ mod tests { .to_path_buf(), ); let (file_path, file_basename) = fs::extract_path_names(&path)?; - assert!(is_excluded( + assert!(match_exclusion( file_path, file_basename, &make_exclusion(exclude,) @@ -481,7 +525,7 @@ mod tests { .to_path_buf(), ); let (file_path, file_basename) = fs::extract_path_names(&path)?; - assert!(!is_excluded( + assert!(!match_exclusion( file_path, file_basename, &make_exclusion(exclude,)