Skip to content

Commit a463090

Browse files
committed
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?
1 parent ac8a5a7 commit a463090

File tree

4 files changed

+34
-3
lines changed

4 files changed

+34
-3
lines changed

mypy/checker.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,7 @@ def _visit_func_def(self, defn: FuncDef) -> None:
10041004
"""Type check a function definition."""
10051005
self.check_func_item(defn, name=defn.name)
10061006
if defn.info:
1007-
if not defn.is_dynamic() and not defn.is_overload and not defn.is_decorated:
1007+
if not defn.is_overload and not defn.is_decorated:
10081008
# If the definition is the implementation for an
10091009
# overload, the legality of the override has already
10101010
# been typechecked, and decorated methods will be

test-data/unit/check-abstract.test

+12-2
Original file line numberDiff line numberDiff line change
@@ -494,8 +494,13 @@ class I(metaclass=ABCMeta):
494494
def g(self, x): pass
495495
class A(I):
496496
def f(self, x): pass
497-
def g(self, x, y): pass
497+
def g(self, x, y): pass # Fail
498498
[out]
499+
main:10: error: Signature of "g" incompatible with supertype "I"
500+
main:10: note: Superclass:
501+
main:10: note: def g(self, x: Any) -> Any
502+
main:10: note: Subclass:
503+
main:10: note: def g(self, x: Any, y: Any) -> Any
499504

500505
[case testAbstractClassWithImplementationUsingDynamicTypes]
501506
from abc import abstractmethod, ABCMeta
@@ -507,8 +512,13 @@ class I(metaclass=ABCMeta):
507512
def g(self, x: int) -> None: pass
508513
class A(I):
509514
def f(self, x): pass
510-
def g(self, x, y): pass
515+
def g(self, x, y): pass # Fail
511516
[out]
517+
main:10: error: Signature of "g" incompatible with supertype "I"
518+
main:10: note: Superclass:
519+
main:10: note: def g(self, x: Any) -> Any
520+
main:10: note: Subclass:
521+
main:10: note: def g(self, x: Any, y: Any) -> Any
512522

513523

514524
-- Special cases

test-data/unit/check-dynamic-typing.test

+5
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,11 @@ class A(B):
727727
def f(self, x, y): # dynamic function not type checked
728728
x()
729729
[out]
730+
main:5: error: Signature of "f" incompatible with supertype "B"
731+
main:5: note: Superclass:
732+
main:5: note: def f(self, x: A) -> None
733+
main:5: note: Subclass:
734+
main:5: note: def f(self, x: Any, y: Any) -> Any
730735

731736
[case testInvalidOverrideArgumentCountWithImplicitSignature2]
732737
import typing

test-data/unit/check-functions.test

+16
Original file line numberDiff line numberDiff line change
@@ -3228,3 +3228,19 @@ class A:
32283228
reveal_type(A.f) # N: Revealed type is "__main__.something_callable"
32293229
reveal_type(A().f) # N: Revealed type is "builtins.str"
32303230
[builtins fixtures/property.pyi]
3231+
3232+
[case testFinalOverrideOnUntypedDef]
3233+
# flags: --python-version 3.12
3234+
3235+
from typing import final
3236+
3237+
class Base:
3238+
@final
3239+
def foo(self):
3240+
pass
3241+
3242+
class Derived(Base):
3243+
def foo(self): # E: Cannot override final attribute "foo" (previously declared in base class "Base")
3244+
pass
3245+
3246+

0 commit comments

Comments
 (0)