From a0fa7619a985301627ffedeaf172ef3f986e8116 Mon Sep 17 00:00:00 2001 From: David Euresti Date: Mon, 5 Feb 2018 15:07:48 -0800 Subject: [PATCH 1/5] Simple type checking of class decorators This does the bare minimum, it checks that the calls are well-type. It does not check whether applying the decorator makes sense. Helps with #3135 --- mypy/checker.py | 5 +++++ test-data/unit/check-classes.test | 11 ++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index 8e4af18c17c3..f9f6aa8043ed 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1254,6 +1254,11 @@ def visit_class_def(self, defn: ClassDef) -> None: # Otherwise we've already found errors; more errors are not useful self.check_multiple_inheritance(typ) + for decorator in defn.decorators: + # Currently this only checks that the decorator itself is well typed. + # TODO: Check that applying the decorator to the class would do the right thing. + self.expr_checker.accept(decorator) + def check_protocol_variance(self, defn: ClassDef) -> None: """Check that protocol definition is compatible with declared variances of type variables. diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 0d6cd0102af7..ac9e5e90b1d7 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -4115,7 +4115,7 @@ 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: Metaclasses not inheriting from 'type' are not supported # E: Argument 1 to "add_metaclass" has incompatible type "Type[A]"; expected "Type[type]" 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 +4223,12 @@ 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 +def decorate(x: int) -> Callable[[type], type]: # N: "decorate" defined here + ... + +@decorate(y=17) # E: Unexpected keyword argument "y" for "decorate" +class A: + pass From e40c9c7539d88c115d036f14b5f32e1b2b858103 Mon Sep 17 00:00:00 2001 From: David Euresti Date: Mon, 5 Feb 2018 17:21:30 -0800 Subject: [PATCH 2/5] More tests/Fix tests --- test-data/unit/check-classes.test | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index ac9e5e90b1d7..c2abc9eab1a6 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -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 # E: Argument 1 to "add_metaclass" has incompatible type "Type[A]"; expected "Type[type]" +@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 @@ -4228,7 +4229,19 @@ class C(Any): from typing import Callable def decorate(x: int) -> Callable[[type], type]: # N: "decorate" defined here ... +def decorate_forward_ref() -> Callable[['A'], 'A']: + ... +not_a_function = 17 +class B: pass +b = B() -@decorate(y=17) # E: Unexpected keyword argument "y" for "decorate" +@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" +@not_a_function() # E: "int" not callable +@b.nothing # E: "B" has no attribute "nothing" +@undefined # E: Name 'undefined' is not defined +@decorate(5) +@decorate_forward_ref() class A: pass From 3d19a707583d4e6a81eb7f4a076df362e366bbb4 Mon Sep 17 00:00:00 2001 From: David Euresti Date: Tue, 6 Feb 2018 07:46:51 -0800 Subject: [PATCH 3/5] Cleaner tests --- test-data/unit/check-classes.test | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index c2abc9eab1a6..fb282042aaba 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -4226,22 +4226,28 @@ class C(Any): [out] [case testClassDecoratorIsTypeChecked] -from typing import Callable +from typing import Callable, Type def decorate(x: int) -> Callable[[type], type]: # N: "decorate" defined here ... -def decorate_forward_ref() -> Callable[['A'], 'A']: +def decorate_forward_ref() -> Callable[[Type[A]], Type[A]]: ... -not_a_function = 17 -class B: pass -b = B() - @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" -@not_a_function() # E: "int" not callable -@b.nothing # E: "B" has no attribute "nothing" -@undefined # E: Name 'undefined' is not defined -@decorate(5) @decorate_forward_ref() +@decorate(11) class A: pass + +not_a_function = 17 +@not_a_function() # E: "int" not callable +class B: pass + +b = object() +@b.nothing # E: "object" has no attribute "nothing" +class C: pass + +@undefined # E: Name 'undefined' is not defined +class D: pass + + From c900ed7d4e68e7b63a905c0f2cf7e55fc7aedd6f Mon Sep 17 00:00:00 2001 From: David Euresti Date: Tue, 6 Feb 2018 08:16:56 -0800 Subject: [PATCH 4/5] Fully decorator support --- mypy/checker.py | 23 ++++++++++++++++++----- test-data/unit/check-classes.test | 13 +++++++++---- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index f9f6aa8043ed..30ac5588d08b 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -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,10 +1254,23 @@ def visit_class_def(self, defn: ClassDef) -> None: # Otherwise we've already found errors; more errors are not useful self.check_multiple_inheritance(typ) - for decorator in defn.decorators: - # Currently this only checks that the decorator itself is well typed. - # TODO: Check that applying the decorator to the class would do the right thing. - self.expr_checker.accept(decorator) + 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)): + continue + + dec = self.expr_checker.accept(decorator) + temp = self.temp_node(sig) + fullname = None + if isinstance(decorator, RefExpr): + fullname = decorator.fullname + + sig, t2 = self.expr_checker.check_call(dec, [temp], + [nodes.ARG_POS], defn, + callable_name=fullname) def check_protocol_variance(self, defn: ClassDef) -> None: """Check that protocol definition is compatible with declared diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index fb282042aaba..9a67ddff9549 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -4236,8 +4236,15 @@ def decorate_forward_ref() -> Callable[[Type[A]], Type[A]]: @decorate(22, 25) # E: Too many arguments for "decorate" @decorate_forward_ref() @decorate(11) -class A: - pass +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 +class A3: pass not_a_function = 17 @not_a_function() # E: "int" not callable @@ -4249,5 +4256,3 @@ class C: pass @undefined # E: Name 'undefined' is not defined class D: pass - - From 8318c8efe334cb55147916f3018455a3184ebd48 Mon Sep 17 00:00:00 2001 From: David Euresti Date: Wed, 7 Feb 2018 07:18:35 -0800 Subject: [PATCH 5/5] Add comments, CR --- mypy/checker.py | 11 ++++++++--- test-data/unit/check-classes.test | 3 +++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 30ac5588d08b..7b83e99cc46f 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1260,6 +1260,7 @@ def visit_class_def(self, defn: ClassDef) -> None: for decorator in reversed(defn.decorators): if (isinstance(decorator, CallExpr) and isinstance(decorator.analyzed, PromoteExpr)): + # _promote is a special type checking related construct. continue dec = self.expr_checker.accept(decorator) @@ -1268,9 +1269,13 @@ def visit_class_def(self, defn: ClassDef) -> None: if isinstance(decorator, RefExpr): fullname = decorator.fullname - sig, t2 = self.expr_checker.check_call(dec, [temp], - [nodes.ARG_POS], defn, - callable_name=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 diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 9a67ddff9549..9137345d141e 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -4250,6 +4250,9 @@ not_a_function = 17 @not_a_function() # E: "int" not callable 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