Skip to content

Commit

Permalink
Exclude private attributes from override checks (#16464)
Browse files Browse the repository at this point in the history
Fixes #9910
Fixes #16452

We already exclude private names from override type compatibility checks
etc., but it looks like some override checks were still performed, we
need to skip them, since private name is actually a different name in
subclass.

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
  • Loading branch information
2 people authored and JukkaL committed Nov 15, 2023
1 parent 4b5b316 commit 88791ca
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 4 deletions.
7 changes: 5 additions & 2 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1879,6 +1879,7 @@ def check_explicit_override_decorator(
found_method_base_classes
and not defn.is_explicit_override
and defn.name not in ("__init__", "__new__")
and not is_private(defn.name)
):
self.msg.explicit_override_decorator_missing(
defn.name, found_method_base_classes[0].fullname, context or defn
Expand Down Expand Up @@ -1921,7 +1922,7 @@ def check_method_or_accessor_override_for_base(
base_attr = base.names.get(name)
if base_attr:
# First, check if we override a final (always an error, even with Any types).
if is_final_node(base_attr.node):
if is_final_node(base_attr.node) and not is_private(name):
self.msg.cant_override_final(name, base.name, defn)
# Second, final can't override anything writeable independently of types.
if defn.is_final:
Expand Down Expand Up @@ -2679,7 +2680,7 @@ class C(B, A[int]): ... # this is unsafe because...
ok = True
# Final attributes can never be overridden, but can override
# non-final read-only attributes.
if is_final_node(second.node):
if is_final_node(second.node) and not is_private(name):
self.msg.cant_override_final(name, base2.name, ctx)
if is_final_node(first.node):
self.check_if_final_var_override_writable(name, second.node, ctx)
Expand Down Expand Up @@ -3293,6 +3294,8 @@ def check_compatibility_final_super(
"""
if not isinstance(base_node, (Var, FuncBase, Decorator)):
return True
if is_private(node.name):
return True
if base_node.is_final and (node.is_final or not isinstance(base_node, Var)):
# Give this error only for explicit override attempt with `Final`, or
# if we are overriding a final method with variable.
Expand Down
13 changes: 13 additions & 0 deletions test-data/unit/check-dataclasses.test
Original file line number Diff line number Diff line change
Expand Up @@ -2531,3 +2531,16 @@ class Foo:

c: int # E: Name "c" already defined on line 5
[builtins fixtures/dataclasses.pyi]

[case testDataclassInheritanceWorksWithExplicitOverrides]
# flags: --enable-error-code explicit-override
from dataclasses import dataclass

@dataclass
class Base:
x: int

@dataclass
class Child(Base):
y: int
[builtins fixtures/dataclasses.pyi]
13 changes: 13 additions & 0 deletions test-data/unit/check-final.test
Original file line number Diff line number Diff line change
Expand Up @@ -1117,3 +1117,16 @@ from typing import Final
class MyClass:
a: None
a: Final[int] = 1 # E: Cannot redefine an existing name as final # E: Name "a" already defined on line 5

[case testFinalOverrideAllowedForPrivate]
from typing import Final, final

class Parent:
__foo: Final[int] = 0
@final
def __bar(self) -> None: ...

class Child(Parent):
__foo: Final[int] = 1
@final
def __bar(self) -> None: ...
10 changes: 10 additions & 0 deletions test-data/unit/check-functions.test
Original file line number Diff line number Diff line change
Expand Up @@ -3159,6 +3159,16 @@ class D(A, B):
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]

[case testExplicitOverrideAllowedForPrivate]
# flags: --enable-error-code explicit-override --python-version 3.12

class B:
def __f(self, y: int) -> str: pass

class C(B):
def __f(self, y: int) -> str: pass # OK
[typing fixtures/typing-override.pyi]

[case testCallableProperty]
from typing import Callable

Expand Down
4 changes: 2 additions & 2 deletions test-data/unit/fine-grained-dataclass-transform.test
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ class A(Dataclass):

[out]
main:7: error: Unexpected keyword argument "x" for "B"
builtins.pyi:13: note: "B" defined here
builtins.pyi:14: note: "B" defined here
main:7: error: Unexpected keyword argument "y" for "B"
builtins.pyi:13: note: "B" defined here
builtins.pyi:14: note: "B" defined here
==

[case frozenInheritanceViaDefault]
Expand Down
5 changes: 5 additions & 0 deletions test-data/unit/fixtures/dataclasses.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ from typing import (
Generic, Iterator, Iterable, Mapping, Optional, Sequence, Tuple,
TypeVar, Union, overload,
)
from typing_extensions import override

_T = TypeVar('_T')
_U = TypeVar('_U')
Expand All @@ -29,8 +30,10 @@ class dict(Mapping[KT, VT]):
def __init__(self, **kwargs: VT) -> None: pass
@overload
def __init__(self, arg: Iterable[Tuple[KT, VT]], **kwargs: VT) -> None: pass
@override
def __getitem__(self, key: KT) -> VT: pass
def __setitem__(self, k: KT, v: VT) -> None: pass
@override
def __iter__(self) -> Iterator[KT]: pass
def __contains__(self, item: object) -> int: pass
def update(self, a: Mapping[KT, VT]) -> None: pass
Expand All @@ -42,7 +45,9 @@ class dict(Mapping[KT, VT]):

class list(Generic[_T], Sequence[_T]):
def __contains__(self, item: object) -> int: pass
@override
def __getitem__(self, key: int) -> _T: pass
@override
def __iter__(self) -> Iterator[_T]: pass

class function: pass
Expand Down

0 comments on commit 88791ca

Please sign in to comment.