Skip to content

Check property decorators stricter #19313

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.

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sterliakov
Copy link
Collaborator

@sterliakov sterliakov commented Jun 18, 2025

Fixes #19312. Fixes #18327.

Only accept @current_prop_name.{setter,deleter} as property-related decorators.

return False
if not isinstance(deco.expr, NameExpr) or deco.expr.name != property_name:
return False
if deco.name not in {"setter", "deleter"}:
Copy link
Collaborator

@brianschubert brianschubert Jun 18, 2025

Choose a reason for hiding this comment

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

Suggested change
if deco.name not in {"setter", "deleter"}:
if deco.name not in {"getter", "setter", "deleter"}:

@x.getter is valid at runtime (albeit not common). We probably don't want to emit an "Only supported top decorators are ..." error for it.

I guess we don't have any tests for it 🤷

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm. This definitely needs a test, but I think rejecting @prop.getter is a reasonable thing to do: it should override the existing getter, supporting it properly won't be trivial, so explicitly saying "we don't support redefined getters" is not that bad (given that such usage should be really rare, 0 primer hits)

Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False negative on setter field of undefined variable Mypy doesn't object to property methods that don't exist
2 participants