F823 (undefined local) triggered by a double import

[ajkerrigan]

First, thanks for ruff! It rocks and I love it 😍 .

I ran across a minor issue recently and wanted to document it here in case other folks do too. I'm not sure if it's an intentional change or not...

Moving from version 0.0.278 to 0.0.279 I noticed F823 Local variable 'sys' referenced before assignment occurring in a new situation. If you import a module twice, this rule flags references in between the two imports. For example:

double_import.py

import sys


def main():
    print(sys.argv)

    try:
        3 / 0
    except ZeroDivisionError:
        import sys

        sys.exit(1)

❯ ruff double_import.py
double_import.py:5:11: F823 Local variable `sys` referenced before assignment
Found 1 error.

It does feel right to call out the double import sys as unnecessary here, but F823 seems misleading. Is this change intentional?

I didn't see an obvious answer in the 0.0.279 changelog though there are a couple changes that seem possibly relevant. Will update here if I find anything useful. The change that seems most relevant at a glance (to my untrained eye anyway) is bcec2f0.

[charliermarsh]

Thanks! Looks like a bug.

[charliermarsh]

Or, this may actually be correct? I'm not 100% sure. Python does error with an early unbound access:

Traceback (most recent call last):
  File "/Users/crmarsh/workspace/ruff/foo.py", line 22, in <module>
    main()
  File "/Users/crmarsh/workspace/ruff/foo.py", line 5, in main
    print(sys.argv)
          ^^^
UnboundLocalError: cannot access local variable 'sys' where it is not associated with a value

So in that case, F823 feels correct to me.

[ajkerrigan]

Ha, you're right 😅 ! I could have seen it either way since it does feel wrong and worth yelling about one way or the other.

My first thought was looking at flake8 which runs clean on this file. But pylint yells all around the issue 😁

double_import.py:11:8: W0621: Redefining name 'sys' from outer scope (line 1) (redefined-outer-name)
double_import.py:6:10: E0601: Using variable 'sys' before assignment (used-before-assignment)
double_import.py:11:8: W0404: Reimport 'sys' (imported line 1) (reimported)
double_import.py:11:8: C0415: Import outside toplevel (sys) (import-outside-toplevel)
double_import.py:1:0: W0611: Unused import sys (unused-import)

This is also a good reminder to not just lint your minimal example but also try, you know... running it 😆 .

Thanks for the follow-up!

[charliermarsh]

I had the same reaction as you, I actually thought that code was fine! I'm gonna add some test cases so that we're at least explicit about this.