Skip to content

"Name already defined" error is confusing #4722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sid-kap opened this issue Mar 12, 2018 · 11 comments
Closed

"Name already defined" error is confusing #4722

sid-kap opened this issue Mar 12, 2018 · 11 comments

Comments

@sid-kap
Copy link
Contributor

sid-kap commented Mar 12, 2018

Mypy complains if an if-else statement redefines a global variable with a class definition.

Failing code:

if 5 % 2 == 0:
  X = 5
else:
  class X:
    pass

Error:

test.py:4: error: Name 'X' already defined on line 2

mypy version: 0.570

@sid-kap sid-kap changed the title Name already defined error with if-else and class definition "Name already defined error with if-else and class definition Mar 12, 2018
@sid-kap sid-kap changed the title "Name already defined error with if-else and class definition "Name already defined" error with if-else and class definition Mar 12, 2018
@gvanrossum
Copy link
Member

That's intentional, because after the if-else X has a useless type.

@sid-kap
Copy link
Contributor Author

sid-kap commented Mar 12, 2018

  1. What do you mean by "has a useless type"?

  2. Here's the code I'm depending on that's causing this error: https://github.com/ipython/ipython/blob/dc46d30ae2318456b5f3fb3cef37066ef2cc305e/IPython/core/history.py#L66. What do you suggest I do to fix this error?

@gvanrossum
Copy link
Member

I was referring to your example. Apparently you didn't capture the essence of the real code you were having trouble with. There are a number of special cases implemented when two branches define different but equivalent (to the user) types, and your example wasn't one of them.

It seems defining two different classes isn't one of them either. So I recommend putting # type: ignore on the exact line where you get the error.

@sid-kap
Copy link
Contributor Author

sid-kap commented Mar 12, 2018

Ok, thanks!

@sid-kap
Copy link
Contributor Author

sid-kap commented Mar 12, 2018

In any case, "Name already defined" seems like a very bad explanation for the error. The problem is not that X is already defined—mutating variables is something you do in Python all the time. The problem is that you're trying to reassign something to X that's of a different type.

@gvanrossum gvanrossum changed the title "Name already defined" error with if-else and class definition "Name already defined" error is confusing Mar 12, 2018
@gvanrossum
Copy link
Member

OK, reopening as a usability issue -- long-term maintainers get blind for those little usability problems, so thanks for bringing that up!

@glenjamin
Copy link

glenjamin commented Apr 2, 2018

I've just run into the same error message, but in a different scenario.

I'm trying to generate some usable stubs for the GitPython project.

This is what __init__.pyi looks like:

# Stubs for git (Python 3.6)
#
# NOTE: This dynamically typed stub was automatically generated by stubgen.

from git.exc import *
from git.objects import *
from git.refs import *
from git.diff import *
from git.db import *
from git.remote import *
from git.index import *
from git.cmd import Git as Git
from git.config import GitConfigParser as GitConfigParser
from git.repo import Repo as Repo
from git.util import Actor as Actor, BlockingLockFile as BlockingLockFile, LockFile as LockFile, Stats as Stats, rmtree as rmtree

Which produces the following error:

type-stubs/git/__init__.pyi:6: error: Name 'util' already defined
type-stubs/git/__init__.pyi:7: error: Name 'tag' already defined
type-stubs/git/__init__.pyi:11: error: Name 'base' already defined

It would be helpful if the error message could tell me where the original definition came from, and possible also the type of each so I can check whether the incompatibility is a type error or a naming/import error.

@sid-kap
Copy link
Contributor Author

sid-kap commented May 16, 2018

I believe @glenjamin's problem would be fixed by passing in existing as the optional argument original_ctx to self.name_already_defined in SemanticAnalyzerPass2.add_symbol at

self.name_already_defined(name, context)

(i.e. changing this line to self.name_already_defined(name, context, existing)).

In my test code, it now prints Name 'util' already defined on line 1, where line 1 is the line that contains the other wildcard import that imported something called util.
Does this seem like a reasonable change to make? Or is there a reason that the authors chose not to print this information?

@JelleZijlstra
Copy link
Member

Yes, that sounds like it would be an improvement!

ilevkivskyi pushed a commit that referenced this issue May 28, 2018
In semanal.py, name_already_defined takes an optional original_ctx: Optional[SymbolTableNode] argument which is used to print the line number of the original context where a name was defined already.

In many cases, the code doesn't pass anything for this argument, even though there is an original context that makes sense. This PR does this for the name_already_defined call in add_symbol, as discussed in #4722.
@Strum355
Copy link

Can this issue be closed as the PR was merged, or is this unresolved?

@gvanrossum
Copy link
Member

Let's close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants