Repository navigation
Conversation
CPython raises `ValueError` when a class lists a name in `__slots__` and also binds that name in its body, for example with an assignment, a method, or a nested class, even when a bare annotation of the same name comes before or after it. Pyrefly does not report this yet. The tests record the current behavior and the cases that must stay accepted: annotations without a value, a subclass that shadows an inherited slot, dynamic `__slots__`, the special names `__dict__`, `__weakref__` and `__qualname__`, and stubs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A class that lists a name in `__slots__` and also binds it in the class body fails at definition time with `ValueError: 'foo' in __slots__ conflicts with class variable`, because the slot descriptor and the class attribute would need the same entry in the class namespace. Pyright and ty report this; Pyrefly did not. When Pyrefly reads a class's own `__slots__` and the names are known, it now looks up each name among the class-body definitions. A name bound by an assignment, a method, a nested class, or an import is reported as `bad-class-definition` on that binding. A bare annotation does not create a class attribute, so it is not reported, and it does not hide a value bound elsewhere in the body. `__dict__`, `__weakref__` and `__qualname__` are skipped, because CPython does not create slot descriptors for them. Stubs are skipped, because they are never executed and stubs for extension modules commonly declare a slot as a property. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
A bare annotation does not create a class attribute, so it cannot conflict with a name in `__slots__`. The check already skipped bare annotations among several definitions of a name, but a single bare annotation in a branch, such as `if flag(): x: int`, is merged into a definition that also covers imports and other bindings, and it was reported. The check now asks the same question of that definition as of each of several definitions: whether its binding is a bare annotation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A metaclass's `__new__` receives the class namespace before the class is created and may remove names from it. Pydantic's `BaseModel` assigns placeholder values to names it lists in `__slots__`, and its metaclass pops them before calling `type.__new__`, so the class is valid; mypy_primer showed these as false positives. The check now runs only when the metaclass is `type` or `ABCMeta`, which create the class from the namespace as written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
This pull request has been imported. If you are a Meta employee, you can view this in D131793863. (Because this pull request was imported automatically, there will not be any future comments.) |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
stroxler
approved these changes
Nov 13, 2026
stroxler
left a comment
stroxler
left a comment
Contributor
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
meta-codesync Bot
pushed a commit
that referenced
this pull request
Nov 13, 2026
Summary: Reviewing #5642 surfaced slot/class-variable declarations inside `TYPE_CHECKING` and a value deleted later in the class body. The review presented these as blockers because CPython accepts the resulting classes, without first establishing the intended static semantics. Typing rules still apply inside `TYPE_CHECKING`; runtime success alone does not establish that a diagnostic is incorrect. Nudge reviews toward consistent static rules and justified support boundaries, rather than requiring a special case for every runtime-admitted corner. This benefits user predictability and avoids accumulating checker complexity. Give the blind reviewers the same principle, ground counterexample expectations in a specification or support contract, and justify blocking severity separately from scores and reviewer agreement. Architectural blockers remain possible when their maintenance impact warrants fixing them now. The example motivates the guidelines; it does not settle those edge cases' semantics. No checker behavior or scoring weights change. ___ landed-with-radar-review Differential Revision: D131832048 fbshipit-source-id: 970adc2f05f68de9ab63bebf09154c32f547fab6
Contributor
|
This pull request has been merged in 3e2fae9. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A class that lists a name in
__slots__and also binds it in the class body fails at class creation withValueError- see (data model).Pyrefly didn't report this.
What is done:
When the names in a class's own
__slots__are known statically, Pyrefly now checks each one against the class body and reportsbad-class-definitionon the first definition that binds a value: an assignment, a method or property, a nested class, or an import.Not reported (matching CPython):
x: int), which doesn't create a class attribute. it also doesn't hide a value bound before or after it, sox = 1followed byx: intis still reported.__dict__,__weakref__and__qualname__are allowed - CPython doesn't create slot descriptors for them__slots__that assigns an inherited slot name.__slots__.Fixes #5641
Test Plan
pyrefly/lib/test/slots.rs- all pass.cargo test -p pyrefly --lib -- --skip deep_phi_chain --skip commands::coverage- all pass.test::cycles::deep_phi_chain_term_exproverflows the stack in a local debug build on main.python3 test.py --no-test: formatting and lint clean, no changes to the conformance output.