[ty] Add invalid-enum-member-annotation lint rule#23648
Merged
AlexWaygood merged 13 commits intomainfrom Mar 2, 2026
Merged
Conversation
Typing conformance results improved 🎉The percentage of diagnostics emitted that were expected errors increased from 85.90% to 85.92%. The percentage of expected errors that received a diagnostic increased from 78.04% to 78.13%. Summary
True positives addedDetails
|
invalid-enum-member-annotation lint ruleinvalid-enum-member-annotation lint rule
|
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
1 | 0 | 0 |
| Total | 1 | 0 | 0 |
AlexWaygood
commented
Mar 1, 2026
Comment on lines
+9682
to
+9694
| if let Some(name_expr) = target.as_name_expr() | ||
| && !name_expr.id.starts_with("__") | ||
| && !matches!(name_expr.id.as_str(), "_ignore_" | "_value_" | "_name_") | ||
| // Not bare Final (bare Final is allowed on enum members) | ||
| && !(declared.qualifiers.contains(TypeQualifiers::FINAL) | ||
| && matches!(declared.inner_type(), Type::Dynamic(DynamicType::Unknown))) | ||
| // Value type would be an enum member at runtime (exclude callables, | ||
| // which are never members) | ||
| && !inferred_ty.is_subtype_of( | ||
| self.db(), | ||
| Type::Callable(CallableType::unknown(self.db())) | ||
| .top_materialization(self.db()), | ||
| ) |
Member
Author
There was a problem hiding this comment.
this all feels a little duplicative of the logic we have elsewhere for deciding whether an assignment in an enum class is an enum member to begin with, but I think it's okay
Member
There was a problem hiding this comment.
Yeah, it's just different enough in form that I agree that we shouldn't bend over backwards to eliminate the copy/pasta. My only thought is that maybe this logic should live as a top-level helper method in types/enums.rs, so that all of this related logic is defined close to each other in the ty source? Then this would become a simple check:
if !allowed_enum_member_annotation(name_expr, inferred_ty, current_scope)
But I'm also fine leaving as-is
ff695ba to
8810a82
Compare
dcreager
approved these changes
Mar 2, 2026
Comment on lines
+119
to
+120
| DOG: int = 2 # error: [invalid-enum-member-annotation] "Type annotation on enum member `DOG` is not allowed" | ||
| BIRD: str = "bird" # error: [invalid-enum-member-annotation] |
Member
There was a problem hiding this comment.
nit: I would suggest either checking both error messages, or neither. (Probably preferably neither, unless you switch to using a snapshot test for that)
Member
Author
There was a problem hiding this comment.
Hmm, I disagree -- I think it's useful to assert the error message once, and a snapshot test feels overkill for that unless you have detailed secondary annotations etc. But asserting it more than once makes it tedious to update the mdtests in the future if you ever change the error message.
Comment on lines
+199
to
+202
| _ignore_ = "A B" | ||
| A: int = 42 # OK: `A` is listed in `_ignore_` | ||
| B: str = "hello" # OK: `B` is listed in `_ignore_` | ||
| C: int = 3 # error: [invalid-enum-member-annotation] |
Comment on lines
+9682
to
+9694
| if let Some(name_expr) = target.as_name_expr() | ||
| && !name_expr.id.starts_with("__") | ||
| && !matches!(name_expr.id.as_str(), "_ignore_" | "_value_" | "_name_") | ||
| // Not bare Final (bare Final is allowed on enum members) | ||
| && !(declared.qualifiers.contains(TypeQualifiers::FINAL) | ||
| && matches!(declared.inner_type(), Type::Dynamic(DynamicType::Unknown))) | ||
| // Value type would be an enum member at runtime (exclude callables, | ||
| // which are never members) | ||
| && !inferred_ty.is_subtype_of( | ||
| self.db(), | ||
| Type::Callable(CallableType::unknown(self.db())) | ||
| .top_materialization(self.db()), | ||
| ) |
Member
There was a problem hiding this comment.
Yeah, it's just different enough in form that I agree that we shouldn't bend over backwards to eliminate the copy/pasta. My only thought is that maybe this logic should live as a top-level helper method in types/enums.rs, so that all of this related logic is defined close to each other in the ty source? Then this would become a simple check:
if !allowed_enum_member_annotation(name_expr, inferred_ty, current_scope)
But I'm also fine leaving as-is
…(`RUF055`) (#23634) ## Summary Fixes #23629. `re.split("", s)` is flagged by RUF055 and auto-fixed to `s.split("")`, but `str.split("")` raises `ValueError: empty separator` while `re.split("", s)` succeeds (returning `["", "a", "b", "c", ""]`). The same applies to bytes (`rb""`). This adds a guard to skip the diagnostic when the separator pattern is an empty string or bytes literal specifically for `re.split` calls. Other `re` functions (`sub`, `match`, `search`, `fullmatch`) are not affected — their `str` equivalents all handle empty strings equivalently. ## Test Plan Added test cases for empty string and bytes patterns in `RUF055_0.py` and `RUF055_3.py`. Verified that no diagnostics are emitted for these cases and all existing RUF055 snapshot tests continue to pass: ``` cargo test -p ruff_linter -- "preview_rules::rule_unnecessaryregularexpression" test result: ok. 4 passed; 0 failed; 0 ignored ```
- Shorten second paragraph of "Why is this bad?" docs - Change lint status from 0.0.16 to 0.0.20 - Change default severity from Error to Warn (no runtime error) - Merge report_lint guard into the let chain above it https://claude.ai/code/session_01MzfceBgafJtwtAwLXarq4o
The previous `matches!(inferred_ty, Type::Callable(_) | Type::FunctionLiteral(_))` was fragile and missed many callable types (bound methods, class instances with `__call__`, etc.). Replace with a proper subtype check against the top materialization of `Callable[..., object]`. Also add test coverage for the callable exclusion. https://claude.ai/code/session_01MzfceBgafJtwtAwLXarq4o
Code blocks within the same subsection are implicitly merged before ty runs on them, so repeated `from enum import Enum` imports are unnecessary. https://claude.ai/code/session_01MzfceBgafJtwtAwLXarq4o
By placing the `[environment] python-version = "3.11"` block at the top of "### Annotated enum members", all Python snippets merge into a single group, so we only need one import block for the entire section. https://claude.ai/code/session_01MzfceBgafJtwtAwLXarq4o
The typing spec does not say that using `member` as a type annotation on an enum member (e.g. `DOG: member = 2`) should be exempt from the invalid-enum-member-annotation diagnostic, and no other type checker exempts it. Remove the special-case logic and the associated test. https://claude.ai/code/session_01MzfceBgafJtwtAwLXarq4o
Enums with `_ignore_ = "A B"` should not emit the diagnostic for names A and B, since those are explicitly excluded from membership. Extract `enum_ignored_names` as a standalone function so it can be queried independently of `enum_metadata` (which returns `None` when an enum has no remaining members after filtering). https://claude.ai/code/session_01MzfceBgafJtwtAwLXarq4o
8810a82 to
c9efb49
Compare
carljm
added a commit
that referenced
this pull request
Mar 2, 2026
* main: (30 commits) [ty] Introduce `types::bool`, `types::context_manager` and `types::iteration` (#23681) [ty] Move `KnownInstanceType`, and related types, to a new `known_instance.rs` submodule (#23680) [ty] Add `invalid-enum-member-annotation` lint rule (#23648) [`ruff`] Fix false positive for `re.split` with empty string pattern (`RUF055`) (#23634) [ty] Move `UnionType` and `IntersectionType` to a new `types::set_theoretic` submodule (#23678) [ty] Add unbound type variable detection in annotations (#23641) [ty] Remove `specialize_constrained` from constraint set module (#23677) [ty] Add partial support and validation for `Unpack` when used with tuple types (#23651) Update prek dependencies (#23661) [ty] make `StaticClassLiteral::explicit_bases` converge better in cycles (#23601) Improvements to CLAUDE.md (#23633) [ty] Move subscript logic out of `builder.rs` (#23653) Update Artifact GitHub Actions dependencies (#23676) Update actions/attest-build-provenance to 4.1.0 (#23654) Update Rust crate clearscreen to v4.0.5 (#23664) fix renovate `actions/*-artifact` updates (#23675) Update Rust crate clap to v4.5.60 (#23663) Update Rust crate unicode-ident to v1.0.24 (#23668) Update Rust crate anyhow to v1.0.102 (#23662) Update Rust crate pyproject-toml to v0.13.7 (#23666) ...
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
4 participants
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.
Summary
This PR fixes the last remaining conformance failure on
enums_members.pyin the conformance suite.Implements a new lint rule
invalid-enum-member-annotationthat detects type annotations on enum members. According to the typing spec, enum members should not have explicit type annotations, as the actual runtime type is the enum class itself, not the annotated type.The rule:
DOG: int = 2in anEnumclass)Finalannotations (which don't specify a type)_value_and_ignore_Test Plan
mdtests