Repository navigation
Accept inferred explicit-Any variable as base class #22037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
c56f103
55d210e
0335237
868b541
08ce848
4e5a044
e7381ca
5b2acbc
a5d92f3
febb05a
28683a3
aafedde
7337b49
6e20719
1a41018
17fab6c
4eb0004
da7c5d1
9024726
4e6dc32
4b224fd
60a8ea5
4297367
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,6 +89,7 @@ def __init__(self) -> None: | |
| is_instance_var, | ||
| ) | ||
| from mypy.checkpattern import PatternChecker | ||
| from mypy.constant_fold import constant_fold_expr | ||
| from mypy.constraints import SUPERTYPE_OF | ||
| from mypy.erasetype import ( | ||
| erase_type, | ||
|
|
@@ -207,6 +208,7 @@ def __init__(self) -> None: | |
| WithStmt, | ||
| YieldExpr, | ||
| get_func_def, | ||
| get_member_expr_fullname, | ||
| is_final_node, | ||
| ) | ||
| from mypy.operators import flip_ops, int_op_to_method, neg_ops | ||
|
|
@@ -220,6 +222,7 @@ def __init__(self) -> None: | |
| from mypy.semanal_shared import SemanticAnalyzerCoreInterface | ||
| from mypy.sharedparse import BINARY_MAGIC_METHODS | ||
| from mypy.state import state | ||
| from mypy.stats import get_original_any | ||
| from mypy.subtypes import ( | ||
| find_member, | ||
| has_any_type, | ||
|
|
@@ -266,6 +269,7 @@ def __init__(self) -> None: | |
| MYPYC_NATIVE_INT_NAMES, | ||
| NOT_IMPLEMENTED_TYPE_NAMES, | ||
| OVERLOAD_NAMES, | ||
| TUPLE_NAMES, | ||
| AnyType, | ||
| BoolTypeQuery, | ||
| CallableType, | ||
|
|
@@ -296,6 +300,7 @@ def __init__(self) -> None: | |
| UnpackType, | ||
| extend_args_for_prefix_and_suffix, | ||
| find_unpack_in_list, | ||
| flatten_nested_tuples, | ||
| flatten_nested_unions, | ||
| get_proper_type, | ||
| get_proper_types, | ||
|
|
@@ -2839,9 +2844,218 @@ def check__exit__return_type(self, defn: FuncItem) -> None: | |
| ): | ||
| self.msg.incorrect__exit__return(defn) | ||
|
|
||
| def is_explicit_any(self, typ: ProperType) -> bool: | ||
| """Is this an Any type that comes from an explicit annotation? | ||
|
|
||
| Unlike ``Any`` that leaks in from unannotated (dynamically typed) code, | ||
| an explicitly written ``Any`` means the user opted into dynamic typing, | ||
| so it is treated like an explicitly ``Any``-typed variable. | ||
| """ | ||
| if not isinstance(typ, AnyType): | ||
| return False | ||
| # Unwrap Any types that come from another Any to get the original. | ||
| typ = get_original_any(typ) | ||
| # Only accept explicitly written Any, not Any from unannotated code | ||
| # or other sources. | ||
| return typ.type_of_any == TypeOfAny.explicit | ||
|
|
||
| def check_deferred_base_classes(self, defn: ClassDef) -> None: | ||
| """Validate base classes deferred from semantic analysis. | ||
|
|
||
| A base class that is a variable with an inferred (not declared) type -- | ||
| e.g. ``x = args[1]`` where ``args[1]`` is ``Any`` -- could not be | ||
| validated during semantic analysis, since the type is only inferred by | ||
| the type checker. Such bases were provisionally treated as ``Any``; | ||
| now that the type is known, accept it if it is ``Any`` (like an | ||
| explicitly ``Any``-typed variable) and otherwise report the errors that | ||
| semantic analysis deferred. | ||
|
|
||
| A base class may also subscript a variable directly, e.g. | ||
| ``class Sub(args[1])``. Then the item type at the subscript is what | ||
| gets validated, in the same way. | ||
| """ | ||
| if not self.in_checked_function(): | ||
| # Semantic analysis silently drops these errors in unchecked | ||
| # functions; do the same here to avoid new reports. | ||
| return | ||
| for deferred in defn.info.deferred_base_classes: | ||
| var = deferred.var | ||
| base_expr = deferred.expr | ||
| defining_literal = deferred.defining_literal | ||
| if defining_literal: | ||
| # Semantic analysis does not report "not valid as a type" for | ||
| # variables inside Literal[...]; do the same here. | ||
| continue | ||
| if isinstance(base_expr, IndexExpr): | ||
| typ = self.deferred_index_base_type(var, base_expr) | ||
| else: | ||
| var_type = var.type | ||
| typ = get_proper_type(var_type) if var_type is not None else None | ||
| # Mirror the cases semantic analysis accepts for an explicitly | ||
| # typed variable (see TypeAnalyser.analyze_unbound_type_without_type_info). | ||
| # An inferred plain ``Any`` is only accepted if it comes from an | ||
| # explicit annotation; ``Any`` leaking in from unannotated code is | ||
| # still rejected, as before. | ||
| is_any = typ is not None and self.is_valid_deferred_base_type(typ) | ||
| if is_any: | ||
| if self.options.disallow_subclassing_any: | ||
| if isinstance(base_expr, (NameExpr, MemberExpr)): | ||
| msg = f'Class cannot subclass "{base_expr.name}" (has type "Any")' | ||
| else: | ||
| msg = 'Class cannot subclass value of type "Any"' | ||
| self.fail(msg, base_expr) | ||
| continue | ||
| if isinstance(base_expr, NameExpr): | ||
| name_repr: str | None = base_expr.name | ||
| elif isinstance(base_expr, MemberExpr): | ||
| name_repr = get_member_expr_fullname(base_expr) | ||
| elif isinstance(base_expr, IndexExpr): | ||
| name_repr = var.name | ||
| else: | ||
| name_repr = None | ||
| self.fail( | ||
| f'Variable "{var.fullname}" is not valid as a type', | ||
| base_expr, | ||
| code=codes.VALID_TYPE, | ||
| ) | ||
| self.note( | ||
| "See https://mypy.readthedocs.io/en/stable/common_issues.html" | ||
| "#variables-vs-type-aliases", | ||
| base_expr, | ||
| code=codes.VALID_TYPE, | ||
| ) | ||
| msg = "Invalid base class" | ||
| if name_repr: | ||
| msg += f' "{name_repr}"' | ||
| self.fail(msg, base_expr) | ||
| if isinstance(base_expr, IndexExpr): | ||
| index_value = self.index_expr_int_value(base_expr.index) | ||
| if index_value is not None: | ||
| # Mirror the hint semantic analysis reports for an | ||
| # integer used as a type argument. | ||
| self.fail( | ||
| f"Invalid type: try using Literal[{index_value}] instead?", | ||
| base_expr, | ||
| code=codes.VALID_TYPE, | ||
| ) | ||
|
|
||
| def is_valid_deferred_base_type(self, typ: ProperType) -> bool: | ||
| """Can a deferred base class have this inferred type? | ||
|
|
||
| Mirror what semantic analysis accepts for an explicitly typed | ||
| variable: ``Any`` from an explicit annotation, a type object, or a | ||
| union of those (e.g. ``type | Any``, when an ambiguous tuple index | ||
| may land on either). Anything else is rejected. | ||
| """ | ||
| if isinstance(typ, UnionType): | ||
| return all( | ||
| self.is_valid_deferred_base_type(get_proper_type(item)) for item in typ.items | ||
| ) | ||
| return ( | ||
| self.is_explicit_any(typ) | ||
| or (isinstance(typ, Instance) and typ.type.fullname == "builtins.type") | ||
| or (isinstance(typ, TypeType) and self.is_explicit_any(typ.item)) | ||
| ) | ||
|
|
||
| @staticmethod | ||
| def index_expr_int_value(index: Expression) -> int | None: | ||
| """Value of a constant integer index expression, or None.""" | ||
| value = constant_fold_expr(index, "<unused>") | ||
| if isinstance(value, int) and not isinstance(value, bool): | ||
| return value | ||
| return None | ||
|
|
||
| def deferred_index_base_type(self, var: Var, base_expr: IndexExpr) -> ProperType | None: | ||
| """Type of a deferred subscripted base class, e.g. ``args[1]``. | ||
|
|
||
| ``var`` is the subscripted variable. Return the type of the item at | ||
| the subscript, or None if it cannot be determined statically; the | ||
| base class is then rejected, as it was before deferred validation. | ||
| Only a constant integer index is supported, and only into a tuple | ||
| or sequence type, mirroring what can be known about the item type. | ||
| """ | ||
| if not isinstance(base_expr.base, NameExpr): | ||
| return None | ||
| index_value = self.index_expr_int_value(base_expr.index) | ||
| if index_value is None: | ||
| return None | ||
| var_type = get_proper_type(var.type) if var.type is not None else None | ||
| if isinstance(var_type, TupleType): | ||
| return self.tuple_item_at_index(var_type, index_value) | ||
| if isinstance(var_type, Instance): | ||
| if var_type.type.fullname in TUPLE_NAMES and var_type.args: | ||
| # Homogeneous tuple[T, ...]: every item has type T. | ||
| return get_proper_type(var_type.args[0]) | ||
| sequence_info = var_type.type.get_base("typing.Sequence") | ||
| if sequence_info is not None: | ||
| mapped = map_instance_to_supertype(var_type, sequence_info) | ||
| if mapped.args: | ||
| return get_proper_type(mapped.args[0]) | ||
| return None | ||
|
|
||
| def tuple_item_at_index(self, tuple_type: TupleType, index: int) -> ProperType | None: | ||
| """Type of a tuple item at a constant index, or None if ambiguous. | ||
|
|
||
| If the tuple type has a variadic (unpacked) item, an index may land | ||
| on more than one item. Then the union of the items it may land on | ||
| is returned, and the caller decides whether that union is an | ||
| acceptable base class type. A single fixed item (or ``Any``, if | ||
| every candidate is ``Any``) is returned directly. | ||
| """ | ||
| items = [get_proper_type(item) for item in flatten_nested_tuples(tuple_type.items)] | ||
| unpack_index = find_unpack_in_list(items) | ||
| if unpack_index is None: | ||
| pos = index if index >= 0 else index + len(items) | ||
| if 0 <= pos < len(items): | ||
| return items[pos] | ||
| return None | ||
| unpack_item = items[unpack_index] | ||
| assert isinstance(unpack_item, UnpackType) | ||
| unpacked = get_proper_type(unpack_item.type) | ||
| if not ( | ||
| isinstance(unpacked, Instance) | ||
| and unpacked.type.fullname in TUPLE_NAMES | ||
| and unpacked.args | ||
| ): | ||
| return None | ||
| variadic_item = get_proper_type(unpacked.args[0]) | ||
| prefix = items[:unpack_index] | ||
| suffix = items[unpack_index + 1 :] | ||
| possible: list[ProperType] | ||
| if index >= 0: | ||
| if index < len(prefix): | ||
| possible = [prefix[index]] | ||
| else: | ||
| possible = [variadic_item] | ||
| # A suffix item sits at position len(prefix) + <variadic | ||
| # length> + k, and the variadic length is unknown, so it may | ||
| # also land on this index. | ||
| for k, item in enumerate(suffix): | ||
| if len(prefix) + k <= index: | ||
| possible.append(item) | ||
| else: | ||
| from_end = -index | ||
| if from_end <= len(suffix): | ||
| possible = [suffix[len(suffix) - from_end]] | ||
| else: | ||
| possible = [variadic_item] | ||
| # A prefix item sits at distance len(suffix) + <variadic | ||
| # length> + (len(prefix) - k) from the end. | ||
| for k, item in enumerate(prefix): | ||
| if len(suffix) + (len(prefix) - k) <= from_end: | ||
| possible.append(item) | ||
| if all(isinstance(item, AnyType) for item in possible): | ||
| return possible[0] | ||
|
Comment on lines
+3047
to
+3048
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about if there are a mix of possible E.g. I think the following should pass ( (Note Noting that in general variables of type So given that |
||
| if len(possible) == 1: | ||
| return possible[0] | ||
| # The index may land on several items; return their union and let | ||
| # the caller decide whether it is an acceptable base class type. | ||
| return make_simplified_union(possible) | ||
|
|
||
| def visit_class_def(self, defn: ClassDef) -> None: | ||
| """Type check a class definition.""" | ||
| typ = defn.info | ||
| self.check_deferred_base_classes(defn) | ||
| for base in typ.mro[1:]: | ||
| if base.is_final: | ||
| self.fail(message_registry.CANNOT_INHERIT_FROM_FINAL.format(base.name), defn) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you are implementing brittle handling for only the simple cases of IndexExpr being an IntExpr or an int primitive modified by a negative sign, instead of handling general Expressions.
Consider adding test cases where
args[1]is typeAnyand confirming that e.g.args[-(-1)]also works in that case, for example by adding another 2 lines to your existing testtestSubclassAnyFromNestedUnpackedUnboundedStarArgsIndexInsideUnambiguousto add another Sub class using one of theargsentries as its base, but using a slightly more complicated double negative expression as the IndexExpr:Currently your latest version fails such a check but I think we would expect such to succeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved in 4e6dc32 .