From 5b488ab8ad82ce11388a63367a60e1443f359f04 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 29 Aug 2023 10:17:52 +0100 Subject: [PATCH] Optimize Unpack for failures (#15967) This is a small but possibly important PR. Wherever possible we should represent user error and/or failed type inference as `*tuple[Any, ...]`/`*tuple[, ...]`, rather than `Unpack[Any]`/`Unpack[]` or plain `Any`/``. This way we will not need any special casing for failure conditions in various places without risking a crash instead of a graceful failure (error message). --- mypy/expandtype.py | 23 ++++++----------------- mypy/semanal_main.py | 2 ++ mypy/semanal_typeargs.py | 21 ++++++++++++++------- test-data/unit/check-typevar-tuple.test | 5 ++--- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/mypy/expandtype.py b/mypy/expandtype.py index ef8ebe1a9128..26353c043cb7 100644 --- a/mypy/expandtype.py +++ b/mypy/expandtype.py @@ -273,7 +273,7 @@ def visit_unpack_type(self, t: UnpackType) -> Type: # example is non-normalized types when called from semanal.py. return UnpackType(t.type.accept(self)) - def expand_unpack(self, t: UnpackType) -> list[Type] | AnyType | UninhabitedType: + def expand_unpack(self, t: UnpackType) -> list[Type]: assert isinstance(t.type, TypeVarTupleType) repl = get_proper_type(self.variables.get(t.type.id, t.type)) if isinstance(repl, TupleType): @@ -285,9 +285,9 @@ def expand_unpack(self, t: UnpackType) -> list[Type] | AnyType | UninhabitedType ): return [UnpackType(typ=repl)] elif isinstance(repl, (AnyType, UninhabitedType)): - # tuple[Any, ...] for Any would be better, but we don't have - # the type info to construct that type here. - return repl + # Replace *Ts = Any with *Ts = *tuple[Any, ...] and some for . + # These types may appear here as a result of user error or failed inference. + return [UnpackType(t.type.tuple_fallback.copy_modified(args=[repl]))] else: raise RuntimeError(f"Invalid type replacement to expand: {repl}") @@ -310,12 +310,7 @@ def interpolate_args_for_unpack(self, t: CallableType, var_arg: UnpackType) -> l # We have plain Unpack[Ts] assert isinstance(var_arg_type, TypeVarTupleType) fallback = var_arg_type.tuple_fallback - expanded_items_res = self.expand_unpack(var_arg) - if isinstance(expanded_items_res, list): - expanded_items = expanded_items_res - else: - # We got Any or - return prefix + [expanded_items_res] + suffix + expanded_items = self.expand_unpack(var_arg) new_unpack = UnpackType(TupleType(expanded_items, fallback)) return prefix + [new_unpack] + suffix @@ -394,14 +389,8 @@ def expand_types_with_unpack( items: list[Type] = [] for item in typs: if isinstance(item, UnpackType) and isinstance(item.type, TypeVarTupleType): - unpacked_items = self.expand_unpack(item) - if isinstance(unpacked_items, (AnyType, UninhabitedType)): - # TODO: better error for , something like tuple of unknown? - return unpacked_items - else: - items.extend(unpacked_items) + items.extend(self.expand_unpack(item)) else: - # Must preserve original aliases when possible. items.append(item.accept(self)) return items diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 51a7014fac1a..ec09deb0952f 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -381,6 +381,7 @@ def check_type_arguments(graph: Graph, scc: list[str], errors: Errors) -> None: errors, state.options, is_typeshed_file(state.options.abs_custom_typeshed_dir, state.path or ""), + state.manager.semantic_analyzer.named_type, ) with state.wrap_context(): with mypy.state.state.strict_optional_set(state.options.strict_optional): @@ -399,6 +400,7 @@ def check_type_arguments_in_targets( errors, state.options, is_typeshed_file(state.options.abs_custom_typeshed_dir, state.path or ""), + state.manager.semantic_analyzer.named_type, ) with state.wrap_context(): with mypy.state.state.strict_optional_set(state.options.strict_optional): diff --git a/mypy/semanal_typeargs.py b/mypy/semanal_typeargs.py index 1ae6fada8f38..749b02391e06 100644 --- a/mypy/semanal_typeargs.py +++ b/mypy/semanal_typeargs.py @@ -7,7 +7,7 @@ from __future__ import annotations -from typing import Sequence +from typing import Callable, Sequence from mypy import errorcodes as codes, message_registry from mypy.errorcodes import ErrorCode @@ -42,11 +42,18 @@ class TypeArgumentAnalyzer(MixedTraverserVisitor): - def __init__(self, errors: Errors, options: Options, is_typeshed_file: bool) -> None: + def __init__( + self, + errors: Errors, + options: Options, + is_typeshed_file: bool, + named_type: Callable[[str, list[Type]], Instance], + ) -> None: super().__init__() self.errors = errors self.options = options self.is_typeshed_file = is_typeshed_file + self.named_type = named_type self.scope = Scope() # Should we also analyze function definitions, or only module top-levels? self.recurse_into_functions = True @@ -243,16 +250,16 @@ def visit_unpack_type(self, typ: UnpackType) -> None: return if isinstance(proper_type, TypeVarTupleType): return + # TODO: this should probably be .has_base("builtins.tuple"), also elsewhere. if isinstance(proper_type, Instance) and proper_type.type.fullname == "builtins.tuple": return - if isinstance(proper_type, AnyType) and proper_type.type_of_any == TypeOfAny.from_error: - return - if not isinstance(proper_type, UnboundType): - # Avoid extra errors if there were some errors already. + if not isinstance(proper_type, (UnboundType, AnyType)): + # Avoid extra errors if there were some errors already. Also interpret plain Any + # as tuple[Any, ...] (this is better for the code in type checker). self.fail( message_registry.INVALID_UNPACK.format(format_type(proper_type, self.options)), typ ) - typ.type = AnyType(TypeOfAny.from_error) + typ.type = self.named_type("builtins.tuple", [AnyType(TypeOfAny.from_error)]) def check_type_var_values( self, name: str, actuals: list[Type], arg_name: str, valids: list[Type], context: Context diff --git a/test-data/unit/check-typevar-tuple.test b/test-data/unit/check-typevar-tuple.test index a36c4d4d6741..c8b33ec96b06 100644 --- a/test-data/unit/check-typevar-tuple.test +++ b/test-data/unit/check-typevar-tuple.test @@ -17,8 +17,7 @@ reveal_type(f(args)) # N: Revealed type is "Tuple[builtins.int, builtins.str]" reveal_type(f(varargs)) # N: Revealed type is "builtins.tuple[builtins.int, ...]" -if object(): - f(0) # E: Argument 1 to "f" has incompatible type "int"; expected +f(0) # E: Argument 1 to "f" has incompatible type "int"; expected "Tuple[, ...]" def g(a: Tuple[Unpack[Ts]], b: Tuple[Unpack[Ts]]) -> Tuple[Unpack[Ts]]: return a @@ -26,7 +25,7 @@ def g(a: Tuple[Unpack[Ts]], b: Tuple[Unpack[Ts]]) -> Tuple[Unpack[Ts]]: reveal_type(g(args, args)) # N: Revealed type is "Tuple[builtins.int, builtins.str]" reveal_type(g(args, args2)) # N: Revealed type is "Tuple[builtins.int, builtins.str]" reveal_type(g(args, args3)) # N: Revealed type is "builtins.tuple[builtins.object, ...]" -reveal_type(g(any, any)) # N: Revealed type is "Any" +reveal_type(g(any, any)) # N: Revealed type is "builtins.tuple[Any, ...]" [builtins fixtures/tuple.pyi] [case testTypeVarTupleMixed]