astral-sh / ruff-pre-commit

A pre-commit hook for Ruff.
Apache License 2.0
802 stars 38 forks source link

Ruff configuration from pyproject.toml is ignored #54

Open beajifu opened 9 months ago

beajifu commented 9 months ago

Hi,

I'm trying to use the pre-commit hook for Ruff, but it ignores my settings from the pyproject.toml. I want Ruff to ignore the line-length but if I commit a file with lines that exceed this limit it is complaining about it. If I use a ruff.toml to configure Ruff it works as expected.

I use version 0.0.292.

My settings in .pre-commit-config.yaml:

# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
repos:
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v3.2.0
    hooks:
    -   id: end-of-file-fixer
    -   id: check-added-large-files
-   repo: https://github.com/charliermarsh/ruff-pre-commit
    rev: v0.0.292
    hooks:
    -   id: ruff

Ruff configuration part in my pyproject.toml (same is used in the ruff.toml)

[tool.ruff]
select = ["E"]

# Never enforce `E501` (line length violations).
ignore = ["E501"]
charliermarsh commented 9 months ago

Hmm, a bit of a tough one to debug. When you switch between pyproject.toml and ruff.toml, are the files located in the same directory?

beajifu commented 9 months ago

Yes they are in the same directory.

I tried it on another laptop with a small example project and this problem did not occur. I try to find a way to reproduce the problem and give you an update.

beajifu commented 9 months ago

I could reproduce the problem and find a solution. If I use a project with already added code and a pyproject.toml to the git repo and then

When I first commit the .pre-commit-config.yaml and the pyproject.toml and after that commit the file with the long line then ruff ignores it as expected. Is this behavior expected?

klow68 commented 9 months ago

same issue with v0.1.0 but committing the config doesn't change anything

repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.1.0
    hooks:
      - id: ruff
        args: [--select=C901, --select=E501]

by default line-length is not checked, seems strange no ?

charliermarsh commented 9 months ago

@klow68 - We removed E501 from the default configuration in v0.1.0 (https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md#breaking-changes), so I think that's working as intended.

dhruvmanila commented 8 months ago

@beajifu I'm unable to reproduce with the steps mentioned although I'm using F401 as an example rule.

Gist: https://gist.github.com/dhruvmanila/8c796087330cb22699f3b20e929fde45

.
├── .pre-commit-config.yaml
├── pyproject.toml
└── test.py

Steps

  1. Install pre-commit and hooks
  2. Comment out the ignore in config file
    
    $ pre-commit run
    ruff.....................................................................Failed
    - hook id: ruff
    - exit code: 1

test.py:1:8: F401 [] os imported but unused Found 1 error. [] 1 potentially fixable with the --fix option.

3. Uncomment the `ignore` in config file
```console
$ pre-commit run
ruff.....................................................................Passed
rpop0 commented 7 months ago

My configuration in pyproject.toml also seems to be having an issue.

Running ruff without any config: 3 errors

Running ruff with 3 select = ["ALL"] in pyproject.toml: 389 errors

Running ruff through pre-commit: 89 errors.

It seems like running ruff through pre-commit is using more rules than default but ignoring the rules in pyproject.toml

rytilahti commented 7 months ago

Chiming in just in case it may help someone. In my case, clearing the precommit cache ($HOME/.cache/pre-commit/) fixed a similarly sounding issue some time ago. Alas, I didn't keep the cache so I couldn't debug it further.

caerulescens commented 6 months ago

This still seems to be an issue; using v0.1.7

charliermarsh commented 6 months ago

Would really appreciate if you could share some more information -- we weren't able to reproduce it above and it doesn't seem to be a common issue, so we need some help. What behavior are you seeing? What commands are you running? How is your project structured? Have you tried clearing the cache, as above? Thank you in advance!

caerulescens commented 6 months ago

It seems I have more than one project reliably producing this bug (even after clearing caches), and I constructed a new project as a minimal example for @charliermarsh to reproduce the bug reliably. The new project I made ended up reading the pyproject.toml correctly, and I checked by causing N999: Invalid module name and then ignoring the rule: ignore = ["N999"] on a separate run. The bugged and new project that I compared both use ruff-pre-commit v0.1.7 and were invoked the same way. I don't think it's related to the version, but if I spend some more time I can isolate the issue.

rafalkrupinski commented 5 months ago
[tool.ruff]
select = ["E"]

# Never enforce `E501` (line length violations).
ignore = ["E501"]

don't these go in tool.ruff.lint?

caerulescens commented 5 months ago

@rafalkrupinski I double checked docs, and you're right. I have been busy with other tasks, but I saved this thread for later. I'll retest, and repost.

charliermarsh commented 5 months ago

@caerulescens - Both are accepted right now -- putting ignore under [tool.ruff] and [tool.ruff.lint] are interchangeable. (We're moving towards the latter, but they both work as-is.)

caerulescens commented 5 months ago

I realized the same thing just now; I'll retest

caerulescens commented 5 months ago

I re-verified that I can [still] produce this bug with projects I'm working on; the [tool.ruff] section is being used. To isolate this bug, I'll probably copy/paste the repository locally, and I'll just start removing things until I can't reproduce the bug anymore, and then I'll post the results here. Earlier I tried to construct the minimum example from scratch, and I wasn't successful.

Quatters commented 5 months ago

@charliermarsh just created repo in which pre-commit hook ignores pyproject.toml config unlike the ruff itself. Hope this would help. https://github.com/Quatters/ruff-pre-commit-bug-demo

dhruvmanila commented 5 months ago

@Quatters Hi, thanks for providing the repository for the problem you're facing. But, I don't think it's a bug with Ruff or pre-commit, I've described in detail about what's happening here: https://github.com/astral-sh/ruff/issues/9696#issuecomment-1916480913.

One solution would be to use pass_filenames: false in the pre-commit config so that the file discovery is done by Ruff taking into account the includes and excludes configured by the user in their pyproject.toml.

repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.1.15
    hooks:
      - id: ruff
        pass_filenames: false

This solves your problem.

That said, maybe Ruff's pre-commit hook should do that by default?

Quatters commented 5 months ago

@dhruvmanila thank you for the quick response. Yes, providing pass_filenames: false helped in my case.

That said, maybe Ruff's pre-commit hook should do that by default?

I'm not sure about doing that by default (will there be any side effects/breaking changes?). Anyway, I think information about pass_filenames would be useful in README.

rafalkrupinski commented 5 months ago

One solution would be to use pass_filenames: false in the pre-commit config so that the file discovery is done by Ruff taking into account the includes and excludes configured by the user in their pyproject.toml. That said, maybe Ruff's pre-commit hook should do that by default?

Wouldn't that make ruff to check all files every time? I think pre-commit users expect it to only check changed files, even if it is fast.

Even if pre-commit passes filenames to ruff check, I still don't see why it wouldn't reed the configuration...

dougthor42 commented 5 months ago

Agreed, running with pass_filenames: False is not a long term solution. Pre-commit needs to be the decider on what files get passed. This is one way of slowly fixing a large project - only touched files get linted/formatted.

It does help narrow down the issue though, good find!

caerulescens commented 5 months ago

@dhruvmanila That is not a usable work around for the reasons that @dougthor42 mentions.


@charliermarsh I played around with this issue tonight, and I have discovered more behavior surrounding this issue. I did something similar to the squeeze theorem but with code; I constructed a minimal project that doesn't have the issue, and I have a fully featured project with the issue (for the same rule). I've been removing parts of the larger project until I identify the issue.

pyproject.toml observations:

  1. If [tool.ruff.lint] is present, then all settings in [tool.ruff] are overridden; I'd imagine this is expected as [tool.ruff.lint] is set to overtake [tool.ruff]. Meaning, ignore = [] in [tool.ruff.lint] overrides all specified rules for ignore in [tool.ruff].
  2. I have no issue with ignoring D rules in [tool.ruff] or [tool.ruff.lint], but N999 is getting ignored regardless. Meaning, I have code that will invoke an error for D100, D104, and N999.

    [tool.ruff]
    line-length = 88  # Not relevant
    target-version = "py310"  # Not relevant
    src = ["src", "tests", "docs"]  # Not relevant
    select = ["F", "E", "W", "C90", "S", "I", "N", "D", "UP"]
    ignore = ["D100", "D104", "N999"]
    
    [tool.ruff.pydocstyle]
    convention = "pep257"  # Not relevant

    Will produce:

    {{ cookiecutter.repository_name }}/src/{{ cookiecutter.package_name }}/__init__.py:1:1: N999 Invalid module name: '{{ cookiecutter.package_name }}'
    Found 1 error.

    Then, removing rule D100 from ignore in [tool.ruff] or [tool.ruff.lint] will produce:

    docs/conf.py:1:1: D100 Missing docstring in public module
    tests/conftest.py:1:1: D100 Missing docstring in public module
    tests/test_cookiecutter_terragrunt.py:1:1: D100 Missing docstring in public module
    {{ cookiecutter.repository_name }}/src/{{ cookiecutter.package_name }}/__init__.py:1:1: N999 Invalid module name: '{{ cookiecutter.package_name }}'
    Found 4 errors.

    This means that [tool.ruff] and [tool.ruff.lint] within pyproject.toml is getting read by pre-commit; this seems to be a specific issue that can be replicated with N999 but not maybe not other rules. This also means that [tool.ruff] and [tool.ruff.lint] suffer from the same problem, so the code that's causing the issue is shared by [tool.ruff] and [tool.ruff.lint], which probably doesn't narrow down much.

  3. I have a minimal project that can correctly ignore N999, yet it has the same pyproject.toml settings seen in (2).
caerulescens commented 5 months ago

@charliermarsh I think I solved it! The problem occurs when there's another pyproject.toml within the project. The problem within the larger project was solved when I removed the second pyproject.toml within {{ cookiecutter.repository_name }}, which is used by cookiecutter. It seems that ruff prioritized [tool.ruff] in {{ cookiecutter.repository_name }}/pyproject.toml over the top-level pyproject.toml. This explains why this issue could be difficult to reproduce; it requires having multiple configurations for ruff present. I had the same D rules ignored in {{ cookiecutter.repository_name }}/pyproject.toml as the top-level pyproject.toml, which was misleading me during the debugging process.

This issue can be closed as ruff does read the pyproject.toml correctly. Maybe it would be helpful to note this behavior in the documentation if it's not already there? I think it could be useful to raise a warning when multiple configurations are found, and indicate that it could lead to unexpected behavior.

rpop0 commented 5 months ago

@charliermarsh I think I solved it! The problem occurs when there's another pyproject.toml within the project. The problem within the larger project was solved when I removed the second pyproject.toml within {{ cookiecutter.repository_name }}, which is used by cookiecutter. It seems that ruff prioritized [tool.ruff] in {{ cookiecutter.repository_name }}/pyproject.toml over the top-level pyproject.toml. This explains why this issue could be difficult to reproduce; it requires having multiple configurations for ruff present. I had the same D rules ignored in {{ cookiecutter.repository_name }}/pyproject.toml as the top-level pyproject.toml, which was misleading me during the debugging process.

This issue can be closed as ruff does read the pyproject.toml correctly. Maybe it would be helpful to note this behavior in the documentation if it's not already there? I think it could be useful to raise a warning when multiple configurations are found, and indicate that it could lead to unexpected behavior.

This happens to me and I have a single pyproject.toml file

caerulescens commented 5 months ago

@rpop0 Could you elaborate further? This issue doesn't seem to be a problem; do you have other types of configurations present like ruff.toml? I identified my situation as user error, and I confirmed the precedence that ruff uses for reading settings from the pyproject.toml.

rpop0 commented 5 months ago

@rpop0 Could you elaborate further? This issue doesn't seem to be a problem; do you have other types of configurations present like ruff.toml? I identified my situation as user error, and I confirmed the precedence that ruff uses for reading settings from the pyproject.toml.

No other types of configurations besides the single pyproject.toml file.

caerulescens commented 5 months ago

@rpop0 How are you reproducing this bug? Could you post a minimal example repository for us or maybe zip a folder and attach here?

caerulescens commented 5 months ago

At the very least, the title for this issue is incorrect because [tool.ruff] and [tool.ruff.lint] within the pyproject.toml is getting read by pre-commit.

wiseyoungbuck commented 4 months ago

I have been having the same issue where the ruff pre-commit hook was ignoring the line-length setting. I only had a single pyproject.toml file and I had the same behavior with a single ruff.toml file. The pass_filenames: false solution would allow the formatter to work properly, but then it would run on all files instead of only running on the newly committed changes.

I am using VSCode and the Ruff VSCode extension. Console commands in VSCode were working fine, but pre-commit would undo the formatting done by the VSCode extension.

When I disabled the VSCode extension, the problematic behavior went away and the precommit behavior starting working as expected. I was also able to reenable the Ruff VSCode extension and the issue has not yet returned. I hope this is helpful.

ezeparziale commented 2 months ago

I have the same problem that ruff-pre-commit does not load ruff config from pyproject.toml.

I resolved it by adding the config in the args.

  - repo: https://github.com/astral-sh/ruff-pre-commit
    # Ruff version.
    rev: v0.4.3
    hooks:
      # Run the linter.
      - id: ruff
        args: ["--select", "E,W,F,I,C,B,UP", "--ignore", "E203,B008,C901"]
      # Run the formatter.
      - id: ruff-format
mazzma12 commented 2 months ago

I have the same problem that ruff-pre-commit does not load ruff config from pyproject.toml.

I resolved it by adding the config in the args.

  - repo: https://github.com/astral-sh/ruff-pre-commit
    # Ruff version.
    rev: v0.4.3
    hooks:
      # Run the linter.
      - id: ruff
        args: ["--select", "E,W,F,I,C,B,UP", "--ignore", "E203,B008,C901"]
      # Run the formatter.
      - id: ruff-format

Same here, but more explicitly for the --exclude config which is not accounted for

artefactop commented 3 weeks ago

I had the same problem and I fixed it with this config, passing the config file to the args explicitly.

repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
  # Ruff version.
  rev: v0.4.5
  hooks:
    # Run the linter.
    - id: ruff
      args: [ --fix, --config=pyproject.toml ]
    # Run the formatter.
    - id: ruff-format
NIvo172 commented 5 days ago

If you think your config is ignored. Add --verbose to the pre-commit hook.

- repo: https://github.com/astral-sh/ruff-pre-commit
  # Ruff: Formatter and Linter.
  rev: v0.5.1
  hooks:
    - id: ruff-format
      args: [--verbose, ]
    - id: ruff
      args: [--verbose, --fix,]

Then you can run pre-commit run --verbose --all-files

The output for me is:

ruff-format..............................................................Passed
- hook id: ruff-format
- duration: 0.08s

[ruff::resolve][DEBUG] Using configuration file (via parent) at: SOMEPATH/pyproject_template\pyproject.toml
ezeparziale commented 3 days ago

I upgraded from 0.4.5 to 0.5.1, and Ruff detects pyproject.toml automatically.

# See https://pre-commit.com for more information
default_language_version:
  python: python3.12
repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    # Ruff version.
    rev: v0.5.1
    hooks:
      # Run the linter.
      - id: ruff
        args: [--fix]
      # Run the formatter.
      - id: ruff-format