Flip the default for recursive aliases flag (#13516)

ilevkivskyi · web-flow · commit ddd917767aa8 · 2022-09-29T14:56:57.000+01:00
I don't think we need to wait long time for this. As soon as next
release goes out, I think we can flip the default. Otherwise, this
feature may degrade, because there are just several dozen tests with
this flag on.

(Also I am curious to see `mypy_primer` on this.)

I manually checked each of couple dozen tests where I currently disable
recursive aliases (they essentially just test that there is no crash and
emit various errors like `Cannot resolve name`).
diff --git a/mypy/main.py b/mypy/main.py
@@ -975,9 +975,9 @@ def add_invertible_flag(
         help="Use a custom typing module",
     )
     internals_group.add_argument(
-        "--enable-recursive-aliases",
+        "--disable-recursive-aliases",
         action="store_true",
-        help="Experimental support for recursive type aliases",
+        help="Disable experimental support for recursive type aliases",
     )
     internals_group.add_argument(
         "--custom-typeshed-dir", metavar="DIR", help="Use the custom typeshed in DIR"
diff --git a/mypy/options.py b/mypy/options.py
@@ -312,8 +312,8 @@ def __init__(self) -> None:
         # skip most errors after this many messages have been reported.
         # -1 means unlimited.
         self.many_errors_threshold = defaults.MANY_ERRORS_THRESHOLD
-        # Enable recursive type aliases (currently experimental)
-        self.enable_recursive_aliases = False
+        # Disable recursive type aliases (currently experimental)
+        self.disable_recursive_aliases = False
 
     # To avoid breaking plugin compatibility, keep providing new_semantic_analyzer
     @property
diff --git a/mypy/semanal.py b/mypy/semanal.py
@@ -3238,7 +3238,7 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
             )
             if not res:
                 return False
-            if self.options.enable_recursive_aliases and not self.is_func_scope():
+            if not self.options.disable_recursive_aliases and not self.is_func_scope():
                 # Only marking incomplete for top-level placeholders makes recursive aliases like
                 # `A = Sequence[str | A]` valid here, similar to how we treat base classes in class
                 # definitions, allowing `class str(Sequence[str]): ...`
@@ -5749,7 +5749,7 @@ def process_placeholder(self, name: str, kind: str, ctx: Context) -> None:
 
     def cannot_resolve_name(self, name: str, kind: str, ctx: Context) -> None:
         self.fail(f'Cannot resolve {kind} "{name}" (possible cyclic definition)', ctx)
-        if self.options.enable_recursive_aliases and self.is_func_scope():
+        if not self.options.disable_recursive_aliases and self.is_func_scope():
             self.note("Recursive types are not allowed at function scope", ctx)
 
     def qualified_name(self, name: str) -> str:
diff --git a/mypy/semanal_namedtuple.py b/mypy/semanal_namedtuple.py
@@ -176,7 +176,7 @@ def check_namedtuple_classdef(
                     # it would be inconsistent with type aliases.
                     analyzed = self.api.anal_type(
                         stmt.type,
-                        allow_placeholder=self.options.enable_recursive_aliases
+                        allow_placeholder=not self.options.disable_recursive_aliases
                         and not self.api.is_func_scope(),
                     )
                     if analyzed is None:
@@ -443,7 +443,7 @@ def parse_namedtuple_fields_with_types(
                 # We never allow recursive types at function scope.
                 analyzed = self.api.anal_type(
                     type,
-                    allow_placeholder=self.options.enable_recursive_aliases
+                    allow_placeholder=not self.options.disable_recursive_aliases
                     and not self.api.is_func_scope(),
                 )
                 # Workaround #4987 and avoid introducing a bogus UnboundType
diff --git a/mypy/semanal_newtype.py b/mypy/semanal_newtype.py
@@ -203,7 +203,7 @@ def check_newtype_args(
             self.api.anal_type(
                 unanalyzed_type,
                 report_invalid_types=False,
-                allow_placeholder=self.options.enable_recursive_aliases
+                allow_placeholder=not self.options.disable_recursive_aliases
                 and not self.api.is_func_scope(),
             )
         )
diff --git a/mypy/semanal_typeddict.py b/mypy/semanal_typeddict.py
@@ -218,7 +218,7 @@ def analyze_base_args(self, base: IndexExpr, ctx: Context) -> list[Type] | None:
             analyzed = self.api.anal_type(
                 type,
                 allow_required=True,
-                allow_placeholder=self.options.enable_recursive_aliases
+                allow_placeholder=not self.options.disable_recursive_aliases
                 and not self.api.is_func_scope(),
             )
             if analyzed is None:
@@ -289,7 +289,7 @@ def analyze_typeddict_classdef_fields(
                     analyzed = self.api.anal_type(
                         stmt.type,
                         allow_required=True,
-                        allow_placeholder=self.options.enable_recursive_aliases
+                        allow_placeholder=not self.options.disable_recursive_aliases
                         and not self.api.is_func_scope(),
                     )
                     if analyzed is None:
@@ -484,7 +484,7 @@ def parse_typeddict_fields_with_types(
             analyzed = self.api.anal_type(
                 type,
                 allow_required=True,
-                allow_placeholder=self.options.enable_recursive_aliases
+                allow_placeholder=not self.options.disable_recursive_aliases
                 and not self.api.is_func_scope(),
             )
             if analyzed is None:
diff --git a/mypy/typeanal.py b/mypy/typeanal.py
@@ -402,7 +402,7 @@ def cannot_resolve_type(self, t: UnboundType) -> None:
         #       need access to MessageBuilder here. Also move the similar
         #       message generation logic in semanal.py.
         self.api.fail(f'Cannot resolve name "{t.name}" (possible cyclic definition)', t)
-        if self.options.enable_recursive_aliases and self.api.is_func_scope():
+        if not self.options.disable_recursive_aliases and self.api.is_func_scope():
             self.note("Recursive types are not allowed at function scope", t)
 
     def apply_concatenate_operator(self, t: UnboundType) -> Type:
diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test
@@ -4779,7 +4779,7 @@ class A(Tuple[int, str]): pass
 -- -----------------------
 
 [case testCrashOnSelfRecursiveNamedTupleVar]
-
+# flags: --disable-recursive-aliases
 from typing import NamedTuple
 
 N = NamedTuple('N', [('x', N)]) # E: Cannot resolve name "N" (possible cyclic definition)
@@ -4809,7 +4809,7 @@ lst = [n, m]
 [builtins fixtures/isinstancelist.pyi]
 
 [case testCorrectJoinOfSelfRecursiveTypedDicts]
-
+# flags: --disable-recursive-aliases
 from mypy_extensions import TypedDict
 
 class N(TypedDict):
diff --git a/test-data/unit/check-dataclasses.test b/test-data/unit/check-dataclasses.test
@@ -1915,7 +1915,6 @@ takes_cp(MyDataclass)
 [builtins fixtures/dataclasses.pyi]
 
 [case testDataclassTypeAnnotationAliasUpdated]
-# flags: --enable-recursive-aliases
 import a
 [file a.py]
 from dataclasses import dataclass
diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test
@@ -4600,7 +4600,7 @@ def outer() -> None:
 [out2]
 
 [case testRecursiveAliasImported]
-
+# flags: --disable-recursive-aliases
 import a
 
 [file a.py]
@@ -5759,7 +5759,7 @@ class C:
 [builtins fixtures/tuple.pyi]
 
 [case testNamedTupleUpdateNonRecursiveToRecursiveCoarse]
-# flags: --enable-recursive-aliases
+# flags: --strict-optional
 import c
 [file a.py]
 from b import M
@@ -5802,7 +5802,7 @@ tmp/c.py:5: error: Incompatible types in assignment (expression has type "Option
 tmp/c.py:7: note: Revealed type is "Tuple[Union[Tuple[Union[..., None], builtins.int, fallback=b.M], None], builtins.int, fallback=a.N]"
 
 [case testTupleTypeUpdateNonRecursiveToRecursiveCoarse]
-# flags: --enable-recursive-aliases
+# flags: --strict-optional
 import c
 [file a.py]
 from b import M
@@ -5835,7 +5835,7 @@ tmp/c.py:4: note: Revealed type is "Tuple[Union[Tuple[Union[..., None], builtins
 tmp/c.py:5: error: Incompatible types in assignment (expression has type "Optional[N]", variable has type "int")
 
 [case testTypeAliasUpdateNonRecursiveToRecursiveCoarse]
-# flags: --enable-recursive-aliases
+# flags: --strict-optional
 import c
 [file a.py]
 from b import M
@@ -5868,7 +5868,7 @@ tmp/c.py:4: note: Revealed type is "Tuple[Union[Tuple[Union[..., None], builtins
 tmp/c.py:5: error: Incompatible types in assignment (expression has type "Optional[N]", variable has type "int")
 
 [case testTypedDictUpdateNonRecursiveToRecursiveCoarse]
-# flags: --enable-recursive-aliases
+# flags: --strict-optional
 import c
 [file a.py]
 from b import M
diff --git a/test-data/unit/check-namedtuple.test b/test-data/unit/check-namedtuple.test
@@ -617,7 +617,7 @@ tmp/b.py:4: note: Revealed type is "Tuple[Any, fallback=a.N]"
 tmp/b.py:7: note: Revealed type is "Tuple[Any, fallback=a.N]"
 
 [case testSimpleSelfReferentialNamedTuple]
-
+# flags: --disable-recursive-aliases
 from typing import NamedTuple
 class MyNamedTuple(NamedTuple):
     parent: 'MyNamedTuple' # E: Cannot resolve name "MyNamedTuple" (possible cyclic definition)
@@ -655,7 +655,7 @@ class B:
 [out]
 
 [case testSelfRefNT1]
-
+# flags: --disable-recursive-aliases
 from typing import Tuple, NamedTuple
 
 Node = NamedTuple('Node', [
@@ -667,7 +667,7 @@ reveal_type(n) # N: Revealed type is "Tuple[builtins.str, builtins.tuple[Any, ..
 [builtins fixtures/tuple.pyi]
 
 [case testSelfRefNT2]
-
+# flags: --disable-recursive-aliases
 from typing import Tuple, NamedTuple
 
 A = NamedTuple('A', [
@@ -683,7 +683,7 @@ reveal_type(n) # N: Revealed type is "Tuple[builtins.str, builtins.tuple[Any, ..
 [builtins fixtures/tuple.pyi]
 
 [case testSelfRefNT3]
-
+# flags: --disable-recursive-aliases
 from typing import NamedTuple, Tuple
 
 class B(NamedTuple):
@@ -703,7 +703,7 @@ reveal_type(lst[0]) # N: Revealed type is "Tuple[builtins.object, builtins.objec
 [builtins fixtures/tuple.pyi]
 
 [case testSelfRefNT4]
-
+# flags: --disable-recursive-aliases
 from typing import NamedTuple
 
 class B(NamedTuple):
@@ -719,7 +719,7 @@ reveal_type(n.y[0]) # N: Revealed type is "Any"
 [builtins fixtures/tuple.pyi]
 
 [case testSelfRefNT5]
-
+# flags: --disable-recursive-aliases
 from typing import NamedTuple
 
 B = NamedTuple('B', [
@@ -737,7 +737,7 @@ reveal_type(f) # N: Revealed type is "def (m: Tuple[Any, builtins.int, fallback=
 [builtins fixtures/tuple.pyi]
 
 [case testRecursiveNamedTupleInBases]
-
+# flags: --disable-recursive-aliases
 from typing import List, NamedTuple, Union
 
 Exp = Union['A', 'B']  # E: Cannot resolve name "Exp" (possible cyclic definition) \
@@ -781,7 +781,7 @@ tp = NamedTuple('tp', [('x', int)])
 [out]
 
 [case testSubclassOfRecursiveNamedTuple]
-
+# flags: --disable-recursive-aliases
 from typing import List, NamedTuple
 
 class Command(NamedTuple):
diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test
@@ -434,6 +434,7 @@ def main() -> None:
             x  # E: Name "x" is not defined
 
 [case testNewAnalyzerCyclicDefinitions]
+# flags: --disable-recursive-aliases
 gx = gy  # E: Cannot resolve name "gy" (possible cyclic definition)
 gy = gx
 def main() -> None:
@@ -1499,6 +1500,7 @@ reveal_type(x[0][0])  # N: Revealed type is "__main__.C"
 [builtins fixtures/list.pyi]
 
 [case testNewAnalyzerAliasToNotReadyDirectBase]
+# flags: --disable-recursive-aliases
 from typing import List
 
 x: B
@@ -1509,11 +1511,11 @@ reveal_type(x)
 reveal_type(x[0][0])
 [builtins fixtures/list.pyi]
 [out]
-main:3: error: Cannot resolve name "B" (possible cyclic definition)
 main:4: error: Cannot resolve name "B" (possible cyclic definition)
-main:4: error: Cannot resolve name "C" (possible cyclic definition)
-main:7: note: Revealed type is "Any"
+main:5: error: Cannot resolve name "B" (possible cyclic definition)
+main:5: error: Cannot resolve name "C" (possible cyclic definition)
 main:8: note: Revealed type is "Any"
+main:9: note: Revealed type is "Any"
 
 [case testNewAnalyzerAliasToNotReadyTwoDeferralsFunction]
 import a
@@ -1532,6 +1534,7 @@ reveal_type(f)  # N: Revealed type is "def (x: builtins.list[a.C]) -> builtins.l
 [builtins fixtures/list.pyi]
 
 [case testNewAnalyzerAliasToNotReadyDirectBaseFunction]
+# flags: --disable-recursive-aliases
 import a
 [file a.py]
 from typing import List
@@ -2119,6 +2122,7 @@ class B(List[C]):
 [builtins fixtures/list.pyi]
 
 [case testNewAnalyzerNewTypeForwardClassAliasDirect]
+# flags: --disable-recursive-aliases
 from typing import NewType, List
 
 x: D
@@ -2131,12 +2135,12 @@ class B(D):
     pass
 [builtins fixtures/list.pyi]
 [out]
-main:3: error: Cannot resolve name "D" (possible cyclic definition)
-main:4: note: Revealed type is "Any"
-main:6: error: Cannot resolve name "D" (possible cyclic definition)
-main:6: error: Cannot resolve name "C" (possible cyclic definition)
-main:7: error: Argument 2 to NewType(...) must be a valid type
-main:7: error: Cannot resolve name "B" (possible cyclic definition)
+main:4: error: Cannot resolve name "D" (possible cyclic definition)
+main:5: note: Revealed type is "Any"
+main:7: error: Cannot resolve name "D" (possible cyclic definition)
+main:7: error: Cannot resolve name "C" (possible cyclic definition)
+main:8: error: Argument 2 to NewType(...) must be a valid type
+main:8: error: Cannot resolve name "B" (possible cyclic definition)
 
 -- Copied from check-classes.test (tricky corner cases).
 [case testNewAnalyzerNoCrashForwardRefToBrokenDoubleNewTypeClass]
@@ -2153,6 +2157,7 @@ class C:
 [builtins fixtures/dict.pyi]
 
 [case testNewAnalyzerForwardTypeAliasInBase]
+# flags: --disable-recursive-aliases
 from typing import List, Generic, TypeVar, NamedTuple
 T = TypeVar('T')
 
@@ -2593,6 +2598,7 @@ import n
 def __getattr__(x): pass
 
 [case testNewAnalyzerReportLoopInMRO2]
+# flags: --disable-recursive-aliases
 def f() -> None:
     class A(A): ... # E: Cannot resolve name "A" (possible cyclic definition)
 
diff --git a/test-data/unit/check-recursive-types.test b/test-data/unit/check-recursive-types.test
diff --git a/test-data/unit/check-type-aliases.test b/test-data/unit/check-type-aliases.test
diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test
diff --git a/test-data/unit/check-unions.test b/test-data/unit/check-unions.test
diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test
diff --git a/test-data/unit/pythoneval.test b/test-data/unit/pythoneval.test

Original file line number	Diff line number	Diff line change
`@@ -975,9 +975,9 @@ def add_invertible_flag(`
`975`	`975`	`help="Use a custom typing module",`
`976`	`976`	`)`
`977`	`977`	`internals_group.add_argument(`
`978`		`- "--enable-recursive-aliases",`
	`978`	`+ "--disable-recursive-aliases",`
`979`	`979`	`action="store_true",`
`980`		`- help="Experimental support for recursive type aliases",`
	`980`	`+ help="Disable experimental support for recursive type aliases",`
`981`	`981`	`)`
`982`	`982`	`internals_group.add_argument(`
`983`	`983`	`"--custom-typeshed-dir", metavar="DIR", help="Use the custom typeshed in DIR"`
Original file line number	Diff line number	Diff line change
`@@ -203,7 +203,7 @@ def check_newtype_args(`
`203`	`203`	`self.api.anal_type(`
`204`	`204`	`unanalyzed_type,`
`205`	`205`	`report_invalid_types=False,`
`206`		`- allow_placeholder=self.options.enable_recursive_aliases`
	`206`	`+ allow_placeholder=not self.options.disable_recursive_aliases`
`207`	`207`	`and not self.api.is_func_scope(),`
`208`	`208`	`)`
`209`	`209`	`)`