Skip to content

Validate more about overrides on untyped methods - #17276

Merged
hauntsaninja merged 2 commits into
python:masterfrom
stroxler:stroxler-fixes-9618
May 23, 2024
Merged

Validate more about overrides on untyped methods#17276
hauntsaninja merged 2 commits into
python:masterfrom
stroxler:stroxler-fixes-9618

Conversation

@stroxler

@stroxler stroxler commented May 21, 2024

Copy link
Copy Markdown
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

This comment has been minimized.

Sign in to view
hauntsaninja
hauntsaninja reviewed May 23, 2024
View reviewed changes
Comment thread test-data/unit/check-dynamic-typing.test Outdated
hauntsaninja
hauntsaninja approved these changes May 23, 2024
View reviewed changes

@hauntsaninja hauntsaninja left a comment

Copy link
Copy Markdown
Collaborator

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
@github-actions

github-actions Bot commented May 23, 2024

Copy link
Copy Markdown
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

asottile-sentry commented Jul 22, 2024

Copy link
Copy Markdown

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

antonagestam commented Aug 1, 2024

Copy link
Copy Markdown
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