Skip to content

Commit fbe1db0

Browse files
committed
[flake8-implicit-str-concat] Avoid invalid fix generated by autofix (ISC003)
As reported in #19757: While attempting ISC003 autofix for an expression with explicit string concatenation, with either operand being a string literal that wraps across multiple lines (in parentheses) - it resulted in generating a fix which caused runtime error. Example: _ = "abc" + ( "def" "ghi" ) was being auto-fixed to: _ = "abc" ( "def" "ghi" ) which raised `TypeError: 'str' object is not callable` This commit makes changes to just report diagnostic - no autofix in such cases. Fixes #19757. Signed-off-by: Prakhar Pratyush <[email protected]>
1 parent 192c37d commit fbe1db0

File tree

4 files changed

+91
-8
lines changed

4 files changed

+91
-8
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_implicit_str_concat/ISC.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,17 @@
208208
_ = f"b {t"abc" \
209209
t"def"} g"
210210

211+
212+
# Explicit concatenation with either operand being
213+
# a string literal that wraps across multiple lines (in parentheses)
214+
# reports diagnostic - no autofix.
215+
# See https://github.com/astral-sh/ruff/issues/19757
216+
_ = "abc" + (
217+
"def"
218+
"ghi"
219+
)
220+
221+
_ = (
222+
"abc"
223+
"def"
224+
) + "ghi"

crates/ruff_linter/src/rules/flake8_implicit_str_concat/rules/explicit.rs

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use ruff_macros::{ViolationMetadata, derive_message_formats};
2+
use ruff_python_ast::parenthesize::parenthesized_range;
23
use ruff_python_ast::{self as ast, Expr, Operator};
34
use ruff_python_trivia::is_python_whitespace;
45
use ruff_source_file::LineRanges;
56
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
67

7-
use crate::AlwaysFixableViolation;
88
use crate::checkers::ast::Checker;
9-
use crate::{Edit, Fix};
9+
use crate::{Edit, Fix, FixAvailability, Violation};
1010

1111
/// ## What it does
1212
/// Checks for string literals that are explicitly concatenated (using the
@@ -36,14 +36,16 @@ use crate::{Edit, Fix};
3636
#[violation_metadata(stable_since = "v0.0.201")]
3737
pub(crate) struct ExplicitStringConcatenation;
3838

39-
impl AlwaysFixableViolation for ExplicitStringConcatenation {
39+
impl Violation for ExplicitStringConcatenation {
40+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
41+
4042
#[derive_message_formats]
4143
fn message(&self) -> String {
4244
"Explicitly concatenated string should be implicitly concatenated".to_string()
4345
}
4446

45-
fn fix_title(&self) -> String {
46-
"Remove redundant '+' operator to implicitly concatenate".to_string()
47+
fn fix_title(&self) -> Option<String> {
48+
Some("Remove redundant '+' operator to implicitly concatenate".to_string())
4749
}
4850
}
4951

@@ -82,9 +84,25 @@ pub(crate) fn explicit(checker: &Checker, expr: &Expr) {
8284
.locator()
8385
.contains_line_break(TextRange::new(left.end(), right.start()))
8486
{
85-
checker
86-
.report_diagnostic(ExplicitStringConcatenation, expr.range())
87-
.set_fix(generate_fix(checker, bin_op));
87+
let mut diagnostic =
88+
checker.report_diagnostic(ExplicitStringConcatenation, expr.range());
89+
90+
let is_parenthesized = |expr: &Expr| {
91+
parenthesized_range(
92+
expr.into(),
93+
bin_op.into(),
94+
checker.comment_ranges(),
95+
checker.source(),
96+
)
97+
.is_some()
98+
};
99+
// If either `left` or `right` is parenthesized, generating
100+
// a fix would be too involved. Just report the diagnostic.
101+
if is_parenthesized(left) || is_parenthesized(right) {
102+
return;
103+
}
104+
105+
diagnostic.set_fix(generate_fix(checker, bin_op));
88106
}
89107
}
90108
}

crates/ruff_linter/src/rules/flake8_implicit_str_concat/snapshots/ruff_linter__rules__flake8_implicit_str_concat__tests__ISC003_ISC.py.snap

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,3 +357,33 @@ help: Remove redundant '+' operator to implicitly concatenate
357357
203 | )
358358
204 |
359359
205 | # nested examples with both t and f-strings
360+
361+
ISC003 Explicitly concatenated string should be implicitly concatenated
362+
--> ISC.py:216:5
363+
|
364+
214 | # reports diagnostic - no autofix.
365+
215 | # See https://github.com/astral-sh/ruff/issues/19757
366+
216 | _ = "abc" + (
367+
| _____^
368+
217 | | "def"
369+
218 | | "ghi"
370+
219 | | )
371+
| |_^
372+
220 |
373+
221 | _ = (
374+
|
375+
help: Remove redundant '+' operator to implicitly concatenate
376+
377+
ISC003 Explicitly concatenated string should be implicitly concatenated
378+
--> ISC.py:221:5
379+
|
380+
219 | )
381+
220 |
382+
221 | _ = (
383+
| _____^
384+
222 | | "abc"
385+
223 | | "def"
386+
224 | | ) + "ghi"
387+
| |_________^
388+
|
389+
help: Remove redundant '+' operator to implicitly concatenate

crates/ruff_linter/src/rules/flake8_implicit_str_concat/snapshots/ruff_linter__rules__flake8_implicit_str_concat__tests__multiline_ISC002_ISC.py.snap

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,24 @@ ISC002 Implicitly concatenated string literals over multiple lines
8989
209 | | t"def"} g"
9090
| |__________^
9191
|
92+
93+
ISC002 Implicitly concatenated string literals over multiple lines
94+
--> ISC.py:217:5
95+
|
96+
215 | # See https://github.com/astral-sh/ruff/issues/19757
97+
216 | _ = "abc" + (
98+
217 | / "def"
99+
218 | | "ghi"
100+
| |_________^
101+
219 | )
102+
|
103+
104+
ISC002 Implicitly concatenated string literals over multiple lines
105+
--> ISC.py:222:5
106+
|
107+
221 | _ = (
108+
222 | / "abc"
109+
223 | | "def"
110+
| |_________^
111+
224 | ) + "ghi"
112+
|

0 commit comments

Comments
 (0)