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

Fixes #5534

Author: ilevkivskyi

mypy/semanal.py

@@ -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):
@@ -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
@@ -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)
@@ -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
         node._fullname = name
         self.locals[-1][name] = SymbolTableNode(LDEF, node)
 

mypy/semanal_pass1.py

@@ -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
@@ -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

test-data/unit/check-incremental.test

@@ -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