merged in master

spinn_utilities/overrides.py

@@ -123,16 +123,24 @@ def _verify_types(self, method_args, super_args, all_args):
                 raise AttributeError(
                     f"Super Method {self._superclass_method.__name__} "
                     f"has no arguments so should declare a return type")
-        if "return" in super_types:
-            if "return" not in method_types:
+            if "return" not in method_types and \
+                    not method_args.varkw and not method_args.varargs:
                 raise AttributeError(
                     f"Method {self._superclass_method.__name__} "
-                    f"has no return type, while super does")
+                    f"has no arguments so should declare a return type")
+
+        if "return" in super_types:
+            if "return" not in method_types:
+                if super_types["return"] is not None:
+                    raise AttributeError(
+                        f"Method {self._superclass_method.__name__} "
+                        f"has no return type, while super does")
         else:
             if "return" in method_types and not self._adds_typing:
-                raise AttributeError(
-                    f"Super Method {self._superclass_method.__name__} "
-                    f"has no return type, while this does")
+                if method_types["return"] is not None:
+                    raise AttributeError(
+                        f"Super Method {self._superclass_method.__name__} "
+                        f"has no return type, while this does")
 
     def __verify_method_arguments(self, method: Method):
         """

unittests/test_overrides.py

@@ -13,56 +13,72 @@
 # limitations under the License.
 
 import pytest
-from typing import Any
+from typing import Any, List
 from spinn_utilities.abstract_base import abstractmethod
 from spinn_utilities.overrides import overrides
 
 WRONG_ARGS = "Method has {} arguments but super class method has 4 arguments"
 BAD_DEFS = "Default arguments don't match super class method"
 
+overrides.check_types()
+
 
 class Base(object):
-    def foo(self, x: Any, y: Any, z: Any):
+    def foo(self, x: int, y: int, z: int) -> List[int]:
         """this is the doc"""
         return [x, y, z]
 
-    def foodef(self, x: Any, y: Any, z: Any = True):
+    def foodef(self, x: Any, y: int, z: Any = True) -> List[Any]:
         """this is the doc"""
         return [x, y, z]
 
     @property
     def boo(self) -> int:
         return 123
 
+    def with_param_no_return(self, x: int) -> None:
+        pass
+
+    def with_param_no_return2(self, x: int):
+        pass
+
+    def no_param_no_return(self):
+        pass
+
+    # This is bad as it does not define a return
+    def bad(self, x: int, y: int, z: int):
+        """this is the doc"""
+        return [x, y, z]
+
 
 def test_basic_use():
     class Sub(Base):
         @overrides(Base.foo)
-        def foo(self, x: Any, y: Any, z: Any):
+        def foo(self, x: int, y: int, z: int) -> List[int]:
             return super().foo(z, y, x)
     assert Sub().foo(1, 2, 3) == [3, 2, 1]
 
 
 def test_doc_no_sub_extend():
     class Sub(Base):
         @overrides(Base.foo, extend_doc=True)
-        def foo(self, x: Any, y: Any, z: Any):
+        def foo(self, x: int, y: int, z: int) -> List[int]:
             return [z, y, x]
     assert Sub.foo.__doc__ == "this is the doc"
 
 
 def test_doc_no_sub_no_extend():
     class Sub(Base):
         @overrides(Base.foo, extend_doc=False)
-        def foo(self, x: Any, y: Any, z: Any):
+        def foo(self, x: int, y: int, z: int) -> List[int]:
             return [z, y, x]
     assert Sub.foo.__doc__ == "this is the doc"
 
 
 def test_doc_sub_no_extend():
     class Sub(Base):
         @overrides(Base.foo, extend_doc=False)
-        def foo(self, x: Any, y: Any, z: Any):
+        def foo(self, x: int, y: int, z: int) -> List[int]:
             """(abc)"""
             return [z, y, x]
     assert Sub.foo.__doc__ == "(abc)"
@@ -71,7 +87,7 @@ def foo(self, x: Any, y: Any, z: Any):
 def test_doc_sub_extend():
     class Sub(Base):
         @overrides(Base.foo, extend_doc=True)
-        def foo(self, x: Any, y: Any, z: Any):
+        def foo(self, x: int, y: int, z: int) -> List[int]:
             """(abc)"""
             return [z, y, x]
     assert Sub.foo.__doc__ == "this is the doc(abc)"
@@ -81,7 +97,7 @@ def test_removes_param():
     with pytest.raises(AttributeError) as e:
         class Sub(Base):
             @overrides(Base.foo)
-            def foo(self, x: Any, y: Any):
+            def foo(self, x: int, y: int) -> List[int]:
                 return [y, x]
     assert str(e.value) == WRONG_ARGS.format(3)
 
@@ -90,15 +106,15 @@ def test_adds_param():
     with pytest.raises(AttributeError) as e:
         class Sub(Base):
             @overrides(Base.foo)
-            def foo(self, x: Any, y: Any, z: Any, w: Any):
+            def foo(self, x: int, y: int, z: int, w: int) -> List[int]:
                 return [w, z, y, x]
     assert str(e.value) == WRONG_ARGS.format(5)
 
 
 def test_adds_expected_param():
     class Sub(Base):
         @overrides(Base.foo, additional_arguments=["w"])
-        def foo(self, x: Any, y: Any, z: Any, w: Any):
+        def foo(self, x: int, y: int, z: int, w: int) -> List[int]:
             return [w, z, y, x]
     assert Sub().foo(1, 2, 3, 4) == [4, 3, 2, 1]
 
@@ -107,7 +123,7 @@ def test_renames_param():
     with pytest.raises(AttributeError) as e:
         class Sub(Base):
             @overrides(Base.foo)
-            def foo(self, x: Any, y: Any, w: Any):
+            def foo(self, x: int, y: int, w: int) -> List[int]:
                 return [w, y, x]
     assert str(e.value) == "Missing argument z"
 
@@ -116,7 +132,7 @@ def test_renames_param_expected():
     with pytest.raises(AttributeError) as e:
         class Sub(Base):
             @overrides(Base.foo, additional_arguments=["w"])
-            def foo(self, x: Any, y: Any, w: Any):
+            def foo(self, x: int, y: int, w: int) -> List[int]:
                 return [w, y, x]
     assert str(e.value) == WRONG_ARGS.format(4)
     # TODO: Fix the AWFUL error message in this case!
@@ -125,7 +141,7 @@ def foo(self, x: Any, y: Any, w: Any):
 def test_changes_params_defaults():
     class Sub(Base):
         @overrides(Base.foodef)
-        def foodef(self, x: Any, y: Any, z: Any = False):
+        def foodef(self, x: Any, y: Any, z: Any = False) -> List[Any]:
             return [z, y, x]
     assert Sub().foodef(1, 2) == [False, 2, 1]
 
@@ -134,16 +150,16 @@ def test_undefaults_super_param():
     with pytest.raises(AttributeError) as e:
         class Sub(Base):
             @overrides(Base.foodef)
-            def foodef(self, x: Any, y: Any, z: Any):
-                return [z, y, x]
+            def foodef(self, x: Any, y: Any, z: Any) -> List[Any]:
+                return (z, y, x)
     assert str(e.value) == BAD_DEFS
 
 
 def test_defaults_super_param():
     with pytest.raises(AttributeError) as e:
         class Sub(Base):
             @overrides(Base.foodef)
-            def foodef(self, x: Any, y: Any = 1, z: Any = 2):
+            def foodef(self, x: Any, y: Any = 1, z: Any = 2) -> List[Any]:
                 return [z, y, x]
     assert str(e.value) == BAD_DEFS
     # TODO: Should this case fail at all?
@@ -152,7 +168,7 @@ def foodef(self, x: Any, y: Any = 1, z: Any = 2):
 def test_defaults_super_param_expected():
     class Sub(Base):
         @overrides(Base.foodef, extend_defaults=True)
-        def foodef(self, x: Any, y: Any = 1, z: Any = 2):
+        def foodef(self, x: Any, y: Any = 1, z: Any = 2) -> List[Any]:
             return [z, y, x]
     assert Sub().foodef(7) == [2, 1, 7]
 
@@ -161,7 +177,8 @@ def test_defaults_extra_param():
     with pytest.raises(AttributeError) as e:
         class Sub(Base):
             @overrides(Base.foodef, additional_arguments=['pdq'])
-            def foodef(self, x: Any, y: Any, z: Any = 1, pdq: Any = 2):
+            def foodef(self, x: Any, y: Any, z: Any = 1, pdq: Any = 2
+                       ) -> List[Any]:
                 return [z, y, x, pdq]
     assert str(e.value) == BAD_DEFS
 
@@ -170,7 +187,7 @@ def test_defaults_super_param_no_super_defaults():
     with pytest.raises(AttributeError) as e:
         class Sub(Base):
             @overrides(Base.foo)
-            def foo(self, x: Any, y: Any, z: Any = 7):
+            def foo(self, x: int, y: int, z: int = 7) -> List[int]:
                 return [z, y, x]
     assert str(e.value) == BAD_DEFS
     # TODO: Should this case fail at all?
@@ -180,7 +197,7 @@ def test_crazy_extends():
     with pytest.raises(AttributeError) as e:
         class Sub(Base):
             @overrides(Base.foo)
-            def bar(self, x: Any, y: Any, z: Any):
+            def bar(self, x: int, y: int, z: int):
                 return [z, y, x]
     assert str(e.value) == \
         "super class method name foo does not match bar. Ensure overrides is "\
@@ -298,3 +315,65 @@ def foo(self) -> int:
                 return 1
     except AttributeError:
         pass
+
+
+def test_add_return():
+    with pytest.raises(AttributeError) as e:
+        class Sub(Base):
+            @overrides(Base.bad)
+            def bad(self, x: int, y: int, z: int) -> List[int]:
+                return super().foo(z, y, x)
+    assert str(e.value) == "Super Method bad has no return type, " \
+                           "while this does"
+
+
+def test_dont_add_return():
+    # This demonstrates that if both have no return we can not check it.
+    # It would be better if there was an error!
+    class Sub(Base):
+        @overrides(Base.bad)
+        def bad(self, x: int, y: int, z: int):
+            return super().foo(z, y, x)
+    assert Sub().bad(1, 2, 3) == [3, 2, 1]
+
+
+def test_missing_return_type():
+    with pytest.raises(AttributeError) as e:
+        class Sub(Base):
+            @property
+            @overrides(Base.boo)
+            def boo(self):
+                return 2
+    assert str(e.value) == "Method boo has no arguments " \
+                           "so should declare a return type"
+
+
+def test_with_missing_return():
+    class Sub(Base):
+        @overrides(Base.with_param_no_return)
+        def with_param_no_return(self, x: int):
+            pass
+
+
+def test_with_missing_return_both():
+    class Sub(Base):
+        @overrides(Base.with_param_no_return2)
+        def with_param_no_return2(self, x: int):
+            pass
+
+
+def test_with_missing_return_super():
+    class Sub(Base):
+        @overrides(Base.with_param_no_return2)
+        def with_param_no_return2(self, x: int) -> None:
+            pass
+
+
+def test_no_param_missing_return():
+    with pytest.raises(AttributeError) as e:
+        class Sub(Base):
+            @overrides(Base.no_param_no_return)
+            def no_param_no_return(self):
+                pass
+    assert str(e.value) == "Super Method no_param_no_return has " \
+                           "no arguments so should declare a return type"