Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slice source code instead of generating it for EM fixes #7746

Merged
merged 1 commit into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_errmsg/EM.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,33 @@ 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""")


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"
)
)
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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(),
));
}
}
Expand All @@ -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(),
));
}
}
Expand All @@ -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(),
));
}
}
Expand Down Expand Up @@ -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 {
Comment on lines +280 to +285
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this too much? 😅

We can just keep a simple format!("msg = ({}){}{}") instead.

format!(
"msg = {}{}{}",
locator.slice(exc_arg.range()),
stylist.line_ending().as_str(),
stmt_indentation,
)
},
stmt.start(),
),
[Edit::range_replacement(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 | def f_multi_line_string():

EM.py:76:9: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first
|
74 | def f_multi_line_string2():
75 | raise RuntimeError(
76 | "This is an {example} exception".format(
| _________^
77 | | example="example"
78 | | )
| |_________^ EM103
79 | )
|
= help: Assign to variable; remove `.format()` string

ℹ Unsafe fix
72 72 |
73 73 |
74 74 | def f_multi_line_string2():
75 |- raise RuntimeError(
75 |+ msg = (
76 76 | "This is an {example} exception".format(
77 77 | example="example"
78 78 | )
79 79 | )
80 |+ raise RuntimeError(
81 |+ msg
82 |+ )
80 83 |
81 84 |
82 85 | def f_multi_line_string2():

EM.py:84:9: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first
|
82 | def f_multi_line_string2():
83 | raise RuntimeError(
84 | (
| _________^
85 | | "This is an "
86 | | "{example} exception"
87 | | ).format(
88 | | example="example"
89 | | )
| |_________^ EM103
90 | )
|
= help: Assign to variable; remove `.format()` string

ℹ Unsafe fix
80 80 |
81 81 |
82 82 | def f_multi_line_string2():
83 |- raise RuntimeError(
83 |+ msg = (
84 84 | (
85 85 | "This is an "
86 86 | "{example} exception"
--------------------------------------------------------------------------------
88 88 | example="example"
89 89 | )
90 90 | )
91 |+ raise RuntimeError(
92 |+ msg
93 |+ )


Original file line number Diff line number Diff line change
Expand Up @@ -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 | def f_multi_line_string():

EM.py:69:9: EM101 [*] Exception must not use a string literal, assign to variable first
|
67 | def f_multi_line_string():
68 | raise RuntimeError(
69 | "first"
| _________^
70 | | "second"
| |________________^ EM101
71 | )
|
= help: Assign to variable; remove string literal

ℹ Unsafe fix
65 65 |
66 66 |
67 67 | def f_multi_line_string():
68 |- raise RuntimeError(
68 |+ msg = (
69 69 | "first"
70 70 | "second"
71 71 | )
72 |+ raise RuntimeError(
73 |+ msg
74 |+ )
72 75 |
73 76 |
74 77 | def f_multi_line_string2():

EM.py:76:9: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first
|
74 | def f_multi_line_string2():
75 | raise RuntimeError(
76 | "This is an {example} exception".format(
| _________^
77 | | example="example"
78 | | )
| |_________^ EM103
79 | )
|
= help: Assign to variable; remove `.format()` string

ℹ Unsafe fix
72 72 |
73 73 |
74 74 | def f_multi_line_string2():
75 |- raise RuntimeError(
75 |+ msg = (
76 76 | "This is an {example} exception".format(
77 77 | example="example"
78 78 | )
79 79 | )
80 |+ raise RuntimeError(
81 |+ msg
82 |+ )
80 83 |
81 84 |
82 85 | def f_multi_line_string2():

EM.py:84:9: EM103 [*] Exception must not use a `.format()` string directly, assign to variable first
|
82 | def f_multi_line_string2():
83 | raise RuntimeError(
84 | (
| _________^
85 | | "This is an "
86 | | "{example} exception"
87 | | ).format(
88 | | example="example"
89 | | )
| |_________^ EM103
90 | )
|
= help: Assign to variable; remove `.format()` string

ℹ Unsafe fix
80 80 |
81 81 |
82 82 | def f_multi_line_string2():
83 |- raise RuntimeError(
83 |+ msg = (
84 84 | (
85 85 | "This is an "
86 86 | "{example} exception"
--------------------------------------------------------------------------------
88 88 | example="example"
89 89 | )
90 90 | )
91 |+ raise RuntimeError(
92 |+ msg
93 |+ )


Loading