Skip to content

Commit

Permalink
Allow function arguments as base classes (#14135)
Browse files Browse the repository at this point in the history
Fixes #5865 

Looks quite easy and safe, unless I am missing something. Most changes
in the diff are just moving stuff around.

Previously we only applied argument types before type checking, but it
looks like we can totally do this in semantic analyzer. I also enable
variable annotated as `type` (or equivalently `Type[Any]`), this use
case was mentioned in the comments.

This PR also accidentally fixes two additional bugs, one related to type
variables with values vs walrus operator, another one for type variables
with values vs self types. I include test cases for those as well.
  • Loading branch information
ilevkivskyi authored Nov 21, 2022
1 parent c660354 commit 3c5f368
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 32 deletions.
29 changes: 3 additions & 26 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@
LiteralType,
NoneType,
Overloaded,
ParamSpecType,
PartialType,
ProperType,
StarType,
Expand All @@ -203,14 +202,14 @@
UnboundType,
UninhabitedType,
UnionType,
UnpackType,
flatten_nested_unions,
get_proper_type,
get_proper_types,
is_literal_type,
is_named_instance,
is_optional,
remove_optional,
store_argument_type,
strip_type,
)
from mypy.typetraverser import TypeTraverserVisitor
Expand Down Expand Up @@ -1174,30 +1173,8 @@ def check_func_def(
if ctx.line < 0:
ctx = typ
self.fail(message_registry.FUNCTION_PARAMETER_CANNOT_BE_COVARIANT, ctx)
if typ.arg_kinds[i] == nodes.ARG_STAR:
if isinstance(arg_type, ParamSpecType):
pass
elif isinstance(arg_type, UnpackType):
if isinstance(get_proper_type(arg_type.type), TupleType):
# Instead of using Tuple[Unpack[Tuple[...]]], just use
# Tuple[...]
arg_type = arg_type.type
else:
arg_type = TupleType(
[arg_type],
fallback=self.named_generic_type(
"builtins.tuple", [self.named_type("builtins.object")]
),
)
else:
# builtins.tuple[T] is typing.Tuple[T, ...]
arg_type = self.named_generic_type("builtins.tuple", [arg_type])
elif typ.arg_kinds[i] == nodes.ARG_STAR2:
if not isinstance(arg_type, ParamSpecType) and not typ.unpack_kwargs:
arg_type = self.named_generic_type(
"builtins.dict", [self.str_type(), arg_type]
)
item.arguments[i].variable.type = arg_type
# Need to store arguments again for the expanded item.
store_argument_type(item, i, typ, self.named_generic_type)

# Type check initialization expressions.
body_is_trivial = is_trivial_body(defn.body)
Expand Down
25 changes: 21 additions & 4 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@
get_proper_types,
invalid_recursive_alias,
is_named_instance,
store_argument_type,
)
from mypy.typevars import fill_typevars
from mypy.util import (
Expand Down Expand Up @@ -1315,7 +1316,10 @@ def analyze_function_body(self, defn: FuncItem) -> None:
# Bind the type variables again to visit the body.
if defn.type:
a = self.type_analyzer()
a.bind_function_type_variables(cast(CallableType, defn.type), defn)
typ = cast(CallableType, defn.type)
a.bind_function_type_variables(typ, defn)
for i in range(len(typ.arg_types)):
store_argument_type(defn, i, typ, self.named_type)
self.function_stack.append(defn)
with self.enter(defn):
for arg in defn.arguments:
Expand Down Expand Up @@ -2018,7 +2022,9 @@ def analyze_base_classes(
continue

try:
base = self.expr_to_analyzed_type(base_expr, allow_placeholder=True)
base = self.expr_to_analyzed_type(
base_expr, allow_placeholder=True, allow_type_any=True
)
except TypeTranslationError:
name = self.get_name_repr_of_expr(base_expr)
if isinstance(base_expr, CallExpr):
Expand Down Expand Up @@ -6139,7 +6145,11 @@ def accept(self, node: Node) -> None:
report_internal_error(err, self.errors.file, node.line, self.errors, self.options)

def expr_to_analyzed_type(
self, expr: Expression, report_invalid_types: bool = True, allow_placeholder: bool = False
self,
expr: Expression,
report_invalid_types: bool = True,
allow_placeholder: bool = False,
allow_type_any: bool = False,
) -> Type | None:
if isinstance(expr, CallExpr):
# This is a legacy syntax intended mostly for Python 2, we keep it for
Expand All @@ -6164,7 +6174,10 @@ def expr_to_analyzed_type(
return TupleType(info.tuple_type.items, fallback=fallback)
typ = self.expr_to_unanalyzed_type(expr)
return self.anal_type(
typ, report_invalid_types=report_invalid_types, allow_placeholder=allow_placeholder
typ,
report_invalid_types=report_invalid_types,
allow_placeholder=allow_placeholder,
allow_type_any=allow_type_any,
)

def analyze_type_expr(self, expr: Expression) -> None:
Expand All @@ -6188,6 +6201,7 @@ def type_analyzer(
allow_param_spec_literals: bool = False,
report_invalid_types: bool = True,
prohibit_self_type: str | None = None,
allow_type_any: bool = False,
) -> TypeAnalyser:
if tvar_scope is None:
tvar_scope = self.tvar_scope
Expand All @@ -6204,6 +6218,7 @@ def type_analyzer(
allow_required=allow_required,
allow_param_spec_literals=allow_param_spec_literals,
prohibit_self_type=prohibit_self_type,
allow_type_any=allow_type_any,
)
tpan.in_dynamic_func = bool(self.function_stack and self.function_stack[-1].is_dynamic())
tpan.global_scope = not self.type and not self.function_stack
Expand All @@ -6224,6 +6239,7 @@ def anal_type(
allow_param_spec_literals: bool = False,
report_invalid_types: bool = True,
prohibit_self_type: str | None = None,
allow_type_any: bool = False,
third_pass: bool = False,
) -> Type | None:
"""Semantically analyze a type.
Expand Down Expand Up @@ -6260,6 +6276,7 @@ def anal_type(
allow_param_spec_literals=allow_param_spec_literals,
report_invalid_types=report_invalid_types,
prohibit_self_type=prohibit_self_type,
allow_type_any=allow_type_any,
)
tag = self.track_incomplete_refs()
typ = typ.accept(a)
Expand Down
2 changes: 1 addition & 1 deletion mypy/stubtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ def _verify_final(
) -> Iterator[Error]:
try:

class SubClass(runtime): # type: ignore[misc,valid-type]
class SubClass(runtime): # type: ignore[misc]
pass

except TypeError:
Expand Down
2 changes: 1 addition & 1 deletion mypy/treetransform.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ def visit_super_expr(self, node: SuperExpr) -> SuperExpr:
return new

def visit_assignment_expr(self, node: AssignmentExpr) -> AssignmentExpr:
return AssignmentExpr(node.target, node.value)
return AssignmentExpr(self.expr(node.target), self.expr(node.value))

def visit_unary_expr(self, node: UnaryExpr) -> UnaryExpr:
new = UnaryExpr(node.op, self.expr(node.expr))
Expand Down
8 changes: 8 additions & 0 deletions mypy/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ def __init__(
allow_param_spec_literals: bool = False,
report_invalid_types: bool = True,
prohibit_self_type: str | None = None,
allow_type_any: bool = False,
) -> None:
self.api = api
self.lookup_qualified = api.lookup_qualified
Expand Down Expand Up @@ -237,6 +238,8 @@ def __init__(
# Names of type aliases encountered while analysing a type will be collected here.
self.aliases_used: set[str] = set()
self.prohibit_self_type = prohibit_self_type
# Allow variables typed as Type[Any] and type (useful for base classes).
self.allow_type_any = allow_type_any

def visit_unbound_type(self, t: UnboundType, defining_literal: bool = False) -> Type:
typ = self.visit_unbound_type_nonoptional(t, defining_literal)
Expand Down Expand Up @@ -730,6 +733,11 @@ def analyze_unbound_type_without_type_info(
return AnyType(
TypeOfAny.from_unimported_type, missing_import_name=typ.missing_import_name
)
elif self.allow_type_any:
if isinstance(typ, Instance) and typ.type.fullname == "builtins.type":
return AnyType(TypeOfAny.special_form)
if isinstance(typ, TypeType) and isinstance(typ.item, AnyType):
return AnyType(TypeOfAny.from_another_any, source_any=typ.item)
# Option 2:
# Unbound type variable. Currently these may be still valid,
# for example when defining a generic type alias.
Expand Down
28 changes: 28 additions & 0 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from typing import (
TYPE_CHECKING,
Any,
Callable,
ClassVar,
Dict,
Iterable,
Expand All @@ -29,6 +30,7 @@
ArgKind,
FakeInfo,
FuncDef,
FuncItem,
SymbolNode,
)
from mypy.state import state
Expand Down Expand Up @@ -3402,3 +3404,29 @@ def callable_with_ellipsis(any_type: AnyType, ret_type: Type, fallback: Instance
fallback=fallback,
is_ellipsis_args=True,
)


def store_argument_type(
defn: FuncItem, i: int, typ: CallableType, named_type: Callable[[str, list[Type]], Instance]
) -> None:
arg_type = typ.arg_types[i]
if typ.arg_kinds[i] == ARG_STAR:
if isinstance(arg_type, ParamSpecType):
pass
elif isinstance(arg_type, UnpackType):
if isinstance(get_proper_type(arg_type.type), TupleType):
# Instead of using Tuple[Unpack[Tuple[...]]], just use
# Tuple[...]
arg_type = arg_type.type
else:
arg_type = TupleType(
[arg_type],
fallback=named_type("builtins.tuple", [named_type("builtins.object", [])]),
)
else:
# builtins.tuple[T] is typing.Tuple[T, ...]
arg_type = named_type("builtins.tuple", [arg_type])
elif typ.arg_kinds[i] == ARG_STAR2:
if not isinstance(arg_type, ParamSpecType) and not typ.unpack_kwargs:
arg_type = named_type("builtins.dict", [named_type("builtins.str", []), arg_type])
defn.arguments[i].variable.type = arg_type
15 changes: 15 additions & 0 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -7664,3 +7664,18 @@ class C(B):
def foo(self) -> int: # E: Signature of "foo" incompatible with supertype "B"
...
[builtins fixtures/property.pyi]

[case testAllowArgumentAsBaseClass]
from typing import Any, Type

def e(b) -> None:
class D(b): ...

def f(b: Any) -> None:
class D(b): ...

def g(b: Type[Any]) -> None:
class D(b): ...

def h(b: type) -> None:
class D(b): ...
16 changes: 16 additions & 0 deletions test-data/unit/check-python38.test
Original file line number Diff line number Diff line change
Expand Up @@ -718,3 +718,19 @@ def f1() -> None:
y = x
z = x
[builtins fixtures/dict.pyi]

[case testNarrowOnSelfInGeneric]
# flags: --strict-optional
from typing import Generic, TypeVar, Optional

T = TypeVar("T", int, str)

class C(Generic[T]):
x: Optional[T]
def meth(self) -> Optional[T]:
if (y := self.x) is not None:
reveal_type(y)
return None
[out]
main:10: note: Revealed type is "builtins.int"
main:10: note: Revealed type is "builtins.str"
13 changes: 13 additions & 0 deletions test-data/unit/check-selftype.test
Original file line number Diff line number Diff line change
Expand Up @@ -1772,3 +1772,16 @@ class D(C): ...

reveal_type(D.f) # N: Revealed type is "def [T] (T`-1) -> T`-1"
reveal_type(D().f) # N: Revealed type is "def () -> __main__.D"

[case testTypingSelfOnSuperTypeVarValues]
from typing import Self, Generic, TypeVar

T = TypeVar("T", int, str)

class B:
def copy(self) -> Self: ...
class C(B, Generic[T]):
def copy(self) -> Self:
inst = super().copy()
reveal_type(inst) # N: Revealed type is "Self`0"
return inst
2 changes: 2 additions & 0 deletions test-data/unit/semanal-types.test
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,7 @@ def f(x: int) -> None: pass
def f(*args) -> None: pass

x = f
[builtins fixtures/tuple.pyi]
[out]
MypyFile:1(
ImportFrom:1(typing, [overload])
Expand Down Expand Up @@ -1032,6 +1033,7 @@ MypyFile:1(

[case testVarArgsAndKeywordArgs]
def g(*x: int, y: str = ''): pass
[builtins fixtures/tuple.pyi]
[out]
MypyFile:1(
FuncDef:1(
Expand Down

0 comments on commit 3c5f368

Please sign in to comment.