search

astral-sh / ruff

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

[Infinite loop] Parser error with I002 (required imports) and F401 (unused import) #12458

Closed AA-Turner closed 1 month ago

AA-Turner commented 2 months ago

I ran into a fun error:

PS> ruff -V
ruff 0.5.4
PS> ruff check --isolated --select I002,F401 --fix --unsafe-fixes --config "lint.isort.required-imports=['from path import Path']" bug.py

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 `bug.py`, the rule codes I002, along with the `pyproject.toml` settings and executed command, we'd be very appreciative!

bug.py:1:1: I002 Missing required import: `from path import Path`
Found 101 errors (100 fixed, 1 remaining).
[*] 1 fixable with the --fix option.

PS> type bug.py
_ = 'Path'

The content of bug.py is "_ = 'Path'\n". If you remove the assignment, all works (one imagines as the string is parsed as a docstring).

_ = 'Path'

The error exists going back at least as far as 0.2.2 (0.2.1 gives errors with mixing --isolated and --config).

I can't see any similar issues, but if I've missed one please close this!

A

charliermarsh commented 2 months ago

Thanks, this makes sense. Will fix.

charliermarsh commented 2 months ago

It's kind of an obvious failure, whoops -- but it's never come up because "required imports" is typically used for from __future imports, which are never marked as unused.

AA-Turner commented 2 months ago

As background, this came up in the context of moving Sphinx to pathlib globally (or at least attempting to). I wanted to add from pathlib import Path to every module to avoid undefined name errors when doing simple regex-based refactorings, but when I ran ruff check . --fix I had this error.

I'm not sure which of the required import or the undefined name should take priority (instict says the required import, as the user has requested it), but hopefully Ruff shouldn't crash!

Thank you for the quick response as always Charlie!

Best, Adam

charliermarsh commented 2 months ago

Totally. That's a clever trick! (I'm guessing you can workaround by running I002 first with the required-import, then F401 second, without the required-import?)

Agree that the required import should take priority. Think we just need to respect that in F401.

AA-Turner commented 2 months ago

I'm guessing you can workaround by running I002 first with the required-import, then F401 second, without the required-import?

Yep, running with --select fixed things -- I just wanted to report upstream to make the team aware!

A

AA-Turner commented 1 month ago

Thanks Charlie!

A