Skip to content

[red-knot] Check decorator consistency on overloads #17684

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
Merged
dhruvmanila merged 5 commits into from
Apr 30, 2025
Merged

[red-knot] Check decorator consistency on overloads #17684

dhruvmanila merged 5 commits into from
Apr 30, 2025

Conversation

@dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Apr 28, 2025 •
edited
Loading

Summary

Part of #15383.

As per the spec (https://typing.python.org/en/latest/spec/overload.html#invalid-overload-definitions):

For @staticmethod and @classmethod:

If one overload signature is decorated with @staticmethod or @classmethod, all overload signatures must be similarly decorated. The implementation, if present, must also have a consistent decorator. Type checkers should report an error if these conditions are not met.

For @final and @override:

If a @final or @override decorator is supplied for a function with overloads, the decorator should be applied only to the overload implementation if it is present. If an overload implementation isn’t present (for example, in a stub file), the @final or @override decorator should be applied only to the first overload. Type checkers should enforce these rules and generate an error when they are violated. If a @final or @override decorator follows these rules, a type checker should treat the decorator as if it is present on all overloads.

Test Plan

Update existing tests; add snapshots.

@dhruvmanila dhruvmanila added the ty Multi-file analysis & type inference label Apr 28, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2025 •
edited
Loading

mypy_primer results

No ecosystem changes detected ✅

@dhruvmanila dhruvmanila mentioned this pull request Apr 28, 2025
Closed
19 tasks
@dhruvmanila dhruvmanila force-pushed the dhruv/overload-without-implementation branch from cb23a34 to 24158a7 Compare April 29, 2025 13:46
@dhruvmanila dhruvmanila force-pushed the dhruv/overload-decorator-consistency branch from 8d14303 to 0b44bd4 Compare April 29, 2025 13:47
@dhruvmanila dhruvmanila force-pushed the dhruv/overload-without-implementation branch from 24158a7 to 10c5adf Compare April 29, 2025 16:16
@dhruvmanila dhruvmanila force-pushed the dhruv/overload-decorator-consistency branch 3 times, most recently from 18779ea to 35771de Compare April 29, 2025 16:45
carljm
carljm approved these changes Apr 29, 2025
View reviewed changes
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Excellent!

@dhruvmanila dhruvmanila force-pushed the dhruv/overload-without-implementation branch from 10c5adf to 9db545d Compare April 30, 2025 14:08
@dhruvmanila dhruvmanila added the diagnostics Related to reporting of diagnostics. label Apr 30, 2025
Base automatically changed from dhruv/overload-without-implementation to main April 30, 2025 14:24
@dhruvmanila dhruvmanila force-pushed the dhruv/overload-decorator-consistency branch from 35771de to 0403e2e Compare April 30, 2025 14:31
AlexWaygood
AlexWaygood reviewed Apr 30, 2025
View reviewed changes
@dhruvmanila
Copy instead of returning a reference
022e630
@dhruvmanila
Update snapshots
31f8319 
Why is this required? I already updated it earlier
@dhruvmanila dhruvmanila force-pushed the dhruv/overload-decorator-consistency branch from 6497f2d to 31f8319 Compare April 30, 2025 14:55
@dhruvmanila dhruvmanila merged commit 7568eeb into main Apr 30, 2025
35 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/overload-decorator-consistency branch April 30, 2025 15:04
dcreager added a commit that referenced this pull request May 1, 2025
@dcreager
Merge branch 'main' into dcreager/legacy-class
f07fce3 
* main:
  [red-knot] Preliminary `NamedTuple` support (#17738)
  [red-knot] Add tests for classes that have incompatible `__new__` and `__init__` methods (#17747)
  Update dependency vite to v6.2.7 (#17746)
  [red-knot] Update call binding to return all matching overloads (#17618)
  [`airflow`] apply Replacement::AutoImport to `AIR312` (#17570)
  [`ruff`] Add fix safety section (`RUF028`) (#17722)
  [syntax-errors] Detect single starred expression assignment `x = *y` (#17624)
  py-fuzzer: fix minimization logic when `--only-new-bugs` is passed (#17739)
  Fix example syntax for pydocstyle ignore_var_parameters option (#17740)
  [red-knot] Update salsa to prevent panic in custom panic-handler (#17742)
  [red-knot] Ban direct instantiation of generic protocols as well as non-generic ones (#17741)
  [red-knot] Lookup of `__new__` (#17733)
  [red-knot] Check decorator consistency on overloads (#17684)
  [`flake8-use-pathlib`] Avoid suggesting `Path.iterdir()` for `os.listdir` with file descriptor (`PTH208`) (#17715)
  [red-knot] Check overloads without an implementation (#17681)
  Expand Semantic Syntax Coverage (#17725)
  [red-knot] Check for invalid overload usages (#17609)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood left review comments

@carljm carljm carljm approved these changes

@sharkdp sharkdp Awaiting requested review from sharkdp

@dcreager dcreager Awaiting requested review from dcreager

Assignees

No one assigned

Labels

diagnostics Related to reporting of diagnostics. ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dhruvmanila @carljm @AlexWaygood