Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect --force-exclude for files passed via stdin #1342

Merged
merged 2 commits into from Dec 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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