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

Exclude failure for tool.ruff[lint|format] #8241

Closed
dhruvmanila opened this issue Oct 26, 2023 · 9 comments
Closed

Exclude failure for tool.ruff[lint|format] #8241

dhruvmanila opened this issue Oct 26, 2023 · 9 comments
Labels
bug Something isn't working cli Related to the command-line interface

Comments

@dhruvmanila
Copy link
Member

Detected here: #8240 (comment)

With the following directory structure:

.
├── pyproject.toml [1]
└── tests
    └── packages
        └── test-bad-syntax
            └── pyproject.toml [2]

And, in pyproject.toml at

[1]

[tool.ruff.lint]
exclude = ["tests/packages/test-bad-syntax"]

[2]

[build-system]
requires = ['bad' 'syntax']

Running the command:

$ ruff check .  
ruff failed
  Cause: TOML parse error at line 2, column 19
  |
2 | requires = ['bad' 'syntax']
  |                   ^
invalid array
expected `]`

With:

$ ruff version
ruff 0.1.2 (3127c79b2 2023-10-24)

It works with [tool.ruff]. The main branch fails as well.

@dhruvmanila dhruvmanila added the bug Something isn't working label Oct 26, 2023
@MichaReiser
Copy link
Member

MichaReiser commented Oct 26, 2023

I think the issue here is that exclude and lint.exclude are semantically different.

  • exclude: Excludes files from discovery.
  • lint.exclude: Excludes files from linting.

The proper fix is to change their configuration to use exclude instead of lint.exclude.

@MichaReiser
Copy link
Member

See: pypa/build#697 I asked if we could improve our documentation to make the distinction more clear.

@MichaReiser MichaReiser added bug Something isn't working cli Related to the command-line interface and removed bug Something isn't working labels Oct 26, 2023
@charliermarsh
Copy link
Member

But it still feels like lint.exclude should respect the same semantics for exclusion, no?

@MichaReiser
Copy link
Member

But it still feels like lint.exclude should respect the same semantics for exclusion, no?

I wouldn't know how that would work in a unified check command. The way it would work there is:

  • Ruff detects all files, only respecting exclude
  • Ruff calls lint for files not in exclude or lint.exclude
  • Ruff calls format for files not in exclude or format.exclude

Applying the same semantics in lint.exclude would mean that it is necessary to run file discovery twice, once for exclude merged with lint.exclude and once with format merged with format.exclude. This would remove all benefits of having a single toolchain because we need to perform file discovery, parsing, setting resolution, etc. twice.

@charliermarsh
Copy link
Member

I don't know, but isn't it strange that setting lint.exclude to exclude = ["tests/packages/test-bad-syntax"] doesn't exclude files in ./tests/packages/test-bad-syntax from linting?

@MichaReiser
Copy link
Member

I don't know, but isn't it strange that setting lint.exclude to exclude = ["tests/packages/test-bad-syntax"] doesn't exclude files in ./tests/packages/test-bad-syntax from linting?

It does exclude files in test-bad-syntax from linting. The reported problem is that test-bad-syntax contains a pyproject.toml with a syntax error and ruff tries to load it when discovering the python files of the project when using lint.exclude but it does not when adding the directory to exclude.

@charliermarsh
Copy link
Member

Hmm, are you sure? I don't believe that's the case, because we're only testing against the file itself, and not its parent directories.

Given:

[tool.ruff.lint]
exclude = ["tests/packages/test-bad-syntax"]

Running:

❯ cargo run -p ruff_cli -- check .
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
     Running `/Users/crmarsh/workspace/ruff/target/debug/ruff check .`
warning: Detected debug build without --no-cache.
tests/packages/test-bad-syntax/foo.py:1:8: F401 [*] `os` imported but unused
Found 1 error.

If I change the pyproject.toml to:

[tool.ruff.lint]
exclude = ["tests/packages/test-bad-syntax/*.py"]

Then:

❯ cargo run -p ruff_cli -- check .
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
     Running `/Users/crmarsh/workspace/ruff/target/debug/ruff check .`
warning: Detected debug build without --no-cache.

@MichaReiser
Copy link
Member

Hmm interesting. Let's create a new issue for this because this is unrelated to the original report.

#8267

@charliermarsh
Copy link
Member

(Sorry, I think I misunderstood the root cause of the reported issue.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli Related to the command-line interface
Projects
None yet
Development

No branches or pull requests

3 participants