-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Type checking of class decorators #4544
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
Changes from all commits
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 |
---|---|---|
|
@@ -40,7 +40,7 @@ | |
from mypy.sametypes import is_same_type, is_same_types | ||
from mypy.messages import MessageBuilder, make_inferred_type_note | ||
import mypy.checkexpr | ||
from mypy.checkmember import map_type_from_supertype, bind_self, erase_to_bound | ||
from mypy.checkmember import map_type_from_supertype, bind_self, erase_to_bound, type_object_type | ||
from mypy import messages | ||
from mypy.subtypes import ( | ||
is_subtype, is_equivalent, is_proper_subtype, is_more_precise, | ||
|
@@ -1254,6 +1254,29 @@ def visit_class_def(self, defn: ClassDef) -> None: | |
# Otherwise we've already found errors; more errors are not useful | ||
self.check_multiple_inheritance(typ) | ||
|
||
if defn.decorators: | ||
sig = type_object_type(defn.info, self.named_type) | ||
# Decorators are applied in reverse order. | ||
for decorator in reversed(defn.decorators): | ||
if (isinstance(decorator, CallExpr) | ||
and isinstance(decorator.analyzed, PromoteExpr)): | ||
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. This exception deserves a comment. 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. Done! |
||
# _promote is a special type checking related construct. | ||
continue | ||
|
||
dec = self.expr_checker.accept(decorator) | ||
temp = self.temp_node(sig) | ||
fullname = None | ||
if isinstance(decorator, RefExpr): | ||
fullname = decorator.fullname | ||
|
||
# TODO: Figure out how to have clearer error messages. | ||
# (e.g. "class decorator must be a function that accepts a type." | ||
sig, _ = self.expr_checker.check_call(dec, [temp], | ||
[nodes.ARG_POS], defn, | ||
callable_name=fullname) | ||
# TODO: Apply the sig to the actual TypeInfo so we can handle decorators | ||
# that completely swap out the type. (e.g. Callable[[Type[A]], Type[B]]) | ||
|
||
def check_protocol_variance(self, defn: ClassDef) -> None: | ||
"""Check that protocol definition is compatible with declared | ||
variances of type variables. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4115,7 +4115,8 @@ def f() -> type: return M | |
class C1(six.with_metaclass(M), object): pass # E: Invalid base class | ||
class C2(C1, six.with_metaclass(M)): pass # E: Invalid base class | ||
class C3(six.with_metaclass(A)): pass # E: Metaclasses not inheriting from 'type' are not supported | ||
@six.add_metaclass(A) # E: Metaclasses not inheriting from 'type' are not supported | ||
@six.add_metaclass(A) # E: Argument 1 to "add_metaclass" has incompatible type "Type[A]"; expected "Type[type]" \ | ||
# E: Metaclasses not inheriting from 'type' are not supported | ||
class D3(A): pass | ||
class C4(six.with_metaclass(M), metaclass=M): pass # E: Multiple metaclass definitions | ||
@six.add_metaclass(M) # E: Multiple metaclass definitions | ||
|
@@ -4223,3 +4224,38 @@ class C(Any): | |
reveal_type(self.bar().__name__) # E: Revealed type is 'builtins.str' | ||
[builtins fixtures/type.pyi] | ||
[out] | ||
|
||
[case testClassDecoratorIsTypeChecked] | ||
from typing import Callable, Type | ||
def decorate(x: int) -> Callable[[type], type]: # N: "decorate" defined here | ||
... | ||
def decorate_forward_ref() -> Callable[[Type[A]], Type[A]]: | ||
... | ||
@decorate(y=17) # E: Unexpected keyword argument "y" for "decorate" | ||
@decorate() # E: Too few arguments for "decorate" | ||
@decorate(22, 25) # E: Too many arguments for "decorate" | ||
@decorate_forward_ref() | ||
@decorate(11) | ||
class A: pass | ||
|
||
@decorate # E: Argument 1 to "decorate" has incompatible type "Type[A2]"; expected "int" | ||
class A2: pass | ||
|
||
[case testClassDecoratorIncorrect] | ||
def not_a_class_decorator(x: int) -> int: ... | ||
@not_a_class_decorator(7) # E: "int" not callable | ||
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. The error message is quite obscure. I would somehow highlight that a class decorator must be a function that accepts a type. If this is too hard, then leave a TODO about this in code. 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. Left a TODO. Only thing I can figure out is collect all the errors with |
||
class A3: pass | ||
|
||
not_a_function = 17 | ||
@not_a_function() # E: "int" not callable | ||
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. Also add a test for directly applying @not_a_function
class C: ... 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. Done |
||
class B: pass | ||
|
||
@not_a_function # E: "int" not callable | ||
class B2: pass | ||
|
||
b = object() | ||
@b.nothing # E: "object" has no attribute "nothing" | ||
class C: pass | ||
|
||
@undefined # E: Name 'undefined' is not defined | ||
class D: pass |
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.
I would leave some kind of
TODO
here. One of main use cases:is still not supported.
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.
Added below