Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validate more about overrides on untyped methods #17276

Merged
merged 2 commits into from
May 23, 2024
Merged

Validate more about overrides on untyped methods #17276

merged 2 commits into from
May 23, 2024
+49 −7

Conversation

stroxler
Copy link
Contributor

This commit fixes #9618 by making MyPy always complain if a method overrides a base class method marked as @final.

In the process, it also adds a few additional validations:

  • Always verify the @override decorator, which ought to be pretty backward-compatible for most projects assuming that strict override checks aren't enabled by default (and it appears to me that --enable-error-code explicit-override is off by default)
  • Verify that the method signature is compatible (which in practice means only arity and argument name checks) if the --check-untyped-defs flag is set; it seems unlikely that a user would want mypy to validate the bodies of untyped functions but wouldn't want to be alerted about incompatible overrides.

Note: I did also explore enabling the signature compatibility check for all code, which in principle makes sense. But the mypy_primer results indicated that there would be backward compability issues because too many libraries rely on us not validating this: #17274

@stroxler
Validate more about overrides on untyped methods
3b5105e 
This commit fixes python#9618 by making MyPy always complain if a method
overrides a base class method marked as `@final`.

In the process, it also adds a few additional validations:
- Always verify the `@override` decorator, which ought to be pretty
  backward-compatible for most projects assuming that strict override
  checks aren't enabled by default (and it appears to me that
  `--enable-error-code explicit-override` is off by default)
- Verify that the method signature is compatible (which in practice
  means only arity and argument name checks) *if* the
  `--check-untyped-defs` flag is set; it seems unlikely that a user
  would want mypy to validate the bodies of untyped functions but
  wouldn't want to be alerted about incompatible overrides.

Note: I did also explore enabling the signature compatibility check
for all code, which in principle makes sense. But the mypy_primer
results indicated that there would be backward compability issues
because too many libraries rely on us not validating this:
python#17274
@stroxler stroxler mentioned this pull request May 21, 2024
Closed
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
hauntsaninja
hauntsaninja approved these changes May 23, 2024
View reviewed changes
Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooray, thank you!

@hauntsaninja hauntsaninja merged commit 25087fd into python:master May 23, 2024
18 checks passed
@github-actions GitHub Actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/arrays/boolean.py:359: error: Signature of "_logical_method" incompatible with supertype "BaseMaskedArray"  [override]
+ pandas/core/arrays/boolean.py:359: note:      Superclass:
+ pandas/core/arrays/boolean.py:359: note:          def _arith_method(self: BaseMaskedArray, other: Any, op: Any) -> Any
+ pandas/core/arrays/boolean.py:359: note:      Subclass:
+ pandas/core/arrays/boolean.py:359: note:          def _logical_method(self, other: Any, op: Any) -> Any
+ pandas/core/arrays/boolean.py:359: note:      Superclass:
+ pandas/core/arrays/boolean.py:359: note:          def _arith_method(self: BaseMaskedArray, other: Any, op: Any) -> Any
+ pandas/core/arrays/boolean.py:359: note:      Subclass:
+ pandas/core/arrays/boolean.py:359: note:          def _logical_method(self, other: Any, op: Any) -> Any
+ pandas/core/indexes/interval.py:993: error: Signature of "_intersection" incompatible with supertype "Index"  [override]
+ pandas/core/indexes/interval.py:993: note:      Superclass:
+ pandas/core/indexes/interval.py:993: note:          def _intersection(self, other: Index, sort: bool = ...) -> Any
+ pandas/core/indexes/interval.py:993: note:      Subclass:
+ pandas/core/indexes/interval.py:993: note:          def _intersection(self, other: Any, sort: Any) -> Any
+ pandas/core/resample.py:1624: error: Signature of "_downsample" incompatible with supertype "Resampler"  [override]
+ pandas/core/resample.py:1624: note:      Superclass:
+ pandas/core/resample.py:1624: note:          def _downsample(self, f: Any, **kwargs: Any) -> Any
+ pandas/core/resample.py:1624: note:      Subclass:
+ pandas/core/resample.py:1624: note:          def _downsample(self, how: Any, **kwargs: Any) -> Any
+ pandas/core/resample.py:1783: error: Signature of "_downsample" incompatible with supertype "Resampler"  [override]
+ pandas/core/resample.py:1783: note:      Superclass:
+ pandas/core/resample.py:1783: note:          def _downsample(self, f: Any, **kwargs: Any) -> Any
+ pandas/core/resample.py:1783: note:      Subclass:
+ pandas/core/resample.py:1783: note:          def _downsample(self, how: Any, **kwargs: Any) -> Any

aioredis (https://github.com/aio-libs/aioredis)
+ aioredis/client.py:146: error: Signature of "update" incompatible with supertype "MutableMapping"  [override]
+ aioredis/client.py:146: note:      Superclass:
+ aioredis/client.py:146: note:          @overload
+ aioredis/client.py:146: note:          def update(self, SupportsKeysAndGetItem[Any, Any], /, **kwargs: Any) -> None
+ aioredis/client.py:146: note:          @overload
+ aioredis/client.py:146: note:          def update(self, Iterable[tuple[Any, Any]], /, **kwargs: Any) -> None
+ aioredis/client.py:146: note:          @overload
+ aioredis/client.py:146: note:          def update(self, **kwargs: Any) -> None
+ aioredis/client.py:146: note:      Subclass:
+ aioredis/client.py:146: note:          def update(self, data: Any) -> Any

tornado (https://github.com/tornadoweb/tornado)
+ tornado/test/websocket_test.py:81: error: Signature of "open" incompatible with supertype "WebSocketHandler"  [override]
+ tornado/test/websocket_test.py:81: note:      Superclass:
+ tornado/test/websocket_test.py:81: note:          def open(self, *args: str, **kwargs: str) -> Optional[Awaitable[None]]
+ tornado/test/websocket_test.py:81: note:      Subclass:
+ tornado/test/websocket_test.py:81: note:          def open(self) -> Any
+ tornado/test/websocket_test.py:123: error: Signature of "open" incompatible with supertype "WebSocketHandler"  [override]
+ tornado/test/websocket_test.py:123: note:      Superclass:
+ tornado/test/websocket_test.py:123: note:          def open(self, *args: str, **kwargs: str) -> Optional[Awaitable[None]]
+ tornado/test/websocket_test.py:123: note:      Subclass:
+ tornado/test/websocket_test.py:123: note:          def open(self) -> Any
+ tornado/test/websocket_test.py:138: error: Signature of "open" incompatible with supertype "WebSocketHandler"  [override]
+ tornado/test/websocket_test.py:138: note:      Superclass:
+ tornado/test/websocket_test.py:138: note:          def open(self, *args: str, **kwargs: str) -> Optional[Awaitable[None]]
+ tornado/test/websocket_test.py:138: note:      Subclass:
+ tornado/test/websocket_test.py:138: note:          def open(self, arg: Any) -> Any
+ tornado/test/websocket_test.py:143: error: Signature of "initialize" incompatible with supertype "TestWebSocketHandler"  [override]
+ tornado/test/websocket_test.py:143: note:      Superclass:
+ tornado/test/websocket_test.py:143: note:          def initialize(self, close_future: Any = ..., compression_options: Any = ...) -> Any
+ tornado/test/websocket_test.py:143: note:      Subclass:
+ tornado/test/websocket_test.py:143: note:          def initialize(self, **kwargs: Any) -> Any
+ tornado/test/websocket_test.py:163: error: Signature of "initialize" incompatible with supertype "TestWebSocketHandler"  [override]
+ tornado/test/websocket_test.py:163: note:      Superclass:
+ tornado/test/websocket_test.py:163: note:          def initialize(self, close_future: Any = ..., compression_options: Any = ...) -> Any
+ tornado/test/websocket_test.py:163: note:      Subclass:
+ tornado/test/websocket_test.py:163: note:          def initialize(self, **kwargs: Any) -> Any
+ tornado/test/websocket_test.py:175: error: Signature of "open" incompatible with supertype "WebSocketHandler"  [override]
+ tornado/test/websocket_test.py:175: note:      Superclass:
+ tornado/test/websocket_test.py:175: note:          def open(self, *args: str, **kwargs: str) -> Optional[Awaitable[None]]
+ tornado/test/websocket_test.py:175: note:      Subclass:
+ tornado/test/websocket_test.py:175: note:          def open(self) -> Any
+ tornado/test/websocket_test.py:182: error: Signature of "initialize" incompatible with supertype "TestWebSocketHandler"  [override]
+ tornado/test/websocket_test.py:182: note:      Superclass:
+ tornado/test/websocket_test.py:182: note:          def initialize(self, close_future: Any = ..., compression_options: Any = ...) -> Any
+ tornado/test/websocket_test.py:182: note:      Subclass:
+ tornado/test/websocket_test.py:182: note:          def initialize(self, test: Any, **kwargs: Any) -> Any
+ tornado/test/websocket_test.py:200: error: Signature of "open" incompatible with supertype "WebSocketHandler"  [override]
+ tornado/test/websocket_test.py:200: note:      Superclass:
+ tornado/test/websocket_test.py:200: note:          def open(self, *args: str, **kwargs: str) -> Optional[Awaitable[None]]
+ tornado/test/websocket_test.py:200: note:      Subclass:
+ tornado/test/websocket_test.py:200: note:          def open(self) -> Any
+ tornado/test/websocket_test.py:205: error: Signature of "open" incompatible with supertype "WebSocketHandler"  [override]
+ tornado/test/websocket_test.py:205: note:      Superclass:
+ tornado/test/websocket_test.py:205: note:          def open(self, *args: str, **kwargs: str) -> Optional[Awaitable[None]]
+ tornado/test/websocket_test.py:205: note:      Subclass:
+ tornado/test/websocket_test.py:205: note:          def open(self) -> Any
+ tornado/test/websocket_test.py:211: error: Signature of "open" incompatible with supertype "WebSocketHandler"  [override]
+ tornado/test/websocket_test.py:211: note:      Superclass:
+ tornado/test/websocket_test.py:211: note:          def open(self, *args: str, **kwargs: str) -> Optional[Awaitable[None]]
+ tornado/test/websocket_test.py:211: note:      Subclass:
+ tornado/test/websocket_test.py:211: note:          def open(self) -> Any
+ tornado/test/websocket_test.py:635: error: Signature of "initialize" incompatible with supertype "TestWebSocketHandler"  [override]
+ tornado/test/websocket_test.py:635: note:      Superclass:
+ tornado/test/websocket_test.py:635: note:          def initialize(self, close_future: Any = ..., compression_options: Any = ...) -> Any
+ tornado/test/websocket_test.py:635: note:      Subclass:
+ tornado/test/websocket_test.py:635: note:          def initialize(self, **kwargs: Any) -> Any
+ tornado/test/web_test.py:114: error: Signature of "get_cookie" incompatible with supertype "RequestHandler"  [override]
+ tornado/test/web_test.py:114: note:      Superclass:
+ tornado/test/web_test.py:114: note:          def get_cookie(self, name: str, default: Optional[str] = ...) -> Optional[str]
+ tornado/test/web_test.py:114: note:      Subclass:
+ tornado/test/web_test.py:114: note:          def get_cookie(self, name: Any) -> Any
+ tornado/test/web_test.py:117: error: Signature of "set_cookie" incompatible with supertype "RequestHandler"  [override]
+ tornado/test/web_test.py:117: note:      Superclass:
+ tornado/test/web_test.py:117: note:          def set_cookie(self, name: str, value: Union[str, bytes], domain: Optional[str] = ..., expires: Union[float, Tuple[Any, ...], datetime, None] = ..., path: str = ..., expires_days: Optional[float] = ..., *, max_age: Optional[int] = ..., httponly: bool = ..., secure: bool = ..., samesite: Optional[str] = ..., **kwargs: Any) -> None
+ tornado/test/web_test.py:117: note:      Subclass:
+ tornado/test/web_test.py:117: note:          def set_cookie(self, name: Any, value: Any, expires_days: Any = ...) -> Any
+ tornado/test/ioloop_test.py:556: error: Signature of "submit" incompatible with supertype "Executor"  [override]
+ tornado/test/ioloop_test.py:556: note:      Superclass:
+ tornado/test/ioloop_test.py:556: note:          def [_P`-1, _T] submit(self, fn: Callable[_P, _T], *args: _P.args, **kwargs: _P.kwargs) -> Future[_T]
+ tornado/test/ioloop_test.py:556: note:      Subclass:
+ tornado/test/ioloop_test.py:556: note:          def submit(self, func: Any, *args: Any) -> Any

pytest (https://github.com/pytest-dev/pytest)
+ testing/test_assertion.py:826: error: Signature of "insert" incompatible with supertype "MutableSequence"  [override]
+ testing/test_assertion.py:826: note:      Superclass:
+ testing/test_assertion.py:826: note:          def insert(self, index: int, value: int) -> None
+ testing/test_assertion.py:826: note:      Subclass:
+ testing/test_assertion.py:826: note:          def insert(self, item: Any, index: Any) -> Any

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
+ pandas-stubs/core/arrays/timedeltas.pyi:45: error: Signature of "__mul__" incompatible with supertype "DatetimeLikeArrayMixin"  [override]
+ pandas-stubs/core/arrays/timedeltas.pyi:45: note:      Superclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:45: note:          EllipsisType
+ pandas-stubs/core/arrays/timedeltas.pyi:45: note:      Subclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:45: note:          def __mul__(self, Any, /) -> Any
+ pandas-stubs/core/arrays/timedeltas.pyi:47: error: Signature of "__truediv__" incompatible with supertype "DatetimeLikeArrayMixin"  [override]
+ pandas-stubs/core/arrays/timedeltas.pyi:47: note:      Superclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:47: note:          EllipsisType
+ pandas-stubs/core/arrays/timedeltas.pyi:47: note:      Subclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:47: note:          def __truediv__(self, Any, /) -> Any
+ pandas-stubs/core/arrays/timedeltas.pyi:48: error: Signature of "__rtruediv__" incompatible with supertype "DatetimeLikeArrayMixin"  [override]
+ pandas-stubs/core/arrays/timedeltas.pyi:48: note:      Superclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:48: note:          EllipsisType
+ pandas-stubs/core/arrays/timedeltas.pyi:48: note:      Subclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:48: note:          def __rtruediv__(self, Any, /) -> Any
+ pandas-stubs/core/arrays/timedeltas.pyi:49: error: Signature of "__floordiv__" incompatible with supertype "DatetimeLikeArrayMixin"  [override]
+ pandas-stubs/core/arrays/timedeltas.pyi:49: note:      Superclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:49: note:          EllipsisType
+ pandas-stubs/core/arrays/timedeltas.pyi:49: note:      Subclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:49: note:          def __floordiv__(self, Any, /) -> Any
+ pandas-stubs/core/arrays/timedeltas.pyi:50: error: Signature of "__rfloordiv__" incompatible with supertype "DatetimeLikeArrayMixin"  [override]
+ pandas-stubs/core/arrays/timedeltas.pyi:50: note:      Superclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:50: note:          EllipsisType
+ pandas-stubs/core/arrays/timedeltas.pyi:50: note:      Subclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:50: note:          def __rfloordiv__(self, Any, /) -> Any
+ pandas-stubs/core/arrays/timedeltas.pyi:51: error: Signature of "__mod__" incompatible with supertype "DatetimeLikeArrayMixin"  [override]
+ pandas-stubs/core/arrays/timedeltas.pyi:51: note:      Superclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:51: note:          EllipsisType
+ pandas-stubs/core/arrays/timedeltas.pyi:51: note:      Subclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:51: note:          def __mod__(self, Any, /) -> Any
+ pandas-stubs/core/arrays/timedeltas.pyi:52: error: Signature of "__rmod__" incompatible with supertype "DatetimeLikeArrayMixin"  [override]
+ pandas-stubs/core/arrays/timedeltas.pyi:52: note:      Superclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:52: note:          EllipsisType
+ pandas-stubs/core/arrays/timedeltas.pyi:52: note:      Subclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:52: note:          def __rmod__(self, Any, /) -> Any
+ pandas-stubs/core/arrays/timedeltas.pyi:53: error: Signature of "__divmod__" incompatible with supertype "DatetimeLikeArrayMixin"  [override]
+ pandas-stubs/core/arrays/timedeltas.pyi:53: note:      Superclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:53: note:          EllipsisType
+ pandas-stubs/core/arrays/timedeltas.pyi:53: note:      Subclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:53: note:          def __divmod__(self, Any, /) -> Any
+ pandas-stubs/core/arrays/timedeltas.pyi:54: error: Signature of "__rdivmod__" incompatible with supertype "DatetimeLikeArrayMixin"  [override]
+ pandas-stubs/core/arrays/timedeltas.pyi:54: note:      Superclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:54: note:          EllipsisType
+ pandas-stubs/core/arrays/timedeltas.pyi:54: note:      Subclass:
+ pandas-stubs/core/arrays/timedeltas.pyi:54: note:          def __rdivmod__(self, other: Any) -> Any

streamlit (https://github.com/streamlit/streamlit)
+ lib/streamlit/web/server/routes.py: note: In member "initialize" of class "StaticFileHandler":
+ lib/streamlit/web/server/routes.py:44:5: error: Signature of "initialize" incompatible with supertype "StaticFileHandler"  [override]
+ lib/streamlit/web/server/routes.py:44:5: note:      Superclass:
+ lib/streamlit/web/server/routes.py:44:5: note:          def initialize(self, path: str, default_filename: Optional[str] = ...) -> None
+ lib/streamlit/web/server/routes.py:44:5: note:      Subclass:
+ lib/streamlit/web/server/routes.py:44:5: note:          def initialize(self, path: Any, default_filename: Any, get_pages: Any) -> Any

pwndbg (https://github.com/pwndbg/pwndbg)
+ pwndbg/commands/segments.py: note: In member "invoke" of class "segment":
+ pwndbg/commands/segments.py:17: error: Signature of "invoke" incompatible with supertype "Function"  [override]
+ pwndbg/commands/segments.py:17: note:      Superclass:
+ pwndbg/commands/segments.py:17: note:          def invoke(self, *args: Value) -> bool | int | float | str | Value | LazyString
+ pwndbg/commands/segments.py:17: note:      Subclass:
+ pwndbg/commands/segments.py:17: note:          def invoke(self, arg: Any = ...) -> Any
+ pwndbg/commands/argv.py: note: In member "invoke" of class "argv_function":
+ pwndbg/commands/argv.py:74: error: Signature of "invoke" incompatible with supertype "Function"  [override]
+ pwndbg/commands/argv.py:74: note:      Superclass:
+ pwndbg/commands/argv.py:74: note:          def invoke(self, *args: Value) -> bool | int | float | str | Value | LazyString
+ pwndbg/commands/argv.py:74: note:      Subclass:
+ pwndbg/commands/argv.py:74: note:          def invoke(self, number: Any = ...) -> Any
+ pwndbg/commands/argv.py: note: In member "invoke" of class "envp_function":
+ pwndbg/commands/argv.py:97: error: Signature of "invoke" incompatible with supertype "Function"  [override]
+ pwndbg/commands/argv.py:97: note:      Superclass:
+ pwndbg/commands/argv.py:97: note:          def invoke(self, *args: Value) -> bool | int | float | str | Value | LazyString
+ pwndbg/commands/argv.py:97: note:      Subclass:
+ pwndbg/commands/argv.py:97: note:          def invoke(self, number: Any = ...) -> Any
+ pwndbg/commands/argv.py: note: In member "invoke" of class "argc_function":
+ pwndbg/commands/argv.py:120: error: Signature of "invoke" incompatible with supertype "Function"  [override]
+ pwndbg/commands/argv.py:120: note:      Superclass:
+ pwndbg/commands/argv.py:120: note:          def invoke(self, *args: Value) -> bool | int | float | str | Value | LazyString
+ pwndbg/commands/argv.py:120: note:      Subclass:
+ pwndbg/commands/argv.py:120: note:          def invoke(self, number: Any = ...) -> Any
+ pwndbg/commands/argv.py: note: In member "invoke" of class "environ_function":
+ pwndbg/commands/argv.py:135: error: Signature of "invoke" incompatible with supertype "Function"  [override]
+ pwndbg/commands/argv.py:135: note:      Superclass:
+ pwndbg/commands/argv.py:135: note:          def invoke(self, *args: Value) -> bool | int | float | str | Value | LazyString
+ pwndbg/commands/argv.py:135: note:      Subclass:
+ pwndbg/commands/argv.py:135: note:          def invoke(self, name: Any) -> Any

@asottile-sentry
Copy link

fwiw this is making it very difficult to upgrade mypy in a codebase with partial typing (notably: sentry) -- and it's especially annoying in tests where it's common to not require annotations but often have inheritance of setUp with alternative parameter structure:

class TestCase:
    def setUp(self, not_required='foo'): ...

class Sub(TestCase):
    def setUp(self):
        super().setUp(not_required='bar')

@AlexWaygood AlexWaygood mentioned this pull request Aug 1, 2024
Merged
@antonagestam
Copy link
Contributor

It looks like this also addressed #7015 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hauntsaninja hauntsaninja hauntsaninja approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

@final decorator is not honored on override with --check-untyped-defs
4 participants
@stroxler @asottile-sentry @antonagestam @hauntsaninja