diff --git a/crates/ruff/resources/test/fixtures/flake8_errmsg/EM.py b/crates/ruff/resources/test/fixtures/flake8_errmsg/EM.py index 144722a60a..1311bbc675 100644 --- a/crates/ruff/resources/test/fixtures/flake8_errmsg/EM.py +++ b/crates/ruff/resources/test/fixtures/flake8_errmsg/EM.py @@ -21,3 +21,36 @@ def f_c(): def f_ok(): msg = "hello" raise RuntimeError(msg) + + +def f_unfixable(): + msg = "hello" + raise RuntimeError("This is an example exception") + + +def f_msg_in_nested_scope(): + def nested(): + msg = "hello" + + raise RuntimeError("This is an example exception") + + +def f_msg_in_parent_scope(): + msg = "hello" + + def nested(): + raise RuntimeError("This is an example exception") + + +def f_fix_indentation_check(foo): + if foo: + raise RuntimeError("This is an example exception") + else: + if foo == "foo": + raise RuntimeError(f"This is an exception: {foo}") + raise RuntimeError("This is an exception: {}".format(foo)) + + +# Report these, but don't fix them +if foo: raise RuntimeError("This is an example exception") +if foo: x = 1; raise RuntimeError("This is an example exception") diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index b3f850d67d..0a20f2a566 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -1512,7 +1512,7 @@ where Rule::DotFormatInException, ]) { if let Some(exc) = exc { - flake8_errmsg::rules::string_in_exception(self, exc); + flake8_errmsg::rules::string_in_exception(self, stmt, exc); } } if self.settings.rules.enabled(Rule::OSErrorAlias) { diff --git a/crates/ruff/src/rules/flake8_errmsg/rules.rs b/crates/ruff/src/rules/flake8_errmsg/rules.rs index 0a1d16a710..ac30f0d7c0 100644 --- a/crates/ruff/src/rules/flake8_errmsg/rules.rs +++ b/crates/ruff/src/rules/flake8_errmsg/rules.rs @@ -1,10 +1,13 @@ -use rustpython_parser::ast::{Constant, Expr, ExprKind}; +use rustpython_parser::ast::{Constant, Expr, ExprContext, ExprKind, Stmt, StmtKind}; -use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::{create_expr, create_stmt, unparse_stmt}; +use ruff_python_ast::source_code::Stylist; +use ruff_python_ast::whitespace; use crate::checkers::ast::Checker; -use crate::registry::Rule; +use crate::registry::{AsRule, Rule}; /// ## What it does /// Checks for the use of string literals in exception constructors. @@ -44,13 +47,22 @@ use crate::registry::Rule; /// RuntimeError: 'Some value' is incorrect /// ``` #[violation] -pub struct RawStringInException; +pub struct RawStringInException { + pub fixable: bool, +} impl Violation for RawStringInException { + const AUTOFIX: AutofixKind = AutofixKind::Sometimes; + #[derive_message_formats] fn message(&self) -> String { format!("Exception must not use a string literal, assign to variable first") } + + fn autofix_title_formatter(&self) -> Option String> { + self.fixable + .then_some(|_| format!("Assign to variable; remove string literal")) + } } /// ## What it does @@ -92,13 +104,22 @@ impl Violation for RawStringInException { /// RuntimeError: 'Some value' is incorrect /// ``` #[violation] -pub struct FStringInException; +pub struct FStringInException { + pub fixable: bool, +} impl Violation for FStringInException { + const AUTOFIX: AutofixKind = AutofixKind::Sometimes; + #[derive_message_formats] fn message(&self) -> String { format!("Exception must not use an f-string literal, assign to variable first") } + + fn autofix_title_formatter(&self) -> Option String> { + self.fixable + .then_some(|_| format!("Assign to variable; remove f-string literal")) + } } /// ## What it does @@ -142,17 +163,62 @@ impl Violation for FStringInException { /// RuntimeError: 'Some value' is incorrect /// ``` #[violation] -pub struct DotFormatInException; +pub struct DotFormatInException { + pub fixable: bool, +} impl Violation for DotFormatInException { + const AUTOFIX: AutofixKind = AutofixKind::Sometimes; + #[derive_message_formats] fn message(&self) -> String { format!("Exception must not use a `.format()` string directly, assign to variable first") } + + fn autofix_title_formatter(&self) -> Option String> { + self.fixable + .then_some(|_| format!("Assign to variable; remove `.format()` string")) + } +} + +/// Generate the [`Fix`] for EM001, EM002, and EM003 violations. +/// +/// This assumes that the violation is fixable and that the patch should +/// be generated. The exception argument should be either a string literal, +/// an f-string, or a `.format` string. +/// +/// The fix includes two edits: +/// 1. Insert the exception argument into a variable assignment before the +/// `raise` statement. The variable name is `msg`. +/// 2. Replace the exception argument with the variable name. +fn generate_fix(stylist: &Stylist, stmt: &Stmt, exc_arg: &Expr, indentation: &str) -> Fix { + let assignment = unparse_stmt( + &create_stmt(StmtKind::Assign { + targets: vec![create_expr(ExprKind::Name { + id: String::from("msg"), + ctx: ExprContext::Store, + })], + value: Box::new(exc_arg.clone()), + type_comment: None, + }), + stylist, + ); + Fix::from_iter([ + Edit::insertion( + format!( + "{}{}{}", + assignment, + stylist.line_ending().as_str(), + indentation, + ), + stmt.start(), + ), + Edit::range_replacement(String::from("msg"), exc_arg.range()), + ]) } /// EM101, EM102, EM103 -pub fn string_in_exception(checker: &mut Checker, exc: &Expr) { +pub fn string_in_exception(checker: &mut Checker, stmt: &Stmt, exc: &Expr) { if let ExprKind::Call { args, .. } = &exc.node { if let Some(first) = args.first() { match &first.node { @@ -163,18 +229,63 @@ pub fn string_in_exception(checker: &mut Checker, exc: &Expr) { } => { if checker.settings.rules.enabled(Rule::RawStringInException) { if string.len() > checker.settings.flake8_errmsg.max_string_length { - checker - .diagnostics - .push(Diagnostic::new(RawStringInException, first.range())); + let indentation = whitespace::indentation(checker.locator, stmt) + .and_then(|indentation| { + if checker.ctx.find_binding("msg").is_none() { + Some(indentation) + } else { + None + } + }); + let mut diagnostic = Diagnostic::new( + RawStringInException { + fixable: indentation.is_some(), + }, + first.range(), + ); + if let Some(indentation) = indentation { + if checker.patch(diagnostic.kind.rule()) { + diagnostic.set_fix(generate_fix( + checker.stylist, + stmt, + first, + indentation, + )); + } + } + checker.diagnostics.push(diagnostic); } } } // Check for f-strings ExprKind::JoinedStr { .. } => { if checker.settings.rules.enabled(Rule::FStringInException) { - checker - .diagnostics - .push(Diagnostic::new(FStringInException, first.range())); + let indentation = whitespace::indentation(checker.locator, stmt).and_then( + |indentation| { + if checker.ctx.find_binding("msg").is_none() { + Some(indentation) + } else { + None + } + }, + ); + let mut diagnostic = Diagnostic::new( + FStringInException { + fixable: indentation.is_some(), + }, + first.range(), + ); + if let Some(indentation) = indentation { + if checker.patch(diagnostic.kind.rule()) { + diagnostic.set_fix(generate_fix( + checker.stylist, + stmt, + first, + indentation, + )); + } + } + checker.diagnostics.push(diagnostic); } } // Check for .format() calls @@ -182,9 +293,31 @@ pub fn string_in_exception(checker: &mut Checker, exc: &Expr) { if checker.settings.rules.enabled(Rule::DotFormatInException) { if let ExprKind::Attribute { value, attr, .. } = &func.node { if attr == "format" && matches!(value.node, ExprKind::Constant { .. }) { - checker - .diagnostics - .push(Diagnostic::new(DotFormatInException, first.range())); + let indentation = whitespace::indentation(checker.locator, stmt) + .and_then(|indentation| { + if checker.ctx.find_binding("msg").is_none() { + Some(indentation) + } else { + None + } + }); + let mut diagnostic = Diagnostic::new( + DotFormatInException { + fixable: indentation.is_some(), + }, + first.range(), + ); + if let Some(indentation) = indentation { + if checker.patch(diagnostic.kind.rule()) { + diagnostic.set_fix(generate_fix( + checker.stylist, + stmt, + first, + indentation, + )); + } + } + checker.diagnostics.push(diagnostic); } } } diff --git a/crates/ruff/src/rules/flake8_errmsg/snapshots/ruff__rules__flake8_errmsg__tests__custom.snap b/crates/ruff/src/rules/flake8_errmsg/snapshots/ruff__rules__flake8_errmsg__tests__custom.snap index 9768f244bd..73074676c1 100644 --- a/crates/ruff/src/rules/flake8_errmsg/snapshots/ruff__rules__flake8_errmsg__tests__custom.snap +++ b/crates/ruff/src/rules/flake8_errmsg/snapshots/ruff__rules__flake8_errmsg__tests__custom.snap @@ -1,26 +1,176 @@ --- source: crates/ruff/src/rules/flake8_errmsg/mod.rs --- -EM.py:5:24: EM101 Exception must not use a string literal, assign to variable first +EM.py:5:24: EM101 [*] Exception must not use a string literal, assign to variable first | 5 | def f_a(): 6 | raise RuntimeError("This is an example exception") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 | + = help: Assign to variable; remove string literal -EM.py:14:24: EM102 Exception must not use an f-string literal, assign to variable first +ℹ Suggested fix +2 2 | +3 3 | +4 4 | def f_a(): +5 |- raise RuntimeError("This is an example exception") + 5 |+ msg = "This is an example exception" + 6 |+ raise RuntimeError(msg) +6 7 | +7 8 | +8 9 | def f_a_short(): + +EM.py:14:24: EM102 [*] Exception must not use an f-string literal, assign to variable first | 14 | def f_b(): 15 | example = "example" 16 | raise RuntimeError(f"This is an {example} exception") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM102 | + = help: Assign to variable; remove f-string literal -EM.py:18:24: EM103 Exception must not use a `.format()` string directly, assign to variable first +ℹ Suggested fix +11 11 | +12 12 | def f_b(): +13 13 | example = "example" +14 |- raise RuntimeError(f"This is an {example} exception") + 14 |+ msg = f"This is an {example} exception" + 15 |+ raise RuntimeError(msg) +15 16 | +16 17 | +17 18 | def f_c(): + +EM.py:18:24: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first | 18 | def f_c(): 19 | raise RuntimeError("This is an {example} exception".format(example="example")) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM103 | + = help: Assign to variable; remove `.format()` string + +ℹ Suggested fix +15 15 | +16 16 | +17 17 | def f_c(): +18 |- raise RuntimeError("This is an {example} exception".format(example="example")) + 18 |+ msg = "This is an {example} exception".format(example="example") + 19 |+ raise RuntimeError(msg) +19 20 | +20 21 | +21 22 | def f_ok(): + +EM.py:28:24: EM101 Exception must not use a string literal, assign to variable first + | +28 | def f_unfixable(): +29 | msg = "hello" +30 | raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 + | + +EM.py:35:24: EM101 [*] Exception must not use a string literal, assign to variable first + | +35 | msg = "hello" +36 | +37 | raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 + | + = help: Assign to variable; remove string literal + +ℹ Suggested fix +32 32 | def nested(): +33 33 | msg = "hello" +34 34 | +35 |- raise RuntimeError("This is an example exception") + 35 |+ msg = "This is an example exception" + 36 |+ raise RuntimeError(msg) +36 37 | +37 38 | +38 39 | def f_msg_in_parent_scope(): + +EM.py:42:28: EM101 Exception must not use a string literal, assign to variable first + | +42 | def nested(): +43 | raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 + | + +EM.py:47:28: EM101 [*] Exception must not use a string literal, assign to variable first + | +47 | def f_fix_indentation_check(foo): +48 | if foo: +49 | raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 +50 | else: +51 | if foo == "foo": + | + = help: Assign to variable; remove string literal + +ℹ Suggested fix +44 44 | +45 45 | def f_fix_indentation_check(foo): +46 46 | if foo: +47 |- raise RuntimeError("This is an example exception") + 47 |+ msg = "This is an example exception" + 48 |+ raise RuntimeError(msg) +48 49 | else: +49 50 | if foo == "foo": +50 51 | raise RuntimeError(f"This is an exception: {foo}") + +EM.py:50:32: EM102 [*] Exception must not use an f-string literal, assign to variable first + | +50 | else: +51 | if foo == "foo": +52 | raise RuntimeError(f"This is an exception: {foo}") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM102 +53 | raise RuntimeError("This is an exception: {}".format(foo)) + | + = help: Assign to variable; remove f-string literal + +ℹ Suggested fix +47 47 | raise RuntimeError("This is an example exception") +48 48 | else: +49 49 | if foo == "foo": +50 |- raise RuntimeError(f"This is an exception: {foo}") + 50 |+ msg = f"This is an exception: {foo}" + 51 |+ raise RuntimeError(msg) +51 52 | raise RuntimeError("This is an exception: {}".format(foo)) +52 53 | +53 54 | + +EM.py:51:24: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first + | +51 | if foo == "foo": +52 | raise RuntimeError(f"This is an exception: {foo}") +53 | raise RuntimeError("This is an exception: {}".format(foo)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM103 + | + = help: Assign to variable; remove `.format()` string + +ℹ Suggested fix +48 48 | else: +49 49 | if foo == "foo": +50 50 | raise RuntimeError(f"This is an exception: {foo}") +51 |- raise RuntimeError("This is an exception: {}".format(foo)) + 51 |+ msg = "This is an exception: {}".format(foo) + 52 |+ raise RuntimeError(msg) +52 53 | +53 54 | +54 55 | # Report these, but don't fix them + +EM.py:55:28: EM101 Exception must not use a string literal, assign to variable first + | +55 | # Report these, but don't fix them +56 | if foo: raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 +57 | if foo: x = 1; raise RuntimeError("This is an example exception") + | + +EM.py:56:35: EM101 Exception must not use a string literal, assign to variable first + | +56 | # Report these, but don't fix them +57 | if foo: raise RuntimeError("This is an example exception") +58 | if foo: x = 1; raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 + | diff --git a/crates/ruff/src/rules/flake8_errmsg/snapshots/ruff__rules__flake8_errmsg__tests__defaults.snap b/crates/ruff/src/rules/flake8_errmsg/snapshots/ruff__rules__flake8_errmsg__tests__defaults.snap index b1adb139c5..87af51141d 100644 --- a/crates/ruff/src/rules/flake8_errmsg/snapshots/ruff__rules__flake8_errmsg__tests__defaults.snap +++ b/crates/ruff/src/rules/flake8_errmsg/snapshots/ruff__rules__flake8_errmsg__tests__defaults.snap @@ -1,33 +1,195 @@ --- source: crates/ruff/src/rules/flake8_errmsg/mod.rs --- -EM.py:5:24: EM101 Exception must not use a string literal, assign to variable first +EM.py:5:24: EM101 [*] Exception must not use a string literal, assign to variable first | 5 | def f_a(): 6 | raise RuntimeError("This is an example exception") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 | + = help: Assign to variable; remove string literal -EM.py:9:24: EM101 Exception must not use a string literal, assign to variable first +ℹ Suggested fix +2 2 | +3 3 | +4 4 | def f_a(): +5 |- raise RuntimeError("This is an example exception") + 5 |+ msg = "This is an example exception" + 6 |+ raise RuntimeError(msg) +6 7 | +7 8 | +8 9 | def f_a_short(): + +EM.py:9:24: EM101 [*] Exception must not use a string literal, assign to variable first | 9 | def f_a_short(): 10 | raise RuntimeError("Error") | ^^^^^^^ EM101 | + = help: Assign to variable; remove string literal -EM.py:14:24: EM102 Exception must not use an f-string literal, assign to variable first +ℹ Suggested fix +6 6 | +7 7 | +8 8 | def f_a_short(): +9 |- raise RuntimeError("Error") + 9 |+ msg = "Error" + 10 |+ raise RuntimeError(msg) +10 11 | +11 12 | +12 13 | def f_b(): + +EM.py:14:24: EM102 [*] Exception must not use an f-string literal, assign to variable first | 14 | def f_b(): 15 | example = "example" 16 | raise RuntimeError(f"This is an {example} exception") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM102 | + = help: Assign to variable; remove f-string literal -EM.py:18:24: EM103 Exception must not use a `.format()` string directly, assign to variable first +ℹ Suggested fix +11 11 | +12 12 | def f_b(): +13 13 | example = "example" +14 |- raise RuntimeError(f"This is an {example} exception") + 14 |+ msg = f"This is an {example} exception" + 15 |+ raise RuntimeError(msg) +15 16 | +16 17 | +17 18 | def f_c(): + +EM.py:18:24: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first | 18 | def f_c(): 19 | raise RuntimeError("This is an {example} exception".format(example="example")) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM103 | + = help: Assign to variable; remove `.format()` string + +ℹ Suggested fix +15 15 | +16 16 | +17 17 | def f_c(): +18 |- raise RuntimeError("This is an {example} exception".format(example="example")) + 18 |+ msg = "This is an {example} exception".format(example="example") + 19 |+ raise RuntimeError(msg) +19 20 | +20 21 | +21 22 | def f_ok(): + +EM.py:28:24: EM101 Exception must not use a string literal, assign to variable first + | +28 | def f_unfixable(): +29 | msg = "hello" +30 | raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 + | + +EM.py:35:24: EM101 [*] Exception must not use a string literal, assign to variable first + | +35 | msg = "hello" +36 | +37 | raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 + | + = help: Assign to variable; remove string literal + +ℹ Suggested fix +32 32 | def nested(): +33 33 | msg = "hello" +34 34 | +35 |- raise RuntimeError("This is an example exception") + 35 |+ msg = "This is an example exception" + 36 |+ raise RuntimeError(msg) +36 37 | +37 38 | +38 39 | def f_msg_in_parent_scope(): + +EM.py:42:28: EM101 Exception must not use a string literal, assign to variable first + | +42 | def nested(): +43 | raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 + | + +EM.py:47:28: EM101 [*] Exception must not use a string literal, assign to variable first + | +47 | def f_fix_indentation_check(foo): +48 | if foo: +49 | raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 +50 | else: +51 | if foo == "foo": + | + = help: Assign to variable; remove string literal + +ℹ Suggested fix +44 44 | +45 45 | def f_fix_indentation_check(foo): +46 46 | if foo: +47 |- raise RuntimeError("This is an example exception") + 47 |+ msg = "This is an example exception" + 48 |+ raise RuntimeError(msg) +48 49 | else: +49 50 | if foo == "foo": +50 51 | raise RuntimeError(f"This is an exception: {foo}") + +EM.py:50:32: EM102 [*] Exception must not use an f-string literal, assign to variable first + | +50 | else: +51 | if foo == "foo": +52 | raise RuntimeError(f"This is an exception: {foo}") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM102 +53 | raise RuntimeError("This is an exception: {}".format(foo)) + | + = help: Assign to variable; remove f-string literal + +ℹ Suggested fix +47 47 | raise RuntimeError("This is an example exception") +48 48 | else: +49 49 | if foo == "foo": +50 |- raise RuntimeError(f"This is an exception: {foo}") + 50 |+ msg = f"This is an exception: {foo}" + 51 |+ raise RuntimeError(msg) +51 52 | raise RuntimeError("This is an exception: {}".format(foo)) +52 53 | +53 54 | + +EM.py:51:24: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first + | +51 | if foo == "foo": +52 | raise RuntimeError(f"This is an exception: {foo}") +53 | raise RuntimeError("This is an exception: {}".format(foo)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM103 + | + = help: Assign to variable; remove `.format()` string + +ℹ Suggested fix +48 48 | else: +49 49 | if foo == "foo": +50 50 | raise RuntimeError(f"This is an exception: {foo}") +51 |- raise RuntimeError("This is an exception: {}".format(foo)) + 51 |+ msg = "This is an exception: {}".format(foo) + 52 |+ raise RuntimeError(msg) +52 53 | +53 54 | +54 55 | # Report these, but don't fix them + +EM.py:55:28: EM101 Exception must not use a string literal, assign to variable first + | +55 | # Report these, but don't fix them +56 | if foo: raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 +57 | if foo: x = 1; raise RuntimeError("This is an example exception") + | + +EM.py:56:35: EM101 Exception must not use a string literal, assign to variable first + | +56 | # Report these, but don't fix them +57 | if foo: raise RuntimeError("This is an example exception") +58 | if foo: x = 1; raise RuntimeError("This is an example exception") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM101 + |