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

Ruff fails when a directory ends with .py #5739

Closed
konstin opened this issue Jul 13, 2023 · 6 comments · Fixed by #5775
Closed

Ruff fails when a directory ends with .py #5739

konstin opened this issue Jul 13, 2023 · 6 comments · Fixed by #5775
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Contributions especially welcome

Comments

@konstin
Copy link
Member

konstin commented Jul 13, 2023

Ruff gets confused when a directory is named like a python file (ends with .py):

$ mkdir repro
$ mkdir repro/some_folder.py
$ cargo run --bin ruff -- --no-cache repro
error: Failed to lint repro/some_folder.py: Is a directory (os error 21)
  Caused by: Is a directory (os error 21)
repro/some_folder.py:1:1: E902 Is a directory (os error 21)
  Caused by: Is a directory (os error 21)
Found 1 error.
@konstin konstin added the bug Something isn't working label Jul 13, 2023
@zanieb
Copy link
Member

zanieb commented Jul 13, 2023

@konstin is this a good first issue?

@charliermarsh
Copy link
Member

(From my perspective, seems like a good issue, will probably require digging into how we collect the "files to lint".)

@zanieb zanieb added the help wanted Contributions especially welcome label Jul 13, 2023
@konstin konstin added the good first issue Good for newcomers label Jul 14, 2023
@konstin
Copy link
Member Author

konstin commented Jul 14, 2023

i've confirmed it's not related to #5611, so yes it's a good first issue

@harupy
Copy link
Contributor

harupy commented Jul 15, 2023

can I work on this?

@harupy
Copy link
Contributor

harupy commented Jul 15, 2023

Is this the part that needs to be fixed?

if result.as_ref().map_or(true, |entry| {
if entry.depth() == 0 {
// Accept all files that are passed-in directly.
entry.file_type().map_or(false, |ft| ft.is_file())
} else {
// Otherwise, check if the file is included.
let path = entry.path();
let resolver = resolver.read().unwrap();
let settings = resolver.resolve(path, pyproject_config);
if settings.include.is_match(path) {
debug!("Included path via `include`: {:?}", path);
true
} else if settings.extend_include.is_match(path) {
debug!("Included path via `extend-include`: {:?}", path);
true
} else {
false
}
}

It looks like the else block is missing is_file check.

@harupy
Copy link
Contributor

harupy commented Jul 15, 2023

Testing the following change:

diff --git a/crates/ruff/src/resolver.rs b/crates/ruff/src/resolver.rs
index 0ad0fa244..0d851135c 100644
--- a/crates/ruff/src/resolver.rs
+++ b/crates/ruff/src/resolver.rs
@@ -330,9 +330,12 @@ pub fn python_files_in_path(
             }
 
             if result.as_ref().map_or(true, |entry| {
+                if !(entry.file_type().map_or(false, |ft| ft.is_file())) {
+                    return false;
+                }
                 if entry.depth() == 0 {
                     // Accept all files that are passed-in directly.
-                    entry.file_type().map_or(false, |ft| ft.is_file())
+                    true
                 } else {
                     // Otherwise, check if the file is included.
                     let path = entry.path();

Result:

> tree dir
dir
├── dir.py
│   └── file.py
└── file.py

1 directory, 2 files

> cargo run -p ruff_cli -- check dir --no-cache
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/ruff check dir --no-cache`
dir/dir.py/file.py:1:7: F821 Undefined name `a`
dir/file.py:1:7: F821 Undefined name `a`
Found 2 errors.

zanieb pushed a commit that referenced this issue Jul 18, 2023
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Fixes #5739

## Test Plan

<!-- How was it tested? -->

Manually tested:

```sh
$ tree dir
dir
├── dir.py
│   └── file.py
└── file.py

1 directory, 2 files

$ cargo run -p ruff_cli -- check dir --no-cache
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/ruff check dir --no-cache`
dir/dir.py/file.py:1:7: F821 Undefined name `a`
dir/file.py:1:7: F821 Undefined name `a`
Found 2 errors.
```

Is a unit test needed?
evanrittenhouse pushed a commit to evanrittenhouse/ruff that referenced this issue Jul 19, 2023
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Fixes astral-sh#5739

## Test Plan

<!-- How was it tested? -->

Manually tested:

```sh
$ tree dir
dir
├── dir.py
│   └── file.py
└── file.py

1 directory, 2 files

$ cargo run -p ruff_cli -- check dir --no-cache
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/ruff check dir --no-cache`
dir/dir.py/file.py:1:7: F821 Undefined name `a`
dir/file.py:1:7: F821 Undefined name `a`
Found 2 errors.
```

Is a unit test needed?
konstin pushed a commit that referenced this issue Jul 19, 2023
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Fixes #5739

## Test Plan

<!-- How was it tested? -->

Manually tested:

```sh
$ tree dir
dir
├── dir.py
│   └── file.py
└── file.py

1 directory, 2 files

$ cargo run -p ruff_cli -- check dir --no-cache
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/ruff check dir --no-cache`
dir/dir.py/file.py:1:7: F821 Undefined name `a`
dir/file.py:1:7: F821 Undefined name `a`
Found 2 errors.
```

Is a unit test needed?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Contributions especially welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants