Skip to content

Original node wins in case of a redefinition (especially if it is a class) #5565

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

Merged
merged 5 commits into from
Sep 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,10 +387,12 @@ def _visit_func_def(self, defn: FuncDef) -> None:
# a common stack of namespaces. As the 3 kinds of namespaces have
# different semantics, this wouldn't always work, but it might still
# be a win.
# Also we can re-use some logic in self.add_symbol().
if self.is_class_scope():
# Method definition
assert self.type is not None, "Type not set at class scope"
defn.info = self.type
add_symbol = True
if not defn.is_decorated and not defn.is_overload:
if (defn.name() in self.type.names and
self.type.names[defn.name()].node != defn):
Expand All @@ -399,7 +401,9 @@ def _visit_func_def(self, defn: FuncDef) -> None:
if not self.set_original_def(n, defn):
self.name_already_defined(defn.name(), defn,
self.type.names[defn.name()])
self.type.names[defn.name()] = SymbolTableNode(MDEF, defn)
add_symbol = False
if add_symbol:
self.type.names[defn.name()] = SymbolTableNode(MDEF, defn)
self.prepare_method_signature(defn, self.type)
elif self.is_func_scope():
# Nested function
Expand Down Expand Up @@ -3277,15 +3281,21 @@ def add_symbol(self, name: str, node: SymbolTableNode,
context: Context) -> None:
# NOTE: This logic mostly parallels SemanticAnalyzerPass1.add_symbol. If you change
# this, you may have to change the other method as well.
# TODO: Combine these methods in the first and second pass into a single one.
if self.is_func_scope():
assert self.locals[-1] is not None
if name in self.locals[-1]:
# Flag redefinition unless this is a reimport of a module.
if not (node.kind == MODULE_REF and
self.locals[-1][name].node == node.node):
self.name_already_defined(name, context, self.locals[-1][name])
return
self.locals[-1][name] = node
elif self.type:
existing = self.type.names.get(name)
if existing and existing.node != node.node:
self.name_already_defined(name, context, existing)
return
self.type.names[name] = node
else:
existing = self.globals.get(name)
Expand All @@ -3301,13 +3311,15 @@ def add_symbol(self, name: str, node: SymbolTableNode,
ok = True
if not ok:
self.name_already_defined(name, context, existing)
return
self.globals[name] = node

def add_local(self, node: Union[Var, FuncDef, OverloadedFuncDef], ctx: Context) -> None:
assert self.locals[-1] is not None, "Should not add locals outside a function"
name = node.name()
if name in self.locals[-1]:
self.name_already_defined(name, ctx, self.locals[-1][name])
return
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test case for this?

node._fullname = name
self.locals[-1][name] = SymbolTableNode(LDEF, node)

Expand Down
2 changes: 2 additions & 0 deletions mypy/semanal_pass1.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ def add_symbol(self, name: str, node: SymbolTableNode,
if not (node.kind == MODULE_REF and
self.sem.locals[-1][name].node == node.node):
self.sem.name_already_defined(name, context, self.sem.locals[-1][name])
return
self.sem.locals[-1][name] = node
else:
assert self.sem.type is None # Pass 1 doesn't look inside classes
Expand All @@ -364,5 +365,6 @@ def add_symbol(self, name: str, node: SymbolTableNode,
ok = True
if not ok:
self.sem.name_already_defined(name, context, existing)
return
elif not existing:
self.sem.globals[name] = node
143 changes: 143 additions & 0 deletions test-data/unit/check-incremental.test
Original file line number Diff line number Diff line change
Expand Up @@ -4885,6 +4885,149 @@ def f(x: str) -> None: pass
[out2]
main:2: error: Argument 1 to "f" has incompatible type "int"; expected "str"

-- These tests should just not crash
[case testOverrideByBadVar]
import a
[file a.py]
import lib
x = 1
[file a.py.2]
import lib
x = 2
[file lib.py]
class Slow:
pass

s: Slow
from cext import Slow # type: ignore
[out]
[out2]

[case testOverrideByBadVarAlias]
import a
[file a.py]
import lib
x = 1
[file a.py.2]
import lib
x = 2
[file lib.py]
class Slow:
pass

A = Slow
from cext import Slow # type: ignore
[out]
[out2]

[case testOverrideByBadVarClass]
import a
[file a.py]
import lib
x = 1
[file a.py.2]
import lib
x = 2
[file lib.py]
class C:
class Slow:
pass
s: Slow
from cext import Slow # type: ignore
[out]
[out2]

[case testOverrideByBadVarClassAlias]
import a
[file a.py]
import lib
x = 1
[file a.py.2]
import lib
x = 2
[file lib.py]
class C:
class Slow:
pass
A = Slow
from cext import Slow # type: ignore
[out]
[out2]

[case testOverrideByBadVarExisting]
import a
[file a.py]
import lib
x = 1
[file a.py.2]
import lib
x = 2
[file lib.py]
class Slow:
pass

s: Slow
from cext import Slow # type: ignore
[file cext.py]
Slow = 1
[out]
[out2]

[case testOverrideByBadVarAliasExisting]
import a
[file a.py]
import lib
x = 1
[file a.py.2]
import lib
x = 2
[file lib.py]
class Slow:
pass

A = Slow
from cext import Slow # type: ignore
[file cext.py]
Slow = 1
[out]
[out2]

[case testOverrideByBadFunction]
import a
[file a.py]
import lib
x = 1
[file a.py.2]
import lib
x = 2
[file lib.py]
class C:
class Slow:
pass

s: Slow
def Slow() -> None: ... # type: ignore
[out]
[out2]

[case testOverrideByBadVarLocal]
import a
[file a.py]
import lib
x = 1
[file a.py.2]
import lib
x = 2
[file lib.py]
def outer() -> None:
class Slow:
pass

s: Slow
from cext import Slow # type: ignore
[out]
[out2]

[case testFollowImportSkipNotInvalidatedOnPresent]
# flags: --follow-imports=skip
# cmd: mypy -m main
Expand Down