Skip to content

Commit

Permalink
Optimize Unpack for failures (#15967)
Browse files Browse the repository at this point in the history
This is a small but possibly important PR. Wherever possible we should
represent user error and/or failed type inference as `*tuple[Any,
...]`/`*tuple[<nothing>, ...]`, rather than
`Unpack[Any]`/`Unpack[<nothing>]` or plain `Any`/`<nothing>`. 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).
  • Loading branch information
ilevkivskyi authored and JukkaL committed Oct 10, 2023
1 parent 4c963c9 commit 5b488ab
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 27 deletions.
23 changes: 6 additions & 17 deletions mypy/expandtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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 <nothing>.
# 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}")

Expand All @@ -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 <nothing>
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

Expand Down Expand Up @@ -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 <nothing>, 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

Expand Down
2 changes: 2 additions & 0 deletions mypy/semanal_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down
21 changes: 14 additions & 7 deletions mypy/semanal_typeargs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions test-data/unit/check-typevar-tuple.test
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@ 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 <nothing>
f(0) # E: Argument 1 to "f" has incompatible type "int"; expected "Tuple[<nothing>, ...]"

def g(a: Tuple[Unpack[Ts]], b: Tuple[Unpack[Ts]]) -> Tuple[Unpack[Ts]]:
return a

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]
Expand Down

0 comments on commit 5b488ab

Please sign in to comment.