[pyupgrade] Replace str,Enum with StrEnum UP042#10713
[pyupgrade] Replace str,Enum with StrEnum UP042#10713charliermarsh merged 6 commits intoastral-sh:mainfrom
pyupgrade] Replace str,Enum with StrEnum UP042#10713Conversation
CodSpeed Performance ReportMerging #10713 will not alter performanceComparing Summary
|
cea7a40 to
3559ba3
Compare
Basically it closes astral-sh#3867
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| UP042 | 1 | 0 | 1 | 0 | 0 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+1 -1 violations, +0 -0 fixes in 1 projects; 43 projects unchanged)
indico/indico (+1 -1 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview
- indico/modules/admin/notices.py:40:1: UP042 Class NoticeSeverity inherits from both `str` and `enum.Enum`. Prefer `enum.StrEnum` instead. + indico/modules/admin/notices.py:40:7: UP042 Class NoticeSeverity inherits from both `str` and `enum.Enum`
Changes by rule (1 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| UP042 | 2 | 1 | 1 | 0 | 0 |
|
Anyone? |
|
I’ll take a look today :) |
|
This is great! I made some changes to the documentation and rule messages just to better align with our style elsewhere. |
| let mut inherits_enum = false; | ||
| for base in arguments.args.iter() { | ||
| if let Some(qualified_name) = checker.semantic().resolve_qualified_name(base) { | ||
| match qualified_name.segments() { |
There was a problem hiding this comment.
I changed this to a match over the segments, rather than an if-else.
| #[derive_message_formats] | ||
| fn message(&self) -> String { | ||
| let ReplaceStrEnum { name, .. } = self; | ||
| format!("Class {name} inherits from both `str` and `enum.Enum`") |
There was a problem hiding this comment.
We prefer to omit the fix in the rule message.
| ReplaceStrEnum { | ||
| name: class_def.name.to_string(), | ||
| }, | ||
| class_def.identifier(), |
There was a problem hiding this comment.
The use of .identifier() here is nice because it confines the range of the diagnostic to the class name, rather than the entire class definition (which would include the class body).
| class D(int, str, Enum): ... | ||
|
|
||
|
|
||
| class E(str, int, Enum): ... |
There was a problem hiding this comment.
(I ran ruff format over the fixture. It's not a hard rule, since fixtures are often intentionally unformatted, but it's nice when we don't rely on the formatting anywhere.)
|
Left a few clarifying comments on changes that I made, but please ask questions if you'd like. |
…#10713) ## Summary Add new rule `pyupgrade - UP042` (I picked next available number). Closes astral-sh#3867 Closes astral-sh#9569 It should warn + provide a fix `class A(str, Enum)` -> `class A(StrEnum)` for py311+. ## Test Plan Added UP042.py test. ## Notes I did not find a way to call `remove_argument` 2 times consecutively, so the automatic fixing works only for classes that inherit exactly `str, Enum` (regardless of the order). I also plan to extend this rule to support IntEnum in next PR.
Disclaimer: I am first time contributor to Ruff, relatively new to Rust. Doing this to learn the language, study how ruff project is structured and to pay back for using my favourite python tool :)
Summary
Add new rule
pyupgrade - UP042(I picked next available number).Closes #3867
Closes #9569
It should warn + provide a fix
class A(str, Enum)->class A(StrEnum)for py311+.Test Plan
Added UP042.py test.
Notes
I did not find a way to call
remove_argument2 times consecutively, so the automatic fixing works only for classes that inherit exactlystr, Enum(regardless of the order).I also plan to extend this rule to support IntEnum in next PR.