From 337bcf9ec3de40ec195d52d5615b875c3fb8ea26 Mon Sep 17 00:00:00 2001 From: Ali Hamdan Date: Fri, 29 Mar 2024 00:49:33 +0100 Subject: [PATCH] Improve error message for bound typevar in TypeAliasType (#17053) Follow up to #17038 When a type variable is bound to a class, it cannot be reused in a type alias. Previously in `TypeAliasType`, this error was reported as "not included in type_params". However in the following example, the error is misleading: ```python from typing import Dict, Generic, TypeVar from typing_extensions import TypeAliasType T = TypeVar("T") class A(Generic[T]): Ta11 = TypeAliasType("Ta11", Dict[str, T], type_params=(T,)) x: A.Ta11 = {"a": 1} reveal_type(x) ``` On the master branch: ``` main.py:8: error: Type variable "T" is not included in type_params [valid-type] main.py:8: error: "T" is a type variable and only valid in type context [misc] main.py:8: error: Free type variable expected in type_params argument to TypeAliasType [type-var] main.py:12: note: Revealed type is "builtins.dict[builtins.str, Any]" Found 3 errors in 1 file (checked 1 source file) ``` With this PR: ``` typealiastype.py:8: error: Can't use bound type variable "T" to define generic alias [valid-type] typealiastype.py:8: error: "T" is a type variable and only valid in type context [misc] typealiastype.py:12: note: Revealed type is "builtins.dict[builtins.str, Any]" Found 2 errors in 1 file (checked 1 source file) ``` This is possible by storing the names of all the declared type_params, even those that are invalid, and checking if the offending type variables are in the list. --- mypy/semanal.py | 49 ++++++++++++++++++-------- mypy/typeanal.py | 26 ++++++++------ test-data/unit/check-type-aliases.test | 5 +++ 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index 5aaf2bc6f433..6832e767c3a4 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3521,6 +3521,7 @@ def analyze_alias( rvalue: Expression, allow_placeholder: bool = False, declared_type_vars: TypeVarLikeList | None = None, + all_declared_type_params_names: list[str] | None = None, ) -> tuple[Type | None, list[TypeVarLikeType], set[str], list[str], bool]: """Check if 'rvalue' is a valid type allowed for aliasing (e.g. not a type variable). @@ -3573,7 +3574,7 @@ def analyze_alias( in_dynamic_func=dynamic, global_scope=global_scope, allowed_alias_tvars=tvar_defs, - has_type_params=declared_type_vars is not None, + alias_type_params_names=all_declared_type_params_names, ) # There can be only one variadic variable at most, the error is reported elsewhere. @@ -3622,14 +3623,16 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: # It can be `A = TypeAliasType('A', ...)` call, in this case, # we just take the second argument and analyze it: type_params: TypeVarLikeList | None + all_type_params_names: list[str] | None if self.check_type_alias_type_call(s.rvalue, name=lvalue.name): rvalue = s.rvalue.args[1] pep_695 = True - type_params = self.analyze_type_alias_type_params(s.rvalue) + type_params, all_type_params_names = self.analyze_type_alias_type_params(s.rvalue) else: rvalue = s.rvalue pep_695 = False type_params = None + all_type_params_names = None if isinstance(rvalue, CallExpr) and rvalue.analyzed: return False @@ -3686,7 +3689,11 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool: else: tag = self.track_incomplete_refs() res, alias_tvars, depends_on, qualified_tvars, empty_tuple_index = self.analyze_alias( - lvalue.name, rvalue, allow_placeholder=True, declared_type_vars=type_params + lvalue.name, + rvalue, + allow_placeholder=True, + declared_type_vars=type_params, + all_declared_type_params_names=all_type_params_names, ) if not res: return False @@ -3803,7 +3810,14 @@ def check_type_alias_type_call(self, rvalue: Expression, *, name: str) -> TypeGu return self.check_typevarlike_name(rvalue, name, rvalue) - def analyze_type_alias_type_params(self, rvalue: CallExpr) -> TypeVarLikeList: + def analyze_type_alias_type_params( + self, rvalue: CallExpr + ) -> tuple[TypeVarLikeList, list[str]]: + """Analyze type_params of TypeAliasType. + + Returns declared unbound type variable expressions and a list of all decalred type + variable names for error reporting. + """ if "type_params" in rvalue.arg_names: type_params_arg = rvalue.args[rvalue.arg_names.index("type_params")] if not isinstance(type_params_arg, TupleExpr): @@ -3811,12 +3825,13 @@ def analyze_type_alias_type_params(self, rvalue: CallExpr) -> TypeVarLikeList: "Tuple literal expected as the type_params argument to TypeAliasType", type_params_arg, ) - return [] + return [], [] type_params = type_params_arg.items else: - type_params = [] + return [], [] declared_tvars: TypeVarLikeList = [] + all_declared_tvar_names: list[str] = [] # includes bound type variables have_type_var_tuple = False for tp_expr in type_params: if isinstance(tp_expr, StarExpr): @@ -3843,16 +3858,19 @@ def analyze_type_alias_type_params(self, rvalue: CallExpr) -> TypeVarLikeList: continue have_type_var_tuple = True elif not self.found_incomplete_ref(tag): - self.fail( - "Free type variable expected in type_params argument to TypeAliasType", - base, - code=codes.TYPE_VAR, - ) sym = self.lookup_qualified(base.name, base) - if sym and sym.fullname in ("typing.Unpack", "typing_extensions.Unpack"): - self.note( - "Don't Unpack type variables in type_params", base, code=codes.TYPE_VAR + if sym and isinstance(sym.node, TypeVarLikeExpr): + all_declared_tvar_names.append(sym.node.name) # Error will be reported later + else: + self.fail( + "Free type variable expected in type_params argument to TypeAliasType", + base, + code=codes.TYPE_VAR, ) + if sym and sym.fullname in ("typing.Unpack", "typing_extensions.Unpack"): + self.note( + "Don't Unpack type variables in type_params", base, code=codes.TYPE_VAR + ) continue if tvar in declared_tvars: self.fail( @@ -3862,8 +3880,9 @@ def analyze_type_alias_type_params(self, rvalue: CallExpr) -> TypeVarLikeList: ) continue if tvar: + all_declared_tvar_names.append(tvar[0]) declared_tvars.append(tvar) - return declared_tvars + return declared_tvars, all_declared_tvar_names def disable_invalid_recursive_aliases( self, s: AssignmentStmt, current_node: TypeAlias diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 470b07948535..3f4b86185f2d 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -141,7 +141,7 @@ def analyze_type_alias( in_dynamic_func: bool = False, global_scope: bool = True, allowed_alias_tvars: list[TypeVarLikeType] | None = None, - has_type_params: bool = False, + alias_type_params_names: list[str] | None = None, ) -> tuple[Type, set[str]]: """Analyze r.h.s. of a (potential) type alias definition. @@ -159,7 +159,7 @@ def analyze_type_alias( allow_placeholder=allow_placeholder, prohibit_self_type="type alias target", allowed_alias_tvars=allowed_alias_tvars, - has_type_params=has_type_params, + alias_type_params_names=alias_type_params_names, ) analyzer.in_dynamic_func = in_dynamic_func analyzer.global_scope = global_scope @@ -212,7 +212,7 @@ def __init__( prohibit_self_type: str | None = None, allowed_alias_tvars: list[TypeVarLikeType] | None = None, allow_type_any: bool = False, - has_type_params: bool = False, + alias_type_params_names: list[str] | None = None, ) -> None: self.api = api self.fail_func = api.fail @@ -234,7 +234,7 @@ def __init__( if allowed_alias_tvars is None: allowed_alias_tvars = [] self.allowed_alias_tvars = allowed_alias_tvars - self.has_type_params = has_type_params + self.alias_type_params_names = alias_type_params_names # If false, record incomplete ref if we generate PlaceholderType. self.allow_placeholder = allow_placeholder # Are we in a context where Required[] is allowed? @@ -275,6 +275,12 @@ def visit_unbound_type(self, t: UnboundType, defining_literal: bool = False) -> return make_optional_type(typ) return typ + def not_declared_in_type_params(self, tvar_name: str) -> bool: + return ( + self.alias_type_params_names is not None + and tvar_name not in self.alias_type_params_names + ) + def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) -> Type: sym = self.lookup_qualified(t.name, t) if sym is not None: @@ -329,7 +335,7 @@ def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) if tvar_def is None: if self.allow_unbound_tvars: return t - if self.defining_alias and self.has_type_params: + if self.defining_alias and self.not_declared_in_type_params(t.name): msg = f'ParamSpec "{t.name}" is not included in type_params' else: msg = f'ParamSpec "{t.name}" is unbound' @@ -357,7 +363,7 @@ def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) and not defining_literal and (tvar_def is None or tvar_def not in self.allowed_alias_tvars) ): - if self.has_type_params: + if self.not_declared_in_type_params(t.name): msg = f'Type variable "{t.name}" is not included in type_params' else: msg = f'Can\'t use bound type variable "{t.name}" to define generic alias' @@ -376,7 +382,7 @@ def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) and self.defining_alias and tvar_def not in self.allowed_alias_tvars ): - if self.has_type_params: + if self.not_declared_in_type_params(t.name): msg = f'Type variable "{t.name}" is not included in type_params' else: msg = f'Can\'t use bound type variable "{t.name}" to define generic alias' @@ -386,7 +392,7 @@ def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) if tvar_def is None: if self.allow_unbound_tvars: return t - if self.defining_alias and self.has_type_params: + if self.defining_alias and self.not_declared_in_type_params(t.name): msg = f'TypeVarTuple "{t.name}" is not included in type_params' else: msg = f'TypeVarTuple "{t.name}" is unbound' @@ -1281,11 +1287,11 @@ def analyze_callable_args_for_paramspec( return None elif ( self.defining_alias - and self.has_type_params + and self.not_declared_in_type_params(tvar_def.name) and tvar_def not in self.allowed_alias_tvars ): self.fail( - f'ParamSpec "{callable_args.name}" is not included in type_params', + f'ParamSpec "{tvar_def.name}" is not included in type_params', callable_args, code=codes.VALID_TYPE, ) diff --git a/test-data/unit/check-type-aliases.test b/test-data/unit/check-type-aliases.test index 7330a04c3647..a9c57d46ad22 100644 --- a/test-data/unit/check-type-aliases.test +++ b/test-data/unit/check-type-aliases.test @@ -1195,6 +1195,11 @@ reveal_type(unbound_ps_alias3) # N: Revealed type is "def [P] (*Any, **Any) -> #unbound_tvt_alias2: Ta10[int] #reveal_type(unbound_tvt_alias2) +class A(Generic[T]): + Ta11 = TypeAliasType("Ta11", Dict[str, T], type_params=(T,)) # E: Can't use bound type variable "T" to define generic alias \ + # E: "T" is a type variable and only valid in type context +x: A.Ta11 = {"a": 1} +reveal_type(x) # N: Revealed type is "builtins.dict[builtins.str, Any]" [builtins fixtures/dict.pyi] [case testTypeAliasTypeNoUnpackInTypeParams311]