New semantic analyzer: Prohibit duplicate type variable definitions and fix stale upper bounds and values

mypy/newsemanal/semanal.py

@@ -997,6 +997,13 @@ def analyze_class(self, defn: ClassDef) -> None:
         bases = defn.base_type_exprs
         bases, tvar_defs, is_protocol = self.clean_up_bases_and_infer_type_variables(defn, bases,
                                                                                      context=defn)
+
+        for tvd in tvar_defs:
+            if any(has_placeholder(t) for t in [tvd.upper_bound] + tvd.values):
+                # Some type variable bounds or values are not ready, we need
+                # to re-analyze this class.
+                self.defer()
+
         self.analyze_class_keywords(defn)
         result = self.analyze_base_classes(bases)
 
@@ -2675,10 +2682,15 @@ def process_typevar_declaration(self, s: AssignmentStmt) -> bool:
         if s.type:
             self.fail("Cannot declare the type of a type variable", s)
             return False
+
+        assert isinstance(s.rvalue, CallExpr)
         name = lvalue.name
         names = self.current_symbol_table()
         existing = names.get(name)
-        if existing and not isinstance(existing.node, (TypeVarExpr, PlaceholderNode)):
+        if existing and not (isinstance(existing.node, PlaceholderNode) or
+                             # Also give error for another type variable with the same name.
+                             (isinstance(existing.node, TypeVarExpr) and
+                              existing.node is s.rvalue.analyzed)):
             self.fail("Cannot redefine '%s' as a type variable" % name, s)
             return False
 
@@ -2687,7 +2699,7 @@ def process_typevar_declaration(self, s: AssignmentStmt) -> bool:
 
         # Constraining types
         n_values = call.arg_kinds[1:].count(ARG_POS)
-        values = self.analyze_types(call.args[1:1 + n_values])
+        values = self.analyze_value_types(call.args[1:1 + n_values])
 
         res = self.process_typevar_parameters(call.args[1 + n_values:],
                                               call.arg_names[1 + n_values:],
@@ -2719,20 +2731,19 @@ def process_typevar_declaration(self, s: AssignmentStmt) -> bool:
             upper_bound = AnyType(TypeOfAny.implementation_artifact)
 
         # Yes, it's a valid type variable definition! Add it to the symbol table.
-        if existing and isinstance(existing.node, TypeVarExpr):
-            # Existing definition from previous semanal iteration, use it.
-            # TODO: This may be confused with a duplicate TypeVar definition.
-            #       Fix this and add corresponding tests.
-            type_var = existing.node
-            type_var.values = values
-            type_var.upper_bound = upper_bound
-            type_var.variance = variance
-        else:
+        if not call.analyzed:
             type_var = TypeVarExpr(name, self.qualified_name(name),
                                    values, upper_bound, variance)
             type_var.line = call.line
             call.analyzed = type_var
-            self.add_symbol(name, type_var, s)
+        else:
+            assert isinstance(call.analyzed, TypeVarExpr)
+            if call.analyzed.values != values or call.analyzed.upper_bound != upper_bound:
+                self.progress = True
+            call.analyzed.upper_bound = upper_bound
+            call.analyzed.values = values
+
+        self.add_symbol(name, call.analyzed, s)
         return True
 
     def check_typevar_name(self, call: CallExpr, name: str, context: Context) -> bool:
@@ -2807,18 +2818,16 @@ def process_typevar_parameters(self, args: List[Expression],
                     # We want to use our custom error message below, so we suppress
                     # the default error message for invalid types here.
                     analyzed = self.expr_to_analyzed_type(param_value,
+                                                          allow_placeholder=True,
                                                           report_invalid_types=False)
                     if analyzed is None:
-                        # It is fine to simply use a temporary Any because we don't need the bound
-                        # for anything before main pass of semantic analysis is finished. We will
-                        # incrementally populate `TypeVarExpr` if some part is missing during main
-                        # pass iterations.
-                        # NOTE: It is safe to not call self.defer() here, because the only way
-                        # we can get None from self.anal_type() is if self.found_incomplete_refs()
-                        # returned True. In turn, the only way it can happen is if someone called
-                        # self.record_incomplete_ref(), and the latter unconditionally calls
-                        # self.defer().
-                        analyzed = AnyType(TypeOfAny.special_form)
+                        # Type variables are special: we need to place them in the symbol table
+                        # soon, even if upper bound is not ready yet. Otherwise avoiding
+                        # a "deadlock" in this common pattern would be tricky:
+                        #     T = TypeVar('T', bound=Custom[Any])
+                        #     class Custom(Generic[T]):
+                        #         ...
+                        analyzed = PlaceholderType('<unknown>', [], context.line)
                     upper_bound = analyzed
                     if isinstance(upper_bound, AnyType) and upper_bound.is_from_error:
                         self.fail("TypeVar 'bound' must be a type", param_value)
@@ -2849,7 +2858,7 @@ def process_typevar_parameters(self, args: List[Expression],
             variance = CONTRAVARIANT
         else:
             variance = INVARIANT
-        return (variance, upper_bound)
+        return variance, upper_bound
 
     def basic_new_typeinfo(self, name: str, basetype_or_fallback: Instance) -> TypeInfo:
         class_def = ClassDef(name, Block([]))
@@ -2872,18 +2881,19 @@ def basic_new_typeinfo(self, name: str, basetype_or_fallback: Instance) -> TypeI
         info.bases = [basetype_or_fallback]
         return info
 
-    def analyze_types(self, items: List[Expression]) -> List[Type]:
+    def analyze_value_types(self, items: List[Expression]) -> List[Type]:
         """Analyze types from values expressions in type variable definition."""
         result = []  # type: List[Type]
         for node in items:
             try:
-                analyzed = self.anal_type(expr_to_unanalyzed_type(node))
-                if analyzed is not None:
-                    result.append(analyzed)
-                else:
-                    # It is fine to simply use temporary Anys because we don't need values
-                    # for anything before main pass of semantic analysis is finished.
-                    result.append(AnyType(TypeOfAny.special_form))
+                analyzed = self.anal_type(expr_to_unanalyzed_type(node),
+                                          allow_placeholder=True)
+                if analyzed is None:
+                    # Type variables are special: we need to place them in the symbol table
+                    # soon, even if some value is not ready yet, see process_typevar_parameters()
+                    # for an example.
+                    analyzed = PlaceholderType('<unknown>', [], node.line)
+                result.append(analyzed)
             except TypeTranslationError:
                 self.fail('Type expected', node)
                 result.append(AnyType(TypeOfAny.from_error))

mypy/type_visitor.py

@@ -282,7 +282,7 @@ def visit_deleted_type(self, t: DeletedType) -> T:
         return self.strategy([])
 
     def visit_type_var(self, t: TypeVarType) -> T:
-        return self.strategy([])
+        return self.query_types([t.upper_bound] + t.values)
 
     def visit_partial_type(self, t: PartialType) -> T:
         return self.query_types(t.inner_types)

test-data/unit/check-newsemanal.test

@@ -1988,6 +1988,109 @@ x: A
 reveal_type(x)  # N: Revealed type is '__main__.G[Tuple[builtins.int, fallback=__main__.C]]'
 [builtins fixtures/list.pyi]
 
+[case testNewAnalyzerDuplicateTypeVar]
+from typing import TypeVar, Generic, Any
+
+T = TypeVar('T', bound=B[Any])
+# The "int" error is because of typing fixture.
+T = TypeVar('T', bound=C)  # E: Cannot redefine 'T' as a type variable \
+                           # E: Invalid assignment target \
+                           # E: "int" not callable
+
+class B(Generic[T]):
+    x: T
+class C: ...
+
+x: B[int]  # E: Type argument "builtins.int" of "B" must be a subtype of "__main__.B[Any]"
+y: B[B[Any]]
+reveal_type(y.x)  # N: Revealed type is '__main__.B*[Any]'
+
+[case testNewAnalyzerDuplicateTypeVarImportCycle]
+import a
+[file a.py]
+from typing import TypeVar, Any
+from b import B, C
+
+T = TypeVar('T', bound=B[Any])
+T = TypeVar('T', bound=C)
+
+[file b.py]
+from typing import Generic, Any
+from a import T
+
+class B(Generic[T]):
+    x: T
+class C: ...
+
+x: B[int]
+y: B[B[Any]]
+reveal_type(y.x)
+[out]
+tmp/b.py:8: error: Type argument "builtins.int" of "B" must be a subtype of "b.B[Any]"
+tmp/b.py:10: note: Revealed type is 'b.B*[Any]'
+tmp/a.py:5: error: Cannot redefine 'T' as a type variable
+tmp/a.py:5: error: Invalid assignment target
+tmp/a.py:5: error: "int" not callable
+
+[case testNewAnalyzerDuplicateTypeVarImportCycleWithAliases]
+import a
+[file a.py]
+from typing import TypeVar, Any
+from b import BA, C
+
+T = TypeVar('T', bound=BAA[Any])
+T = TypeVar('T', bound=C)
+BAA = BA
+
+[file b.py]
+from typing import Generic, Any
+from a import T
+
+BA = B
+class B(Generic[T]):
+    x: T
+class C: ...
+
+x: B[int]
+y: B[B[Any]]
+reveal_type(y.x)
+[out]
+tmp/b.py:9: error: Type argument "builtins.int" of "B" must be a subtype of "b.B[Any]"
+tmp/b.py:11: note: Revealed type is 'b.B*[Any]'
+tmp/a.py:5: error: Cannot redefine 'T' as a type variable
+tmp/a.py:5: error: Invalid assignment target
+
+[case testNewAnalyzerTypeVarBoundInCycle]
+import factory, box
+
+[file factory.py]
+from typing import Generic, Type
+
+from box import BoxT
+
+class Factory(Generic[BoxT]):
+    value: int
+
+    def create(self, boxClass: Type[BoxT]) -> BoxT:
+        reveal_type(boxClass.create(self))  # N: Revealed type is 'BoxT`1'
+        return boxClass.create(self)
+
+[file box.py]
+from typing import TYPE_CHECKING, Type, TypeVar
+
+if TYPE_CHECKING:
+    from factory import Factory
+
+BoxT = TypeVar('BoxT', bound='Box')
+
+class Box:
+    @classmethod
+    def create(cls: Type[BoxT], f: Factory) -> BoxT:
+        return cls(f.value)
+
+    def __init__(self, value: int) -> None: ...
+[builtins fixtures/classmethod.pyi]
+
 [case testNewAnalyzerCastForward1]
 from typing import cast