Skip to content

Commit

Permalink
Respect --force-exclude for files passed via stdin (#1342)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Dec 22, 2022
1 parent 451047c commit 3ac5a9a
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 62 deletions.
33 changes: 19 additions & 14 deletions resources/test/project/README.md
Expand Up @@ -9,29 +9,29 @@ 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.
```

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.
```

Expand All @@ -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.
```

Expand All @@ -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
Expand All @@ -61,29 +59,36 @@ 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
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.
```

Passing an excluded directory directly should report errors in the contained files:

```
∴ 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
```
19 changes: 13 additions & 6 deletions src/commands.rs
Expand Up @@ -115,17 +115,24 @@ fn read_from_stdin() -> Result<String> {
/// 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<Diagnostics> {
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)
Expand Down
8 changes: 7 additions & 1 deletion src/main.rs
Expand Up @@ -224,7 +224,13 @@ fn inner_main() -> Result<ExitCode> {

// 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,
Expand Down
126 changes: 85 additions & 41 deletions src/resolver.rs
Expand Up @@ -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.
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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;
Expand All @@ -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<bool> {
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;
Expand All @@ -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]
Expand Down Expand Up @@ -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,)
Expand All @@ -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,)
Expand All @@ -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,)
Expand All @@ -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,)
Expand All @@ -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,)
Expand All @@ -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,)
Expand All @@ -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,)
Expand Down

0 comments on commit 3ac5a9a

Please sign in to comment.