From 7fef24fcfa689cac80d58b2ece866a3a366395e9 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Sat, 24 Jun 2023 22:58:59 +0200 Subject: [PATCH 1/6] Add strict_override_decorator option (PEP 698) --- docs/source/class_basics.rst | 6 ++ docs/source/config_file.rst | 7 ++ mypy/checker.py | 20 +++++- mypy/main.py | 8 +++ mypy/messages.py | 7 ++ mypy/options.py | 4 ++ test-data/unit/check-functions.test | 72 +++++++++++++++++++++ test-data/unit/fixtures/typing-override.pyi | 24 +++++++ 8 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 test-data/unit/fixtures/typing-override.pyi diff --git a/docs/source/class_basics.rst b/docs/source/class_basics.rst index 82bbf00b830d..aec9e386d55e 100644 --- a/docs/source/class_basics.rst +++ b/docs/source/class_basics.rst @@ -233,6 +233,12 @@ show an error: def g(self, y: str) -> None: # Error: no corresponding base method found ... +.. note:: + + Use ``--strict-override-decorator`` or + :confval:`strict_override_decorator = True ` to require + methods overrides use the ``@override`` decorator. Emit an error if it is missing. + You can also override a statically typed method with a dynamically typed one. This allows dynamically typed code to override methods defined in library classes without worrying about their type diff --git a/docs/source/config_file.rst b/docs/source/config_file.rst index 9e79ff99937b..37861ec4d750 100644 --- a/docs/source/config_file.rst +++ b/docs/source/config_file.rst @@ -714,6 +714,13 @@ section of the command line docs. Prohibit equality checks, identity checks, and container checks between non-overlapping types. +.. confval:: strict_override_decorator + + :type: boolean + :default: False + + Require ``override`` decorator if method is overriding a base class method. + .. confval:: strict :type: boolean diff --git a/mypy/checker.py b/mypy/checker.py index bf200299d8b3..7e025b07cb99 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -646,6 +646,12 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None: found_base_method = self.check_method_override(defn) if defn.is_explicit_override and found_base_method is False: self.msg.no_overridable_method(defn.name, defn) + elif ( + found_base_method + and self.options.strict_override_decorator + and not defn.is_explicit_override + ): + self.msg.override_decorator_missing(defn.name, defn.impl or defn) self.check_inplace_operator_method(defn) if not defn.is_property: self.check_overlapping_overloads(defn) @@ -972,7 +978,13 @@ def _visit_func_def(self, defn: FuncDef) -> None: # overload, the legality of the override has already # been typechecked, and decorated methods will be # checked when the decorator is. - self.check_method_override(defn) + found_base_method = self.check_method_override(defn) + if ( + found_base_method + and self.options.strict_override_decorator + and defn.name not in ("__init__", "__new__") + ): + self.msg.override_decorator_missing(defn.name, defn) self.check_inplace_operator_method(defn) if defn.original_def: # Override previous definition. @@ -4743,6 +4755,12 @@ def visit_decorator(self, e: Decorator) -> None: found_base_method = self.check_method_override(e) if e.func.is_explicit_override and found_base_method is False: self.msg.no_overridable_method(e.func.name, e.func) + elif ( + found_base_method + and self.options.strict_override_decorator + and not e.func.is_explicit_override + ): + self.msg.override_decorator_missing(e.func.name, e.func) if e.func.info and e.func.name in ("__init__", "__new__"): if e.type and not isinstance(get_proper_type(e.type), (FunctionLike, AnyType)): diff --git a/mypy/main.py b/mypy/main.py index f6e617e4d84f..e6dc8ffe257f 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -824,6 +824,14 @@ def add_invertible_flag( group=strictness_group, ) + add_invertible_flag( + "--strict-override-decorator", + default=False, + strict_flag=False, + help="Require override decorator if method is overriding a base class method.", + group=strictness_group, + ) + add_invertible_flag( "--extra-checks", default=False, diff --git a/mypy/messages.py b/mypy/messages.py index 021ad2c7390c..34df4a6e62cb 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1525,6 +1525,13 @@ def no_overridable_method(self, name: str, context: Context) -> None: context, ) + def override_decorator_missing(self, name: str, context: Context) -> None: + self.fail( + f'Method "{name}" is not marked as override ' + "but is overriding a method in a base class", + context, + ) + def final_cant_override_writable(self, name: str, ctx: Context) -> None: self.fail(f'Cannot override writable attribute "{name}" with a final one', ctx) diff --git a/mypy/options.py b/mypy/options.py index 75343acd38bb..0f3400bf9152 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -51,6 +51,7 @@ class BuildType: "strict_concatenate", "strict_equality", "strict_optional", + "strict_override_decorator", "warn_no_return", "warn_return_any", "warn_unreachable", @@ -200,6 +201,9 @@ def __init__(self) -> None: # This makes 1 == '1', 1 in ['1'], and 1 is '1' errors. self.strict_equality = False + # Require override decorator. Strict mode for PEP 698. + self.strict_override_decorator = False + # Deprecated, use extra_checks instead. self.strict_concatenate = False diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index b5d540b105e3..fafabb04e24a 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -3007,3 +3007,75 @@ class C(A): def f(self, y: int | str) -> str: pass [typing fixtures/typing-full.pyi] [builtins fixtures/tuple.pyi] + +[case requireExplicitOverrideMethod] +# flags: --strict-override-decorator --python-version 3.12 +from typing import override + +class A: + def f(self, x: int) -> str: pass + +class B(A): + @override + def f(self, y: int) -> str: pass + +class C(A): + def f(self, y: int) -> str: pass # E: Method "f" is not marked as override but is overriding a method in a base class +[typing fixtures/typing-override.pyi] + +[case requireExplicitOverrideSpecialMethod] +# flags: --strict-override-decorator --python-version 3.12 +from typing import Self, override + +# Don't require override decorator for __init__ and __new__ +# See: https://github.com/python/typing/issues/1376 +class A: + def __init__(self) -> None: pass + def __new__(cls) -> Self: pass +[typing fixtures/typing-override.pyi] + +[case requireExplicitOverrideProperty] +# flags: --strict-override-decorator --python-version 3.12 +from typing import override + +class A: + @property + def prop(self) -> int: pass + +class B(A): + @override + @property + def prop(self) -> int: pass + +class C(A): + @property + def prop(self) -> int: pass # E: Method "prop" is not marked as override but is overriding a method in a base class +[typing fixtures/typing-override.pyi] +[builtins fixtures/property.pyi] + +[case requireExplicitOverrideOverload] +# flags: --strict-override-decorator --python-version 3.12 +from typing import overload, override + +class A: + @overload + def f(self, x: int) -> str: ... + @overload + def f(self, x: str) -> str: ... + def f(self, x): pass + +class B(A): + @overload + def f(self, y: int) -> str: ... + @overload + def f(self, y: str) -> str: ... + @override + def f(self, y): pass + +class C(A): + @overload + def f(self, y: int) -> str: ... + @overload + def f(self, y: str) -> str: ... + def f(self, y): pass # E: Method "f" is not marked as override but is overriding a method in a base class +[typing fixtures/typing-override.pyi] diff --git a/test-data/unit/fixtures/typing-override.pyi b/test-data/unit/fixtures/typing-override.pyi new file mode 100644 index 000000000000..b56be5cd819c --- /dev/null +++ b/test-data/unit/fixtures/typing-override.pyi @@ -0,0 +1,24 @@ +TypeVar = 0 +Generic = 0 +Any = 0 +overload = 0 +Type = 0 +Literal = 0 +Optional = 0 +Self = 0 +Tuple = 0 +ClassVar = 0 + +T = TypeVar('T') +T_co = TypeVar('T_co', covariant=True) +KT = TypeVar('KT') + +class Iterable(Generic[T_co]): pass +class Iterator(Iterable[T_co]): pass +class Sequence(Iterable[T_co]): pass +class Mapping(Iterable[KT], Generic[KT, T_co]): + def keys(self) -> Iterable[T]: pass # Approximate return type + def __getitem__(self, key: T) -> T_co: pass + + +def override(__arg: T) -> T: ... From 537e794691a6c4775e8997d0e23bd8154af47400 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Wed, 28 Jun 2023 12:33:44 +0200 Subject: [PATCH 2/6] Update fixtures on existing override tests --- test-data/unit/check-functions.test | 31 +++++++++++------------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index fafabb04e24a..ebd914f27fd7 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -2752,8 +2752,7 @@ class E(D): pass class F(E): @override def f(self, x: int) -> str: pass -[typing fixtures/typing-full.pyi] -[builtins fixtures/tuple.pyi] +[typing fixtures/typing-override.pyi] [case explicitOverrideStaticmethod] # flags: --python-version 3.12 @@ -2785,8 +2784,8 @@ class D(A): def f(x: str) -> str: pass # E: Argument 1 of "f" is incompatible with supertype "A"; supertype defines the argument type as "int" \ # N: This violates the Liskov substitution principle \ # N: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides -[typing fixtures/typing-full.pyi] -[builtins fixtures/callable.pyi] +[typing fixtures/typing-override.pyi] +[builtins fixtures/staticmethod.pyi] [case explicitOverrideClassmethod] # flags: --python-version 3.12 @@ -2818,8 +2817,8 @@ class D(A): def f(cls, x: str) -> str: pass # E: Argument 1 of "f" is incompatible with supertype "A"; supertype defines the argument type as "int" \ # N: This violates the Liskov substitution principle \ # N: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides -[typing fixtures/typing-full.pyi] -[builtins fixtures/callable.pyi] +[typing fixtures/typing-override.pyi] +[builtins fixtures/classmethod.pyi] [case explicitOverrideProperty] # flags: --python-version 3.12 @@ -2853,8 +2852,8 @@ class D(A): # N: str \ # N: Subclass: \ # N: int +[typing fixtures/typing-override.pyi] [builtins fixtures/property.pyi] -[typing fixtures/typing-full.pyi] [case explicitOverrideSettableProperty] # flags: --python-version 3.12 @@ -2891,8 +2890,8 @@ class D(A): @f.setter def f(self, value: int) -> None: pass +[typing fixtures/typing-override.pyi] [builtins fixtures/property.pyi] -[typing fixtures/typing-full.pyi] [case invalidExplicitOverride] # flags: --python-version 3.12 @@ -2907,8 +2906,7 @@ class A: pass def g() -> None: @override # E: "override" used with a non-method def h(b: bool) -> int: pass -[typing fixtures/typing-full.pyi] -[builtins fixtures/tuple.pyi] +[typing fixtures/typing-override.pyi] [case explicitOverrideSpecialMethods] # flags: --python-version 3.12 @@ -2924,8 +2922,7 @@ class B(A): class C: @override def __init__(self, a: int) -> None: pass -[typing fixtures/typing-full.pyi] -[builtins fixtures/tuple.pyi] +[typing fixtures/typing-override.pyi] [case explicitOverrideFromExtensions] from typing_extensions import override @@ -2936,7 +2933,6 @@ class A: class B(A): @override def f2(self, x: int) -> str: pass # E: Method "f2" is marked as an override, but no base method was found with this name -[typing fixtures/typing-full.pyi] [builtins fixtures/tuple.pyi] [case explicitOverrideOverloads] @@ -2953,8 +2949,7 @@ class B(A): def f2(self, x: str) -> str: pass @override def f2(self, x: int | str) -> str: pass -[typing fixtures/typing-full.pyi] -[builtins fixtures/tuple.pyi] +[typing fixtures/typing-override.pyi] [case explicitOverrideNotOnOverloadsImplementation] # flags: --python-version 3.12 @@ -2978,8 +2973,7 @@ class C(A): @overload def f(self, y: str) -> str: pass def f(self, y: int | str) -> str: pass -[typing fixtures/typing-full.pyi] -[builtins fixtures/tuple.pyi] +[typing fixtures/typing-override.pyi] [case explicitOverrideOnMultipleOverloads] # flags: --python-version 3.12 @@ -3005,8 +2999,7 @@ class C(A): def f(self, y: str) -> str: pass @override def f(self, y: int | str) -> str: pass -[typing fixtures/typing-full.pyi] -[builtins fixtures/tuple.pyi] +[typing fixtures/typing-override.pyi] [case requireExplicitOverrideMethod] # flags: --strict-override-decorator --python-version 3.12 From 6e40627918fa9239aad8e2aab3b47162f8568b88 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Wed, 5 Jul 2023 10:48:05 +0200 Subject: [PATCH 3/6] Include method base class in error message --- mypy/checker.py | 39 ++++++++++++++++++----------- mypy/messages.py | 4 +-- test-data/unit/check-functions.test | 26 ++++++++++++++++--- 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 7e025b07cb99..b578cc5fae26 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -643,15 +643,17 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None: if defn.impl: defn.impl.accept(self) if defn.info: - found_base_method = self.check_method_override(defn) - if defn.is_explicit_override and found_base_method is False: + method_base_class = self.check_method_override(defn) + if defn.is_explicit_override and not method_base_class: self.msg.no_overridable_method(defn.name, defn) elif ( - found_base_method + method_base_class and self.options.strict_override_decorator and not defn.is_explicit_override ): - self.msg.override_decorator_missing(defn.name, defn.impl or defn) + self.msg.override_decorator_missing( + defn.name, method_base_class.fullname, defn.impl or defn + ) self.check_inplace_operator_method(defn) if not defn.is_property: self.check_overlapping_overloads(defn) @@ -978,13 +980,15 @@ def _visit_func_def(self, defn: FuncDef) -> None: # overload, the legality of the override has already # been typechecked, and decorated methods will be # checked when the decorator is. - found_base_method = self.check_method_override(defn) + method_base_class = self.check_method_override(defn) if ( - found_base_method + method_base_class and self.options.strict_override_decorator and defn.name not in ("__init__", "__new__") ): - self.msg.override_decorator_missing(defn.name, defn) + self.msg.override_decorator_missing( + defn.name, method_base_class.fullname, defn + ) self.check_inplace_operator_method(defn) if defn.original_def: # Override previous definition. @@ -1826,23 +1830,26 @@ def expand_typevars( else: return [(defn, typ)] - def check_method_override(self, defn: FuncDef | OverloadedFuncDef | Decorator) -> bool | None: + def check_method_override( + self, defn: FuncDef | OverloadedFuncDef | Decorator + ) -> TypeInfo | None: """Check if function definition is compatible with base classes. This may defer the method if a signature is not available in at least one base class. Return ``None`` if that happens. + Return the first base class if an attribute with the method name was found within it. Return ``True`` if an attribute with the method name was found in the base class. """ # Check against definitions in base classes. - found_base_method = False for base in defn.info.mro[1:]: result = self.check_method_or_accessor_override_for_base(defn, base) if result is None: # Node was deferred, we will have another attempt later. return None - found_base_method |= result - return found_base_method + if result: + return base + return None def check_method_or_accessor_override_for_base( self, defn: FuncDef | OverloadedFuncDef | Decorator, base: TypeInfo @@ -4752,15 +4759,17 @@ def visit_decorator(self, e: Decorator) -> None: self.check_incompatible_property_override(e) # For overloaded functions we already checked override for overload as a whole. if e.func.info and not e.func.is_dynamic() and not e.is_overload: - found_base_method = self.check_method_override(e) - if e.func.is_explicit_override and found_base_method is False: + method_base_class = self.check_method_override(e) + if e.func.is_explicit_override and not method_base_class: self.msg.no_overridable_method(e.func.name, e.func) elif ( - found_base_method + method_base_class and self.options.strict_override_decorator and not e.func.is_explicit_override ): - self.msg.override_decorator_missing(e.func.name, e.func) + self.msg.override_decorator_missing( + e.func.name, method_base_class.fullname, e.func + ) if e.func.info and e.func.name in ("__init__", "__new__"): if e.type and not isinstance(get_proper_type(e.type), (FunctionLike, AnyType)): diff --git a/mypy/messages.py b/mypy/messages.py index 34df4a6e62cb..af276c670bf5 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1525,10 +1525,10 @@ def no_overridable_method(self, name: str, context: Context) -> None: context, ) - def override_decorator_missing(self, name: str, context: Context) -> None: + def override_decorator_missing(self, name: str, base_name: str, context: Context) -> None: self.fail( f'Method "{name}" is not marked as override ' - "but is overriding a method in a base class", + f'but is overriding a method in class "{base_name}"', context, ) diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index ebd914f27fd7..cd0b30a8f2fd 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -3013,7 +3013,10 @@ class B(A): def f(self, y: int) -> str: pass class C(A): - def f(self, y: int) -> str: pass # E: Method "f" is not marked as override but is overriding a method in a base class + def f(self, y: int) -> str: pass # E: Method "f" is not marked as override but is overriding a method in class "__main__.A" + +class D(B): + def f(self, y: int) -> str: pass # E: Method "f" is not marked as override but is overriding a method in class "__main__.B" [typing fixtures/typing-override.pyi] [case requireExplicitOverrideSpecialMethod] @@ -3042,7 +3045,7 @@ class B(A): class C(A): @property - def prop(self) -> int: pass # E: Method "prop" is not marked as override but is overriding a method in a base class + def prop(self) -> int: pass # E: Method "prop" is not marked as override but is overriding a method in class "__main__.A" [typing fixtures/typing-override.pyi] [builtins fixtures/property.pyi] @@ -3070,5 +3073,22 @@ class C(A): def f(self, y: int) -> str: ... @overload def f(self, y: str) -> str: ... - def f(self, y): pass # E: Method "f" is not marked as override but is overriding a method in a base class + def f(self, y): pass # E: Method "f" is not marked as override but is overriding a method in class "__main__.A" +[typing fixtures/typing-override.pyi] + +[case requireExplicitOverrideMultipleInheritance] +# flags: --strict-override-decorator --python-version 3.12 +from typing import override + +class A: + def f(self, x: int) -> str: pass +class B: + def f(self, y: int) -> str: pass + +class C(A, B): + @override + def f(self, z: int) -> str: pass + +class D(A, B): + def f(self, z: int) -> str: pass # E: Method "f" is not marked as override but is overriding a method in class "__main__.A" [typing fixtures/typing-override.pyi] From b33386c3fdeb05b74a821f32ffb81ab1896587d9 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Wed, 5 Jul 2023 11:47:58 +0200 Subject: [PATCH 4/6] Fix --- mypy/checker.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index b578cc5fae26..be0f8d83646a 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -643,16 +643,16 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None: if defn.impl: defn.impl.accept(self) if defn.info: - method_base_class = self.check_method_override(defn) - if defn.is_explicit_override and not method_base_class: + found_method_base_classes = self.check_method_override(defn) + if defn.is_explicit_override and not found_method_base_classes: self.msg.no_overridable_method(defn.name, defn) elif ( - method_base_class + found_method_base_classes and self.options.strict_override_decorator and not defn.is_explicit_override ): self.msg.override_decorator_missing( - defn.name, method_base_class.fullname, defn.impl or defn + defn.name, found_method_base_classes[0].fullname, defn.impl or defn ) self.check_inplace_operator_method(defn) if not defn.is_property: @@ -980,14 +980,14 @@ def _visit_func_def(self, defn: FuncDef) -> None: # overload, the legality of the override has already # been typechecked, and decorated methods will be # checked when the decorator is. - method_base_class = self.check_method_override(defn) + found_method_base_classes = self.check_method_override(defn) if ( - method_base_class + found_method_base_classes and self.options.strict_override_decorator and defn.name not in ("__init__", "__new__") ): self.msg.override_decorator_missing( - defn.name, method_base_class.fullname, defn + defn.name, found_method_base_classes[0].fullname, defn ) self.check_inplace_operator_method(defn) if defn.original_def: @@ -1832,24 +1832,24 @@ def expand_typevars( def check_method_override( self, defn: FuncDef | OverloadedFuncDef | Decorator - ) -> TypeInfo | None: + ) -> list[TypeInfo] | None: """Check if function definition is compatible with base classes. This may defer the method if a signature is not available in at least one base class. Return ``None`` if that happens. - Return the first base class if an attribute with the method name was found within it. - Return ``True`` if an attribute with the method name was found in the base class. + Return a list of base classes which contain an attribute with the method name. """ # Check against definitions in base classes. + found_method_base_classes: list[TypeInfo] = [] for base in defn.info.mro[1:]: result = self.check_method_or_accessor_override_for_base(defn, base) if result is None: # Node was deferred, we will have another attempt later. return None if result: - return base - return None + found_method_base_classes.append(base) + return found_method_base_classes def check_method_or_accessor_override_for_base( self, defn: FuncDef | OverloadedFuncDef | Decorator, base: TypeInfo @@ -4759,16 +4759,16 @@ def visit_decorator(self, e: Decorator) -> None: self.check_incompatible_property_override(e) # For overloaded functions we already checked override for overload as a whole. if e.func.info and not e.func.is_dynamic() and not e.is_overload: - method_base_class = self.check_method_override(e) - if e.func.is_explicit_override and not method_base_class: + found_method_base_classes = self.check_method_override(e) + if e.func.is_explicit_override and not found_method_base_classes: self.msg.no_overridable_method(e.func.name, e.func) elif ( - method_base_class + found_method_base_classes and self.options.strict_override_decorator and not e.func.is_explicit_override ): self.msg.override_decorator_missing( - e.func.name, method_base_class.fullname, e.func + e.func.name, found_method_base_classes[0].fullname, e.func ) if e.func.info and e.func.name in ("__init__", "__new__"): From e4d7096751d862b33ccc15f8091860cb49369020 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Wed, 5 Jul 2023 21:17:46 +0200 Subject: [PATCH 5/6] Fix deferred case --- mypy/checker.py | 12 ++++++++-- test-data/unit/check-functions.test | 26 +++++++++++++++++++++ test-data/unit/fixtures/typing-override.pyi | 1 + 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index be0f8d83646a..cc4554def9e2 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -644,7 +644,11 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None: defn.impl.accept(self) if defn.info: found_method_base_classes = self.check_method_override(defn) - if defn.is_explicit_override and not found_method_base_classes: + if ( + defn.is_explicit_override + and not found_method_base_classes + and found_method_base_classes is not None + ): self.msg.no_overridable_method(defn.name, defn) elif ( found_method_base_classes @@ -4760,7 +4764,11 @@ def visit_decorator(self, e: Decorator) -> None: # For overloaded functions we already checked override for overload as a whole. if e.func.info and not e.func.is_dynamic() and not e.is_overload: found_method_base_classes = self.check_method_override(e) - if e.func.is_explicit_override and not found_method_base_classes: + if ( + e.func.is_explicit_override + and not found_method_base_classes + and found_method_base_classes is not None + ): self.msg.no_overridable_method(e.func.name, e.func) elif ( found_method_base_classes diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index cd0b30a8f2fd..0d116b416d63 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -3001,6 +3001,32 @@ class C(A): def f(self, y: int | str) -> str: pass [typing fixtures/typing-override.pyi] +[case explicitOverrideCyclicDependency] +# flags: --python-version 3.12 +import b +[file a.py] +from typing import override +import b +import c + +class A(b.B): + @override # This is fine + @c.deco + def meth(self) -> int: ... +[file b.py] +import a +import c + +class B: + @c.deco + def meth(self) -> int: ... +[file c.py] +from typing import TypeVar, Tuple, Callable +T = TypeVar('T') +def deco(f: Callable[..., T]) -> Callable[..., Tuple[T, int]]: ... +[builtins fixtures/tuple.pyi] +[typing fixtures/typing-override.pyi] + [case requireExplicitOverrideMethod] # flags: --strict-override-decorator --python-version 3.12 from typing import override diff --git a/test-data/unit/fixtures/typing-override.pyi b/test-data/unit/fixtures/typing-override.pyi index b56be5cd819c..606ca63d4f0d 100644 --- a/test-data/unit/fixtures/typing-override.pyi +++ b/test-data/unit/fixtures/typing-override.pyi @@ -8,6 +8,7 @@ Optional = 0 Self = 0 Tuple = 0 ClassVar = 0 +Callable = 0 T = TypeVar('T') T_co = TypeVar('T_co', covariant=True) From 2a55f90cf9ab52f8b397716762201b188ec77680 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Tue, 11 Jul 2023 20:42:41 +0200 Subject: [PATCH 6/6] Switch to new error code + code review --- docs/source/class_basics.rst | 9 +++-- docs/source/config_file.rst | 7 ---- docs/source/error_code_list2.rst | 39 ++++++++++++++++++++ mypy/checker.py | 42 +++++++++------------ mypy/errorcodes.py | 6 +++ mypy/main.py | 8 ---- mypy/messages.py | 7 +++- mypy/options.py | 4 -- test-data/unit/check-functions.test | 57 +++++++++++++++++++++++------ 9 files changed, 119 insertions(+), 60 deletions(-) diff --git a/docs/source/class_basics.rst b/docs/source/class_basics.rst index aec9e386d55e..73f95f1c5658 100644 --- a/docs/source/class_basics.rst +++ b/docs/source/class_basics.rst @@ -210,7 +210,9 @@ override has a compatible signature: In order to ensure that your code remains correct when renaming methods, it can be helpful to explicitly mark a method as overriding a base -method. This can be done with the ``@override`` decorator. If the base +method. This can be done with the ``@override`` decorator. ``@override`` +can be imported from ``typing`` starting with Python 3.12 or from +``typing_extensions`` for use with older Python versions. If the base method is then renamed while the overriding method is not, mypy will show an error: @@ -235,9 +237,8 @@ show an error: .. note:: - Use ``--strict-override-decorator`` or - :confval:`strict_override_decorator = True ` to require - methods overrides use the ``@override`` decorator. Emit an error if it is missing. + Use :ref:`--enable-error-code explicit-override ` to require + that method overrides use the ``@override`` decorator. Emit an error if it is missing. You can also override a statically typed method with a dynamically typed one. This allows dynamically typed code to override methods diff --git a/docs/source/config_file.rst b/docs/source/config_file.rst index 37861ec4d750..9e79ff99937b 100644 --- a/docs/source/config_file.rst +++ b/docs/source/config_file.rst @@ -714,13 +714,6 @@ section of the command line docs. Prohibit equality checks, identity checks, and container checks between non-overlapping types. -.. confval:: strict_override_decorator - - :type: boolean - :default: False - - Require ``override`` decorator if method is overriding a base class method. - .. confval:: strict :type: boolean diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index e1d47f7cbec0..30fad0793771 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -442,3 +442,42 @@ Example: # The following will not generate an error on either # Python 3.8, or Python 3.9 42 + "testing..." # type: ignore + +.. _code-explicit-override: + +Check that ``@override`` is used when overriding a base class method [explicit-override] +---------------------------------------------------------------------------------------- + +If you use :option:`--enable-error-code explicit-override ` +mypy generates an error if you override a base class method without using the +``@override`` decorator. An error will not be emitted for overrides of ``__init__`` +or ``__new__``. See `PEP 698 `_. + +.. note:: + + Starting with Python 3.12, the ``@override`` decorator can be imported from ``typing``. + To use it with older Python versions, import it from ``typing_extensions`` instead. + +Example: + +.. code-block:: python + + # Use "mypy --enable-error-code explicit-override ..." + + from typing import override + + class Parent: + def f(self, x: int) -> None: + pass + + def g(self, y: int) -> None: + pass + + + class Child(Parent): + def f(self, x: int) -> None: # Error: Missing @override decorator + pass + + @override + def g(self, y: int) -> None: + pass diff --git a/mypy/checker.py b/mypy/checker.py index f26c0085f9d0..71c9746ce24f 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -650,14 +650,7 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None: and found_method_base_classes is not None ): self.msg.no_overridable_method(defn.name, defn) - elif ( - found_method_base_classes - and self.options.strict_override_decorator - and not defn.is_explicit_override - ): - self.msg.override_decorator_missing( - defn.name, found_method_base_classes[0].fullname, defn.impl or defn - ) + self.check_explicit_override_decorator(defn, found_method_base_classes, defn.impl) self.check_inplace_operator_method(defn) if not defn.is_property: self.check_overlapping_overloads(defn) @@ -985,14 +978,7 @@ def _visit_func_def(self, defn: FuncDef) -> None: # been typechecked, and decorated methods will be # checked when the decorator is. found_method_base_classes = self.check_method_override(defn) - if ( - found_method_base_classes - and self.options.strict_override_decorator - and defn.name not in ("__init__", "__new__") - ): - self.msg.override_decorator_missing( - defn.name, found_method_base_classes[0].fullname, defn - ) + self.check_explicit_override_decorator(defn, found_method_base_classes) self.check_inplace_operator_method(defn) if defn.original_def: # Override previous definition. @@ -1833,6 +1819,21 @@ def expand_typevars( else: return [(defn, typ)] + def check_explicit_override_decorator( + self, + defn: FuncDef | OverloadedFuncDef, + found_method_base_classes: list[TypeInfo] | None, + context: Context | None = None, + ) -> None: + if ( + found_method_base_classes + and not defn.is_explicit_override + and defn.name not in ("__init__", "__new__") + ): + self.msg.explicit_override_decorator_missing( + defn.name, found_method_base_classes[0].fullname, context or defn + ) + def check_method_override( self, defn: FuncDef | OverloadedFuncDef | Decorator ) -> list[TypeInfo] | None: @@ -4769,14 +4770,7 @@ def visit_decorator(self, e: Decorator) -> None: and found_method_base_classes is not None ): self.msg.no_overridable_method(e.func.name, e.func) - elif ( - found_method_base_classes - and self.options.strict_override_decorator - and not e.func.is_explicit_override - ): - self.msg.override_decorator_missing( - e.func.name, found_method_base_classes[0].fullname, e.func - ) + self.check_explicit_override_decorator(e.func, found_method_base_classes) if e.func.info and e.func.name in ("__init__", "__new__"): if e.type and not isinstance(get_proper_type(e.type), (FunctionLike, AnyType)): diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index 68ae4b49a806..717629ad1f11 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -235,6 +235,12 @@ def __hash__(self) -> int: UNUSED_IGNORE: Final = ErrorCode( "unused-ignore", "Ensure that all type ignores are used", "General", default_enabled=False ) +EXPLICIT_OVERRIDE_REQUIRED: Final = ErrorCode( + "explicit-override", + "Require @override decorator if method is overriding a base class method", + "General", + default_enabled=False, +) # Syntax errors are often blocking. diff --git a/mypy/main.py b/mypy/main.py index e6dc8ffe257f..f6e617e4d84f 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -824,14 +824,6 @@ def add_invertible_flag( group=strictness_group, ) - add_invertible_flag( - "--strict-override-decorator", - default=False, - strict_flag=False, - help="Require override decorator if method is overriding a base class method.", - group=strictness_group, - ) - add_invertible_flag( "--extra-checks", default=False, diff --git a/mypy/messages.py b/mypy/messages.py index af276c670bf5..ae7fba1473ac 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1525,11 +1525,14 @@ def no_overridable_method(self, name: str, context: Context) -> None: context, ) - def override_decorator_missing(self, name: str, base_name: str, context: Context) -> None: + def explicit_override_decorator_missing( + self, name: str, base_name: str, context: Context + ) -> None: self.fail( - f'Method "{name}" is not marked as override ' + f'Method "{name}" is not using @override ' f'but is overriding a method in class "{base_name}"', context, + code=codes.EXPLICIT_OVERRIDE_REQUIRED, ) def final_cant_override_writable(self, name: str, ctx: Context) -> None: diff --git a/mypy/options.py b/mypy/options.py index 0f3400bf9152..75343acd38bb 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -51,7 +51,6 @@ class BuildType: "strict_concatenate", "strict_equality", "strict_optional", - "strict_override_decorator", "warn_no_return", "warn_return_any", "warn_unreachable", @@ -201,9 +200,6 @@ def __init__(self) -> None: # This makes 1 == '1', 1 in ['1'], and 1 is '1' errors. self.strict_equality = False - # Require override decorator. Strict mode for PEP 698. - self.strict_override_decorator = False - # Deprecated, use extra_checks instead. self.strict_concatenate = False diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index 309b5e475a36..0de4798ea1f5 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -3035,7 +3035,7 @@ def deco(f: Callable[..., T]) -> Callable[..., Tuple[T, int]]: ... [typing fixtures/typing-override.pyi] [case requireExplicitOverrideMethod] -# flags: --strict-override-decorator --python-version 3.12 +# flags: --enable-error-code explicit-override --python-version 3.12 from typing import override class A: @@ -3046,25 +3046,52 @@ class B(A): def f(self, y: int) -> str: pass class C(A): - def f(self, y: int) -> str: pass # E: Method "f" is not marked as override but is overriding a method in class "__main__.A" + def f(self, y: int) -> str: pass # E: Method "f" is not using @override but is overriding a method in class "__main__.A" class D(B): - def f(self, y: int) -> str: pass # E: Method "f" is not marked as override but is overriding a method in class "__main__.B" + def f(self, y: int) -> str: pass # E: Method "f" is not using @override but is overriding a method in class "__main__.B" [typing fixtures/typing-override.pyi] [case requireExplicitOverrideSpecialMethod] -# flags: --strict-override-decorator --python-version 3.12 -from typing import Self, override +# flags: --enable-error-code explicit-override --python-version 3.12 +from typing import Callable, Self, TypeVar, override, overload + +T = TypeVar('T') +def some_decorator(f: Callable[..., T]) -> Callable[..., T]: ... # Don't require override decorator for __init__ and __new__ # See: https://github.com/python/typing/issues/1376 class A: def __init__(self) -> None: pass def __new__(cls) -> Self: pass + +class B(A): + def __init__(self) -> None: pass + def __new__(cls) -> Self: pass + +class C(A): + @some_decorator + def __init__(self) -> None: pass + + @some_decorator + def __new__(cls) -> Self: pass + +class D(A): + @overload + def __init__(self, x: int) -> None: ... + @overload + def __init__(self, x: str) -> None: ... + def __init__(self, x): pass + + @overload + def __new__(cls, x: int) -> Self: pass + @overload + def __new__(cls, x: str) -> Self: pass + def __new__(cls, x): pass [typing fixtures/typing-override.pyi] [case requireExplicitOverrideProperty] -# flags: --strict-override-decorator --python-version 3.12 +# flags: --enable-error-code explicit-override --python-version 3.12 from typing import override class A: @@ -3078,12 +3105,12 @@ class B(A): class C(A): @property - def prop(self) -> int: pass # E: Method "prop" is not marked as override but is overriding a method in class "__main__.A" + def prop(self) -> int: pass # E: Method "prop" is not using @override but is overriding a method in class "__main__.A" [typing fixtures/typing-override.pyi] [builtins fixtures/property.pyi] [case requireExplicitOverrideOverload] -# flags: --strict-override-decorator --python-version 3.12 +# flags: --enable-error-code explicit-override --python-version 3.12 from typing import overload, override class A: @@ -3102,15 +3129,23 @@ class B(A): def f(self, y): pass class C(A): + @overload + @override + def f(self, y: int) -> str: ... + @overload + def f(self, y: str) -> str: ... + def f(self, y): pass + +class D(A): @overload def f(self, y: int) -> str: ... @overload def f(self, y: str) -> str: ... - def f(self, y): pass # E: Method "f" is not marked as override but is overriding a method in class "__main__.A" + def f(self, y): pass # E: Method "f" is not using @override but is overriding a method in class "__main__.A" [typing fixtures/typing-override.pyi] [case requireExplicitOverrideMultipleInheritance] -# flags: --strict-override-decorator --python-version 3.12 +# flags: --enable-error-code explicit-override --python-version 3.12 from typing import override class A: @@ -3123,5 +3158,5 @@ class C(A, B): def f(self, z: int) -> str: pass class D(A, B): - def f(self, z: int) -> str: pass # E: Method "f" is not marked as override but is overriding a method in class "__main__.A" + def f(self, z: int) -> str: pass # E: Method "f" is not using @override but is overriding a method in class "__main__.A" [typing fixtures/typing-override.pyi]