New analyzer: fix issues with order-dependent results (#7158)

When we add a placeholder, we should do it through `mark_incomplete`,
since the name needs to added to the set of missing names. Otherwise
a later definition can take precedence over an earlier definition,
if the first definition initially creates a placeholder.

Fixes #7157 and probably another issue related to type aliases, which
was exposed by the fix.

Author: JukkaL

mypy/newsemanal/semanal.py

@@ -203,7 +203,10 @@ class NewSemanticAnalyzer(NodeVisitor[None],
     # Note that missing names are per module, _not_ per namespace. This means that e.g.
     # a missing name at global scope will block adding same name at a class scope.
     # This should not affect correctness and is purely a performance issue,
-    # since it can cause unnecessary deferrals.
+    # since it can cause unnecessary deferrals. These are represented as
+    # PlaceholderNodes in the symbol table. We use this to ensure that the first
+    # definition takes precedence even if it's incomplete.
+    #
     # Note that a star import adds a special name '*' to the set, this blocks
     # adding _any_ names in the current file.
     missing_names = None  # type: Set[str]
@@ -1702,7 +1705,9 @@ def process_imported_symbol(self,
             if self.final_iteration:
                 self.report_missing_module_attribute(module_id, id, imported_id, context)
                 return
-            self.record_incomplete_ref()
+            else:
+                # This might become a type.
+                self.mark_incomplete(imported_id, node.node, becomes_typeinfo=True)
         existing_symbol = self.globals.get(imported_id)
         if (existing_symbol and not isinstance(existing_symbol.node, PlaceholderNode) and
                 not isinstance(node.node, PlaceholderNode)):
@@ -2408,11 +2413,7 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
             if self.found_incomplete_ref(tag) or has_placeholder(res):
                 # Since we have got here, we know this must be a type alias (incomplete refs
                 # may appear in nested positions), therefore use becomes_typeinfo=True.
-                placeholder = PlaceholderNode(self.qualified_name(lvalue.name),
-                                              rvalue,
-                                              s.line,
-                                              becomes_typeinfo=True)
-                self.add_symbol(lvalue.name, placeholder, s)
+                self.mark_incomplete(lvalue.name, rvalue, becomes_typeinfo=True)
                 return True
         self.add_type_alias_deps(depends_on)
         # In addition to the aliases used, we add deps on unbound
@@ -4139,6 +4140,9 @@ def add_symbol_table_node(self,
             # need to do anything.
             old = existing.node
             new = symbol.node
+            if isinstance(new, PlaceholderNode):
+                # We don't know whether this is okay. Let's wait until the next iteration.
+                return False
             if not is_same_symbol(old, new):
                 if isinstance(new, (FuncDef, Decorator, OverloadedFuncDef, TypeInfo)):
                     self.add_redefinition(names, name, symbol)
@@ -4306,7 +4310,7 @@ def mark_incomplete(self, name: str, node: Node,
         self.defer(node)
         if name == '*':
             self.incomplete = True
-        elif name not in self.current_symbol_table() and not self.is_global_or_nonlocal(name):
+        elif not self.is_global_or_nonlocal(name):
             fullname = self.qualified_name(name)
             assert self.statement
             placeholder = PlaceholderNode(fullname, node, self.statement.line,
@@ -4809,10 +4813,12 @@ def is_valid_replacement(old: SymbolTableNode, new: SymbolTableNode) -> bool:
     2. Placeholder that isn't known to become type replaced with a
        placeholder that can become a type
     """
-    return (isinstance(old.node, PlaceholderNode)
-            and (not isinstance(new.node, PlaceholderNode)
-                 or (not old.node.becomes_typeinfo
-                     and new.node.becomes_typeinfo)))
+    if isinstance(old.node, PlaceholderNode):
+        if isinstance(new.node, PlaceholderNode):
+            return not old.node.becomes_typeinfo and new.node.becomes_typeinfo
+        else:
+            return True
+    return False
 
 
 def is_same_symbol(a: Optional[SymbolNode], b: Optional[SymbolNode]) -> bool:

test-data/unit/check-incremental.test

@@ -4497,29 +4497,36 @@ tmp/a.py:3: note: Revealed type is 'builtins.list[builtins.list[builtins.list[An
 [case testRecursiveAliasImported2]
 # flags: --new-semantic-analyzer
 import a
+
 [file a.py]
 import lib
 x: int
+
 [file a.py.2]
 import lib
 x: lib.A
 reveal_type(x)
+
 [file lib.pyi]
 from typing import List
-from other import B
+MYPY = False
+if MYPY:  # Force processing order
+    from other import B
 A = List[B]  # type: ignore
+
 [file other.pyi]
 from typing import List
 from lib import A
 B = List[A]
+
 [builtins fixtures/list.pyi]
 [out]
+tmp/other.pyi:2: error: Module 'lib' has no attribute 'A'
 tmp/other.pyi:3: error: Cannot resolve name "B" (possible cyclic definition)
-tmp/lib.pyi:2: error: Module 'other' has no attribute 'B'
 [out2]
+tmp/other.pyi:2: error: Module 'lib' has no attribute 'A'
 tmp/other.pyi:3: error: Cannot resolve name "B" (possible cyclic definition)
-tmp/lib.pyi:2: error: Module 'other' has no attribute 'B'
-tmp/a.py:3: note: Revealed type is 'builtins.list[Any]'
+tmp/a.py:3: note: Revealed type is 'builtins.list[builtins.list[Any]]'
 
 [case testRecursiveNamedTupleTypedDict]
 # https://github.com/python/mypy/issues/7125

test-data/unit/check-newsemanal.test

@@ -272,7 +272,33 @@ reveal_type(x) # N: Revealed type is 'TypedDict('__main__.T2', {'x': builtins.st
 y: T4
 reveal_type(y) # N: Revealed type is 'TypedDict('__main__.T4', {'x': builtins.str, 'y': __main__.A})'
 
-[case testNewAnalyzerRedefinitionAndDeferral]
+[case testNewAnalyzerRedefinitionAndDeferral1a]
+import a
+
+[file a.py]
+MYPY = False
+if MYPY:
+    from b import x as y
+x = 0
+
+def y(): pass # E: Name 'y' already defined on line 4
+reveal_type(y) # N: Revealed type is 'builtins.int'
+
+y2 = y
+class y2: pass # E: Name 'y2' already defined on line 9
+reveal_type(y2) # N: Revealed type is 'builtins.int'
+
+y3, y4 = y, y
+if MYPY:  # Tweak processing order
+    from b import f as y3 # E: Incompatible import of "y3" (imported name has type "Callable[[], Any]", local name has type "int")
+reveal_type(y3) # N: Revealed type is 'builtins.int'
+
+[file b.py]
+from a import x
+
+def f(): pass
+
+[case testNewAnalyzerRedefinitionAndDeferral1b]
 import a
 
 [file a.py]
@@ -291,11 +317,26 @@ from b import f as y3 # E: Incompatible import of "y3" (imported name has type "
 reveal_type(y3) # N: Revealed type is 'builtins.int'
 
 [file b.py]
-from a import x
+MYPY = False
+if MYPY:  # Tweak processing order
+    from a import x
 
 def f(): pass
 
-[case testNewAnalyzerRedefinitionAndDeferral2]
+[case testNewAnalyzerRedefinitionAndDeferral2a]
+import a
+
+[file a.py]
+MYPY = False
+if MYPY:  # Tweak processing order
+    from b import C as C2
+class C: pass
+class C2: pass # E: Name 'C2' already defined on line 4
+
+[file b.py]
+from a import C
+
+[case testNewAnalyzerRedefinitionAndDeferral2b]
 import a
 
 [file a.py]
@@ -304,7 +345,9 @@ class C: pass
 
 class C2: pass # E: Name 'C2' already defined on line 2
 [file b.py]
-from a import C
+MYPY = False
+if MYPY:  # Tweak processing order
+    from a import C
 
 [case testNewAnalyzerRedefinitionAndDeferral3]
 import a
@@ -320,7 +363,7 @@ reveal_type(b) # N: Revealed type is 'Any'
 [file b.py]
 from a import f
 
-[case testNewAnalyzerImportStarForwardRef]
+[case testNewAnalyzerImportStarForwardRef1]
 import a
 
 [file a.py]
@@ -331,6 +374,25 @@ from b import *
 
 class A: pass # E: Name 'A' already defined (possibly by an import)
 
+[file b.py]
+class A: pass
+MYPY = False
+if MYPY:  # Tweak processing order
+    from a import x
+
+[case testNewAnalyzerImportStarForwardRef2]
+import a
+
+[file a.py]
+x: A
+reveal_type(x) # N: Revealed type is 'b.A'
+
+MYPY = False
+if MYPY: # Tweak processing order
+    from b import *
+
+class A: pass # E: Name 'A' already defined (possibly by an import)
+
 [file b.py]
 class A: pass
 from a import x
@@ -1005,19 +1067,34 @@ import a
 class C(B): ...
 class B: ...
 
-[case testNewAnalyzerImportOverExistingInCycleStar]
+[case testNewAnalyzerImportOverExistingInCycleStar1]
 import a
 [file a.py]
 C = 1
-from b import *  # E: Name 'C' already defined on line 1 \
-                 # E: Incompatible import of "C" (imported name has type "Type[C]", local name has type "int")
+MYPY = False
+if MYPY:  # Tweak processing ordre
+    from b import *  # E: Incompatible import of "C" (imported name has type "Type[C]", local name has type "int")
 
 [file b.py]
 import a
 
 class C(B): ...
 class B: ...
 
+[case testNewAnalyzerImportOverExistingInCycleStar2]
+import a
+[file a.py]
+C = 1
+from b import *  # E: Incompatible import of "C" (imported name has type "Type[C]", local name has type "int")
+
+[file b.py]
+MYPY = False
+if MYPY:  # Tweak processing order
+    import a
+
+class C(B): ...
+class B: ...
+
 [case testNewAnalyzerIncompleteFixture]
 from typing import Tuple