Use of star-imports nearly always yields "name already defined" errors

[davidfstr]

If I have a module with star imports, such as:

# A.py
from B import *
from C import *

It is very common to get "name already defined" errors on one of the star-import lines:

$ mypy A.py
A.py:2: error: Name 'Boom' already defined

In this example, the related files are:

# B.py
from D import Boom

# C.py
from D import Boom

# D.py
class Boom:
    pass

Currently I use "# type: ignore" on pretty much all star-imports due to these "name already defined" errors.

Assuming this is a real issue that hasn't already been reported elsewhere, I'm planning to use its issue URL as a reference to justify the use of "# type: ignore".

[Michael0x2a]

Hmm, interesting!

After doing some more digging, it looks like the issue is the currently-existing checks (semenal.process_import_over_existing_name) on imports of the form from x import y and from x import * are somewhat broken -- it allows Var and FuncDef nodes, but doesn't allow TypeInfo nodes (which correspond to classes). You can test this by changing Boom in your second example to either a function or a simple variable declaration.

Within this check, mypy also seems to expect that the two duplicate symbols have the exact same type, which is problematic if your B and C files were actually defining two entirely separate Boom classes, two functions with different signatures, two variables of different type, etc.

It's unclear to me if this behavior is something that needs to be changed or not?

[davidfstr]

Within this check, mypy also seems to expect that the two duplicate symbols have the exact same type, which is problematic if your B and C files were actually defining two entirely separate Boom classes, two functions with different signatures, two variables of different type, etc.

The referenced classes (ex: Boom) are the same for 100% of the errors that occur in my Django codebase. I would expect that if the symbols were different that the last one would win (or at most issue a warning), as per standard Python runtime behavior.

[gvanrossum]

Mypy tries to be more precise, since redefining something is often a symptom of an error. But I can see the point that if you import the same thing twice it should not be considered an error (or at least not when using import *). I think it's a reasonable proposal to change the import * checking to allow benign redefinitions. However it should still balk if it overrides a previously imported or defined object -- that is exactly one of the dangers of import * that we are trying to catch!

I would also suggest that maybe you should add more precise __all__ lists to your modules if import * is a common pattern. Perhaps re-exporting everything you've imported is not what you want.

[davidfstr]

I don't believe #2139 fixes the issue.

I just ran mypy 0.4.5-dev-a2d6638b3b746b54f2dd911c460148e1a1886ab9 on the example given above and still get error: Name 'Boom' already defined.

[Michael0x2a]

That's odd -- I don't seem to be able to repro? For what it's worth, here's the bash script I used for testing (which I'm running from within a clone of the mypy repo)"

cat >A.py <<EOL
from B import *
from C import *
EOL

cat >B.py <<EOL
from D import Boom
EOL

cat >C.py <<EOL
from D import Boom
EOL

cat >D.py <<EOL
class Boom:
    pass
EOL

git rev-parse HEAD  # Should output a2d6638b3b746b54f2dd911c460148e1a1886ab9
python3 -m mypy A.py

[davidfstr]

@Michael0x2a I was able to get it to work in a fresh Docker container with your above steps. I must have done something wrong when getting mypy inside a virtualenv...

Additionally, with this fix I am able to remove all of the # type: ignores on star-imports on my Django codebase and still have mypy pass it. 👍