Slice source code instead of generating it for EM fixes

astral-sh · Nov 9, 2023 · 451acea · 451acea
1 parent 722687a
commit 451acea
Show file tree

Hide file tree

Showing 4 changed files with 247 additions and 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
@@ -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
@@ -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/...r/src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__custom.snap b/...r/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/...src/rules/flake8_errmsg/snapshots/ruff_linter__rules__flake8_errmsg__tests__defaults.snap b/...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 |+    )
+