diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP042.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP042.py new file mode 100644 index 00000000000000..db7a0974579571 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP042.py @@ -0,0 +1,15 @@ +from enum import Enum + +class A(str, Enum): + ... + +class B(Enum, str): + ... + +class D(int, str, Enum): + ... + +class E(str, int, Enum): + ... + +# TODO: add more cases diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 7e8293172916f5..cc027dc208dc0c 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -406,6 +406,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::UselessObjectInheritance) { pyupgrade::rules::useless_object_inheritance(checker, class_def); } + if checker.enabled(Rule::ReplaceStrEnum) { + if checker.settings.target_version >= PythonVersion::Py311 { + pyupgrade::rules::replace_str_enum(checker, class_def); + } + } if checker.enabled(Rule::UnnecessaryClassParentheses) { pyupgrade::rules::unnecessary_class_parentheses(checker, class_def); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index a6db70d9433022..210d7b56827f2d 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -546,6 +546,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pyupgrade, "039") => (RuleGroup::Stable, rules::pyupgrade::rules::UnnecessaryClassParentheses), (Pyupgrade, "040") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP695TypeAlias), (Pyupgrade, "041") => (RuleGroup::Stable, rules::pyupgrade::rules::TimeoutErrorAlias), + (Pyupgrade, "042") => (RuleGroup::Stable, rules::pyupgrade::rules::ReplaceStrEnum), // pydocstyle (Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule), diff --git a/crates/ruff_linter/src/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs index 0a70cc4e2327c6..4bba6aec099c82 100644 --- a/crates/ruff_linter/src/fix/edits.rs +++ b/crates/ruff_linter/src/fix/edits.rs @@ -136,6 +136,17 @@ pub(crate) enum Parentheses { /// /// Supports the removal of parentheses when this is the only (kw)arg left. /// For this behavior, set `remove_parentheses` to `true`. +/// +/// NOTE: does not support multiple consecutive calls within a single [`Fix`]. +/// Because the first call deletes one arg, one arg remains (as expected), +/// the second call will return an [`Edit`] with positions on a locator which +/// has 2 args, so the second call will corrupt the source (likely, comma +/// and a space will remain). +/// Eventually this causes ruff to fail on this assertion: +/// ``` +/// panicked at /ruff/crates/ruff_text_size/src/range.rs:48:9: +/// assertion failed: start.raw <= end.raw +/// ``` pub(crate) fn remove_argument( argument: &T, arguments: &Arguments, diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index 10fae94973062a..427b448d765b1b 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -61,6 +61,7 @@ mod tests { #[test_case(Rule::ReplaceUniversalNewlines, Path::new("UP021.py"))] #[test_case(Rule::SuperCallWithParameters, Path::new("UP008.py"))] #[test_case(Rule::TimeoutErrorAlias, Path::new("UP041.py"))] + #[test_case(Rule::ReplaceStrEnum, Path::new("UP042.py"))] #[test_case(Rule::TypeOfPrimitive, Path::new("UP003.py"))] #[test_case(Rule::TypingTextStrAlias, Path::new("UP019.py"))] #[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_0.py"))] diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs index aa6a60732fbed1..3b7928f6e90202 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs @@ -18,6 +18,7 @@ pub(crate) use printf_string_formatting::*; pub(crate) use quoted_annotation::*; pub(crate) use redundant_open_modes::*; pub(crate) use replace_stdout_stderr::*; +pub(crate) use replace_str_enum::*; pub(crate) use replace_universal_newlines::*; pub(crate) use super_call_with_parameters::*; pub(crate) use timeout_error_alias::*; @@ -58,6 +59,7 @@ mod printf_string_formatting; mod quoted_annotation; mod redundant_open_modes; mod replace_stdout_stderr; +mod replace_str_enum; mod replace_universal_newlines; mod super_call_with_parameters; mod timeout_error_alias; diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs new file mode 100644 index 00000000000000..d506db31882d28 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/replace_str_enum.rs @@ -0,0 +1,123 @@ +use crate::checkers::ast::Checker; +use crate::importer::ImportRequest; +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast}; +use ruff_text_size::Ranged; + +/// ## What it does +/// Checks for classes that inherit from both `str` and `enum.Enum`. +/// +/// ## Why is this bad? +/// Since Python 3.11, `enum.StrEnum` exists and is preferred over +/// inheriting from `str` and `enum.Enum`. +/// +/// ## Example +/// ```python +/// class Foo(str, enum.Enum): +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// class Foo(enum.StrEnum): +/// ... +/// ``` +/// +/// ## References +/// - [enum.StrEnum](https://docs.python.org/3/library/enum.html#enum.StrEnum) + +#[violation] +pub struct ReplaceStrEnum { + name: String, + args_len: usize, +} + +impl Violation for ReplaceStrEnum { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + let ReplaceStrEnum { name, .. } = self; + format!( + "Class {name} inherits from both `str` and `enum.Enum`. Prefer `enum.StrEnum` instead." + ) + } + + fn fix_title(&self) -> Option { + let ReplaceStrEnum { args_len, .. } = self; + + if *args_len == 2 { + Some("Replace `str` and `enum.Enum` with `enum.StrEnum`".to_string()) + } else { + None + } + } +} + +/// UP042 +pub(crate) fn replace_str_enum(checker: &mut Checker, class_def: &ast::StmtClassDef) { + let Some(arguments) = class_def.arguments.as_deref() else { + // class does not inherit anything, exit early + return; + }; + + let mut inherits_str = false; + let mut inherits_enum = false; + for base in arguments.args.iter() { + if let Some(qualified_name) = checker.semantic().resolve_qualified_name(base) { + if matches!(qualified_name.segments(), ["", "str"]) { + inherits_str = true; + } else if matches!(qualified_name.segments(), ["enum", "Enum"]) { + inherits_enum = true; + } + } + + if inherits_str && inherits_enum { + // no need to check other inherited classes, we found both str & enum.Enum + break; + } + } + + if !inherits_str || !inherits_enum { + // exit early if class does not inherit both str and enum.Enum + return; + }; + + let mut diagnostic = Diagnostic::new( + ReplaceStrEnum { + name: class_def.name.to_string(), + args_len: arguments.len(), + }, + class_def.range(), + ); + + if arguments.len() == 2 { + // a fix is available only for classes that inherit exactly 2 arguments: str, Enum, + // because `remove_argument` cannot be called multiple times consecutively... + // for classes that inherit str, Enum and something else, generate a warning. + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("enum", "StrEnum"), + class_def.start(), + checker.semantic(), + )?; + + // `binding` here is `StrEnum`. + + // class inherits exactly 2 arguments. + // replace all `(str, Enum)` arguments with `(StrEnum)`. + let fix = Fix::unsafe_edits( + import_edit, + [Edit::range_replacement( + format!("({binding})"), + arguments.range(), + )], + ); + + Ok(fix) + }); + } + + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP042.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP042.py.snap new file mode 100644 index 00000000000000..b40bd414b2d8e3 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP042.py.snap @@ -0,0 +1,71 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +UP042.py:3:1: UP042 [*] Class A inherits from both `str` and `enum.Enum`. Prefer `enum.StrEnum` instead. + | +1 | from enum import Enum +2 | +3 | / class A(str, Enum): +4 | | ... + | |_______^ UP042 +5 | +6 | class B(Enum, str): + | + = help: Replace `str` and `enum.Enum` with `enum.StrEnum` + +ℹ Unsafe fix +1 |-from enum import Enum + 1 |+from enum import Enum, StrEnum +2 2 | +3 |-class A(str, Enum): + 3 |+class A(StrEnum): +4 4 | ... +5 5 | +6 6 | class B(Enum, str): + +UP042.py:6:1: UP042 [*] Class B inherits from both `str` and `enum.Enum`. Prefer `enum.StrEnum` instead. + | +4 | ... +5 | +6 | / class B(Enum, str): +7 | | ... + | |_______^ UP042 +8 | +9 | class D(int, str, Enum): + | + = help: Replace `str` and `enum.Enum` with `enum.StrEnum` + +ℹ Unsafe fix +1 |-from enum import Enum + 1 |+from enum import Enum, StrEnum +2 2 | +3 3 | class A(str, Enum): +4 4 | ... +5 5 | +6 |-class B(Enum, str): + 6 |+class B(StrEnum): +7 7 | ... +8 8 | +9 9 | class D(int, str, Enum): + +UP042.py:9:1: UP042 Class D inherits from both `str` and `enum.Enum`. Prefer `enum.StrEnum` instead. + | + 7 | ... + 8 | + 9 | / class D(int, str, Enum): +10 | | ... + | |_______^ UP042 +11 | +12 | class E(str, int, Enum): + | + +UP042.py:12:1: UP042 Class E inherits from both `str` and `enum.Enum`. Prefer `enum.StrEnum` instead. + | +10 | ... +11 | +12 | / class E(str, int, Enum): +13 | | ... + | |_______^ UP042 +14 | +15 | # TODO: add more cases + | diff --git a/ruff.schema.json b/ruff.schema.json index 1c26154c2d19f0..ec167eb8049fe2 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3864,6 +3864,7 @@ "UP04", "UP040", "UP041", + "UP042", "W", "W1", "W19",