astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
33k stars 1.1k forks source link

[Infinite loop] ICN001 conflicts with I002 and F401 if an unused unconventionally aliased import is required #14389

Open dscorbett opened 6 days ago

dscorbett commented 6 days ago

unconventional-import-alias (ICN001) conflicts with missing-required-import (I002) in Ruff 0.7.4 when unused-import (F401) is also enabled, if an import is required without a conventional alias but is unused. ICN001’s fix should be skipped for required imports, even if the required imports lack conventional aliases. It would still be helpful for ICN001 to report a violation without a fix.

$ echo 1 | ruff check --isolated --select I002,ICN001,F401 --config 'lint.isort.required-imports = ["import pandas"]' - --unsafe-fixes --fix

error: Failed to converge after 100 iterations.

This indicates a bug in Ruff. If you could open an issue at:

    https://github.com/astral-sh/ruff/issues/new?title=%5BInfinite%20loop%5D

...quoting the contents of `-`, the rule codes ICN001, along with the `pyproject.toml` settings and executed command, we'd be very appreciative!

import pandas
1
-:1:8: ICN001 `pandas` should be imported as `pd`
  |
1 | import pandas
  |        ^^^^^^ ICN001
2 | 1
  |
  = help: Alias `pandas` to `pd`

Found 101 errors (100 fixed, 1 remaining).
[*] 1 fixable with the --fix option.

In that example, I002 inserts import pandas, ICN001 changes it to import pandas as pd, I002 inserts import pandas again, F401 removes the unused import pandas as pd, and then back to ICN001 and so on.

dylwil3 commented 5 days ago

I'm wondering if, unlike the previous infinite-loop fix, this should be fixed by giving an error directly on the user's configuration? Especially in the case where the user has entered in both the required import and the required alias.

AlexWaygood commented 5 days ago

I'm wondering if, unlike the previous infinite-loop fix, this should be fixed by giving an error directly on the user's configuration? Especially in the case where the user has entered in both the required import and the required alias.

Yes, that makes sense to me! These two configuration options just flatly contradict each other in this case, I think.