From 451acea1c655ce86ca65ecaa27e516f707f6b578 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Mon, 2 Oct 2023 00:25:52 +0530 Subject: [PATCH] Slice source code instead of generating it for `EM` fixes --- .../test/fixtures/flake8_errmsg/EM.py | 31 +++++ .../rules/string_in_exception.rs | 48 ++++---- ...__rules__flake8_errmsg__tests__custom.snap | 82 +++++++++++++ ...rules__flake8_errmsg__tests__defaults.snap | 110 ++++++++++++++++++ 4 files changed, 247 insertions(+), 24 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_errmsg/EM.py b/crates/ruff_linter/resources/test/fixtures/flake8_errmsg/EM.py index 9a94763c365281..50d045bc572914 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_errmsg/EM.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_errmsg/EM.py @@ -58,3 +58,34 @@ def f_fix_indentation_check(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") + + +def f_triple_quoted_string(): + raise RuntimeError(f"""This is an {"example"} exception""") + + +# Generate a violaiton for this, but don't fix it +def f_multi_line_string(): + raise RuntimeError( + "first" + "second" + ) + + +def f_multi_line_string2(): + raise RuntimeError( + "This is an {example} exception".format( + example="example" + ) + ) + + +def f_multi_line_string2(): + raise RuntimeError( + ( + "This is an " + "{example} exception" + ).format( + example="example" + ) + ) diff --git a/crates/ruff_linter/src/rules/flake8_errmsg/rules/string_in_exception.rs b/crates/ruff_linter/src/rules/flake8_errmsg/rules/string_in_exception.rs index caec91dd7ab986..173e60078977ba 100644 --- a/crates/ruff_linter/src/rules/flake8_errmsg/rules/string_in_exception.rs +++ b/crates/ruff_linter/src/rules/flake8_errmsg/rules/string_in_exception.rs @@ -1,10 +1,11 @@ -use ruff_python_ast::{self as ast, Arguments, Expr, ExprContext, Stmt}; -use ruff_text_size::{Ranged, TextRange}; +use ruff_python_ast::{self as ast, Arguments, Expr, Stmt}; +use ruff_source_file::Locator; +use ruff_text_size::Ranged; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::whitespace; -use ruff_python_codegen::{Generator, Stylist}; +use ruff_python_codegen::Stylist; use crate::checkers::ast::Checker; use crate::registry::Rule; @@ -196,7 +197,7 @@ pub(crate) fn string_in_exception(checker: &mut Checker, stmt: &Stmt, exc: &Expr first, indentation, checker.stylist(), - checker.generator(), + checker.locator(), )); } } @@ -216,7 +217,7 @@ pub(crate) fn string_in_exception(checker: &mut Checker, stmt: &Stmt, exc: &Expr first, indentation, checker.stylist(), - checker.generator(), + checker.locator(), )); } } @@ -241,7 +242,7 @@ pub(crate) fn string_in_exception(checker: &mut Checker, stmt: &Stmt, exc: &Expr first, indentation, checker.stylist(), - checker.generator(), + checker.locator(), )); } } @@ -269,28 +270,27 @@ pub(crate) fn string_in_exception(checker: &mut Checker, stmt: &Stmt, exc: &Expr fn generate_fix( stmt: &Stmt, exc_arg: &Expr, - indentation: &str, + stmt_indentation: &str, stylist: &Stylist, - generator: Generator, + locator: &Locator, ) -> Fix { - let assignment = Stmt::Assign(ast::StmtAssign { - targets: vec![Expr::Name(ast::ExprName { - id: "msg".into(), - ctx: ExprContext::Store, - range: TextRange::default(), - })], - value: Box::new(exc_arg.clone()), - range: TextRange::default(), - }); - Fix::unsafe_edits( Edit::insertion( - format!( - "{}{}{}", - generator.stmt(&assignment), - stylist.line_ending().as_str(), - indentation, - ), + if locator.contains_line_break(exc_arg.range()) { + format!( + "msg = ({line_ending}{stmt_indentation}{indentation}{}{line_ending}{stmt_indentation}){line_ending}{stmt_indentation}", + locator.slice(exc_arg.range()), + line_ending = stylist.line_ending().as_str(), + indentation = stylist.indentation().as_str(), + ) + } else { + format!( + "msg = {}{}{}", + locator.slice(exc_arg.range()), + stylist.line_ending().as_str(), + stmt_indentation, + ) + }, stmt.start(), ), [Edit::range_replacement( diff --git a/crates/ruff_linter/src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__custom.snap b/crates/ruff_linter/src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__custom.snap index 8274c69781bbe2..7c237fcc3a1e1b 100644 --- a/crates/ruff_linter/src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__custom.snap +++ b/crates/ruff_linter/src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__custom.snap @@ -177,4 +177,86 @@ EM.py:60:35: EM101 Exception must not use a string literal, assign to variable f | = help: Assign to variable; remove string literal +EM.py:64:24: EM102 [*] Exception must not use an f-string literal, assign to variable first + | +63 | def f_triple_quoted_string(): +64 | raise RuntimeError(f"""This is an {"example"} exception""") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM102 + | + = help: Assign to variable; remove f-string literal + +ℹ Unsafe fix +61 61 | +62 62 | +63 63 | def f_triple_quoted_string(): +64 |- raise RuntimeError(f"""This is an {"example"} exception""") + 64 |+ msg = f"""This is an {"example"} exception""" + 65 |+ raise RuntimeError(msg) +65 66 | +66 67 | +67 68 | # Generate a violaiton for this, but don't fix it + +EM.py:77:9: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first + | +75 | def f_multi_line_string2(): +76 | raise RuntimeError( +77 | "This is an {example} exception".format( + | _________^ +78 | | example="example" +79 | | ) + | |_________^ EM103 +80 | ) + | + = help: Assign to variable; remove `.format()` string + +ℹ Unsafe fix +73 73 | +74 74 | +75 75 | def f_multi_line_string2(): +76 |- raise RuntimeError( + 76 |+ msg = ( +77 77 | "This is an {example} exception".format( +78 78 | example="example" +79 79 | ) +80 80 | ) + 81 |+ raise RuntimeError( + 82 |+ msg + 83 |+ ) +81 84 | +82 85 | +83 86 | def f_multi_line_string2(): + +EM.py:85:9: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first + | +83 | def f_multi_line_string2(): +84 | raise RuntimeError( +85 | ( + | _________^ +86 | | "This is an " +87 | | "{example} exception" +88 | | ).format( +89 | | example="example" +90 | | ) + | |_________^ EM103 +91 | ) + | + = help: Assign to variable; remove `.format()` string + +ℹ Unsafe fix +81 81 | +82 82 | +83 83 | def f_multi_line_string2(): +84 |- raise RuntimeError( + 84 |+ msg = ( +85 85 | ( +86 86 | "This is an " +87 87 | "{example} exception" +-------------------------------------------------------------------------------- +89 89 | example="example" +90 90 | ) +91 91 | ) + 92 |+ raise RuntimeError( + 93 |+ msg + 94 |+ ) + diff --git a/crates/ruff_linter/src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__defaults.snap b/crates/ruff_linter/src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__defaults.snap index b97a3d336c4967..914c556ca6fd04 100644 --- a/crates/ruff_linter/src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__defaults.snap +++ b/crates/ruff_linter/src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__defaults.snap @@ -215,4 +215,114 @@ EM.py:60:35: EM101 Exception must not use a string literal, assign to variable f | = help: Assign to variable; remove string literal +EM.py:64:24: EM102 [*] Exception must not use an f-string literal, assign to variable first + | +63 | def f_triple_quoted_string(): +64 | raise RuntimeError(f"""This is an {"example"} exception""") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ EM102 + | + = help: Assign to variable; remove f-string literal + +ℹ Unsafe fix +61 61 | +62 62 | +63 63 | def f_triple_quoted_string(): +64 |- raise RuntimeError(f"""This is an {"example"} exception""") + 64 |+ msg = f"""This is an {"example"} exception""" + 65 |+ raise RuntimeError(msg) +65 66 | +66 67 | +67 68 | # Generate a violaiton for this, but don't fix it + +EM.py:70:9: EM101 [*] Exception must not use a string literal, assign to variable first + | +68 | def f_multi_line_string(): +69 | raise RuntimeError( +70 | "first" + | _________^ +71 | | "second" + | |________________^ EM101 +72 | ) + | + = help: Assign to variable; remove string literal + +ℹ Unsafe fix +66 66 | +67 67 | # Generate a violaiton for this, but don't fix it +68 68 | def f_multi_line_string(): +69 |- raise RuntimeError( + 69 |+ msg = ( +70 70 | "first" +71 71 | "second" +72 72 | ) + 73 |+ raise RuntimeError( + 74 |+ msg + 75 |+ ) +73 76 | +74 77 | +75 78 | def f_multi_line_string2(): + +EM.py:77:9: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first + | +75 | def f_multi_line_string2(): +76 | raise RuntimeError( +77 | "This is an {example} exception".format( + | _________^ +78 | | example="example" +79 | | ) + | |_________^ EM103 +80 | ) + | + = help: Assign to variable; remove `.format()` string + +ℹ Unsafe fix +73 73 | +74 74 | +75 75 | def f_multi_line_string2(): +76 |- raise RuntimeError( + 76 |+ msg = ( +77 77 | "This is an {example} exception".format( +78 78 | example="example" +79 79 | ) +80 80 | ) + 81 |+ raise RuntimeError( + 82 |+ msg + 83 |+ ) +81 84 | +82 85 | +83 86 | def f_multi_line_string2(): + +EM.py:85:9: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first + | +83 | def f_multi_line_string2(): +84 | raise RuntimeError( +85 | ( + | _________^ +86 | | "This is an " +87 | | "{example} exception" +88 | | ).format( +89 | | example="example" +90 | | ) + | |_________^ EM103 +91 | ) + | + = help: Assign to variable; remove `.format()` string + +ℹ Unsafe fix +81 81 | +82 82 | +83 83 | def f_multi_line_string2(): +84 |- raise RuntimeError( + 84 |+ msg = ( +85 85 | ( +86 86 | "This is an " +87 87 | "{example} exception" +-------------------------------------------------------------------------------- +89 89 | example="example" +90 90 | ) +91 91 | ) + 92 |+ raise RuntimeError( + 93 |+ msg + 94 |+ ) +