From a463090c398e0f8fc4772df743a5ba266fd90522 Mon Sep 17 00:00:00 2001 From: Steven Troxler Date: Tue, 21 May 2024 13:30:52 -0400 Subject: [PATCH 1/3] Check override compatibility for dynamically typed methods This fixes #9618 in a fairly aggressive way, by turning on all override checks for all methods. This means that with this change, MyPy will complain in cases where just inspecting type signatures involving `Any` is enough to establish that an override is a type error: - This includes the case of overriding `@final` as listed in #9618 - But it also includes arity mismatches, for example when an untyped subclass method adds a required attribute not in the parent class method. The tests illustrate this, I added a test case to `check-functions.test` (is this a good place for it?) for the `@final` check specifically but also updated a few existing tests of dynamic logic. The resulting code change is very simple (since all override checks happen in the same place), and I think conceptually it is right to enable all these checks because the contract is that MyPy won't generally type check *bodies* of functions, but the function signatures are actually part of the *class* type definition which is (in other cases) sanity-checked even when there are no annotations. If we think that the arity checks are too big a change, I can work on a refactor that would let us validate *only* `@final` but not other properties. Maybe waiting for `mypy_primer` output is the way to go? --- mypy/checker.py | 2 +- test-data/unit/check-abstract.test | 14 ++++++++++++-- test-data/unit/check-dynamic-typing.test | 5 +++++ test-data/unit/check-functions.test | 16 ++++++++++++++++ 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 3daf64daaac4a..460ec5fd77fc2 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1004,7 +1004,7 @@ def _visit_func_def(self, defn: FuncDef) -> None: """Type check a function definition.""" self.check_func_item(defn, name=defn.name) if defn.info: - if not defn.is_dynamic() and not defn.is_overload and not defn.is_decorated: + if not defn.is_overload and not defn.is_decorated: # If the definition is the implementation for an # overload, the legality of the override has already # been typechecked, and decorated methods will be diff --git a/test-data/unit/check-abstract.test b/test-data/unit/check-abstract.test index 3b0b9c520b75f..65f74ffe0b4f8 100644 --- a/test-data/unit/check-abstract.test +++ b/test-data/unit/check-abstract.test @@ -494,8 +494,13 @@ class I(metaclass=ABCMeta): def g(self, x): pass class A(I): def f(self, x): pass - def g(self, x, y): pass + def g(self, x, y): pass # Fail [out] +main:10: error: Signature of "g" incompatible with supertype "I" +main:10: note: Superclass: +main:10: note: def g(self, x: Any) -> Any +main:10: note: Subclass: +main:10: note: def g(self, x: Any, y: Any) -> Any [case testAbstractClassWithImplementationUsingDynamicTypes] from abc import abstractmethod, ABCMeta @@ -507,8 +512,13 @@ class I(metaclass=ABCMeta): def g(self, x: int) -> None: pass class A(I): def f(self, x): pass - def g(self, x, y): pass + def g(self, x, y): pass # Fail [out] +main:10: error: Signature of "g" incompatible with supertype "I" +main:10: note: Superclass: +main:10: note: def g(self, x: Any) -> Any +main:10: note: Subclass: +main:10: note: def g(self, x: Any, y: Any) -> Any -- Special cases diff --git a/test-data/unit/check-dynamic-typing.test b/test-data/unit/check-dynamic-typing.test index 0dc05a7a0ea13..a0e3a46068a9d 100644 --- a/test-data/unit/check-dynamic-typing.test +++ b/test-data/unit/check-dynamic-typing.test @@ -727,6 +727,11 @@ class A(B): def f(self, x, y): # dynamic function not type checked x() [out] +main:5: error: Signature of "f" incompatible with supertype "B" +main:5: note: Superclass: +main:5: note: def f(self, x: A) -> None +main:5: note: Subclass: +main:5: note: def f(self, x: Any, y: Any) -> Any [case testInvalidOverrideArgumentCountWithImplicitSignature2] import typing diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index 3aecbe065c275..4cea9e51f8aab 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -3228,3 +3228,19 @@ class A: reveal_type(A.f) # N: Revealed type is "__main__.something_callable" reveal_type(A().f) # N: Revealed type is "builtins.str" [builtins fixtures/property.pyi] + +[case testFinalOverrideOnUntypedDef] +# flags: --python-version 3.12 + +from typing import final + +class Base: + @final + def foo(self): + pass + +class Derived(Base): + def foo(self): # E: Cannot override final attribute "foo" (previously declared in base class "Base") + pass + + From de53ba3c2d7056ddbe5cc2d3e8b62c3f3d052b2e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 21 May 2024 18:03:47 +0000 Subject: [PATCH 2/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- test-data/unit/check-functions.test | 2 -- 1 file changed, 2 deletions(-) diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index 4cea9e51f8aab..16b2993dc3f3f 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -3242,5 +3242,3 @@ class Base: class Derived(Base): def foo(self): # E: Cannot override final attribute "foo" (previously declared in base class "Base") pass - - From 9068fdf2fbbd9d866ebc63e01dcc80f97c8453ca Mon Sep 17 00:00:00 2001 From: Steven Troxler Date: Tue, 21 May 2024 14:42:29 -0400 Subject: [PATCH 3/3] Fix bad test case --- test-data/unit/check-abstract.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-data/unit/check-abstract.test b/test-data/unit/check-abstract.test index 65f74ffe0b4f8..2549daaa1af61 100644 --- a/test-data/unit/check-abstract.test +++ b/test-data/unit/check-abstract.test @@ -516,7 +516,7 @@ class A(I): [out] main:10: error: Signature of "g" incompatible with supertype "I" main:10: note: Superclass: -main:10: note: def g(self, x: Any) -> Any +main:10: note: def g(self, x: int) -> None main:10: note: Subclass: main:10: note: def g(self, x: Any, y: Any) -> Any