mirror of https://github.com/astral-sh/ruff
[`flake8-implicit-str-concat`] Avoid invalid fix generated by autofix (`ISC003`) (#21517)
## Summary 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. ## Test Plan Added example scenarios in `crates/ruff_linter/resources/test/fixtures/flake8_implicit_str_concat/ISC.py`. Signed-off-by: Prakhar Pratyush <prakhar1144@gmail.com>
This commit is contained in:
parent
ddc1417f22
commit
492d676736
|
|
@ -208,3 +208,17 @@ _ = t"b {f"c" f"d {t"e" t"f"} g"} h"
|
|||
_ = f"b {t"abc" \
|
||||
t"def"} g"
|
||||
|
||||
|
||||
# Explicit concatenation with either operand being
|
||||
# a string literal that wraps across multiple lines (in parentheses)
|
||||
# reports diagnostic - no autofix.
|
||||
# See https://github.com/astral-sh/ruff/issues/19757
|
||||
_ = "abc" + (
|
||||
"def"
|
||||
"ghi"
|
||||
)
|
||||
|
||||
_ = (
|
||||
"abc"
|
||||
"def"
|
||||
) + "ghi"
|
||||
|
|
|
|||
|
|
@ -1,12 +1,12 @@
|
|||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||
use ruff_python_ast::parenthesize::parenthesized_range;
|
||||
use ruff_python_ast::{self as ast, Expr, Operator};
|
||||
use ruff_python_trivia::is_python_whitespace;
|
||||
use ruff_source_file::LineRanges;
|
||||
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
|
||||
|
||||
use crate::AlwaysFixableViolation;
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::{Edit, Fix};
|
||||
use crate::{Edit, Fix, FixAvailability, Violation};
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for string literals that are explicitly concatenated (using the
|
||||
|
|
@ -36,14 +36,16 @@ use crate::{Edit, Fix};
|
|||
#[violation_metadata(stable_since = "v0.0.201")]
|
||||
pub(crate) struct ExplicitStringConcatenation;
|
||||
|
||||
impl AlwaysFixableViolation for ExplicitStringConcatenation {
|
||||
impl Violation for ExplicitStringConcatenation {
|
||||
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
|
||||
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
"Explicitly concatenated string should be implicitly concatenated".to_string()
|
||||
}
|
||||
|
||||
fn fix_title(&self) -> String {
|
||||
"Remove redundant '+' operator to implicitly concatenate".to_string()
|
||||
fn fix_title(&self) -> Option<String> {
|
||||
Some("Remove redundant '+' operator to implicitly concatenate".to_string())
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -82,9 +84,27 @@ pub(crate) fn explicit(checker: &Checker, expr: &Expr) {
|
|||
.locator()
|
||||
.contains_line_break(TextRange::new(left.end(), right.start()))
|
||||
{
|
||||
checker
|
||||
.report_diagnostic(ExplicitStringConcatenation, expr.range())
|
||||
.set_fix(generate_fix(checker, bin_op));
|
||||
let mut diagnostic =
|
||||
checker.report_diagnostic(ExplicitStringConcatenation, expr.range());
|
||||
|
||||
let is_parenthesized = |expr: &Expr| {
|
||||
parenthesized_range(
|
||||
expr.into(),
|
||||
bin_op.into(),
|
||||
checker.comment_ranges(),
|
||||
checker.source(),
|
||||
)
|
||||
.is_some()
|
||||
};
|
||||
// If either `left` or `right` is parenthesized, generating
|
||||
// a fix would be too involved. Just report the diagnostic.
|
||||
// Currently, attempting `generate_fix` would result in
|
||||
// an invalid code. See: #19757
|
||||
if is_parenthesized(left) || is_parenthesized(right) {
|
||||
return;
|
||||
}
|
||||
|
||||
diagnostic.set_fix(generate_fix(checker, bin_op));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -357,3 +357,33 @@ help: Remove redundant '+' operator to implicitly concatenate
|
|||
203 | )
|
||||
204 |
|
||||
205 | # nested examples with both t and f-strings
|
||||
|
||||
ISC003 Explicitly concatenated string should be implicitly concatenated
|
||||
--> ISC.py:216:5
|
||||
|
|
||||
214 | # reports diagnostic - no autofix.
|
||||
215 | # See https://github.com/astral-sh/ruff/issues/19757
|
||||
216 | _ = "abc" + (
|
||||
| _____^
|
||||
217 | | "def"
|
||||
218 | | "ghi"
|
||||
219 | | )
|
||||
| |_^
|
||||
220 |
|
||||
221 | _ = (
|
||||
|
|
||||
help: Remove redundant '+' operator to implicitly concatenate
|
||||
|
||||
ISC003 Explicitly concatenated string should be implicitly concatenated
|
||||
--> ISC.py:221:5
|
||||
|
|
||||
219 | )
|
||||
220 |
|
||||
221 | _ = (
|
||||
| _____^
|
||||
222 | | "abc"
|
||||
223 | | "def"
|
||||
224 | | ) + "ghi"
|
||||
| |_________^
|
||||
|
|
||||
help: Remove redundant '+' operator to implicitly concatenate
|
||||
|
|
|
|||
|
|
@ -89,3 +89,24 @@ ISC002 Implicitly concatenated string literals over multiple lines
|
|||
209 | | t"def"} g"
|
||||
| |__________^
|
||||
|
|
||||
|
||||
ISC002 Implicitly concatenated string literals over multiple lines
|
||||
--> ISC.py:217:5
|
||||
|
|
||||
215 | # See https://github.com/astral-sh/ruff/issues/19757
|
||||
216 | _ = "abc" + (
|
||||
217 | / "def"
|
||||
218 | | "ghi"
|
||||
| |_________^
|
||||
219 | )
|
||||
|
|
||||
|
||||
ISC002 Implicitly concatenated string literals over multiple lines
|
||||
--> ISC.py:222:5
|
||||
|
|
||||
221 | _ = (
|
||||
222 | / "abc"
|
||||
223 | | "def"
|
||||
| |_________^
|
||||
224 | ) + "ghi"
|
||||
|
|
||||
|
|
|
|||
Loading…
Reference in New Issue