Add Autofix for ISC003

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

@@ -91,3 +91,99 @@
 _ = "\12""8" # fix should be "\128"
 _ = "\12""foo" # fix should be "\12foo"
 _ = "\12" ""  # fix should be "\12"
+
+
+# Mixed literal + non-literal scenarios
+_ = (
+    "start" +
+    variable +
+    "end"
+)
+
+_ = (
+    f"format" +
+    func_call() +
+    "literal"
+)
+
+_ = (
+    rf"raw_f{x}" +
+    r"raw_normal"
+)
+
+
+# Different prefix combinations
+_ = (
+    u"unicode" +
+    r"raw"
+)
+
+_ = (
+    rb"raw_bytes" +
+    b"normal_bytes"
+)
+
+_ = (
+    b"bytes" +
+    b"with_bytes"
+)
+
+# Repeated concatenation
+
+_ = ("a" +
+    "b" +
+    "c" +
+    "d" + "e"
+)
+
+_ = ("a"
+    + "b"
+    + "c"
+    + "d"
+    + "e"
+)
+
+_ = (
+    "start" +
+    variable + # comment
+    "end"
+)
+
+_ = (
+    "start" +
+    variable
+		# leading comment
+    + "end"
+)
+
+_ = (
+    "first"
+    +    "second"  # extra spaces around +
+)
+
+_ = (
+    "first"    +  # trailing spaces before +
+    "second"
+)
+
+_ = ((
+    "deep" +
+    "nesting"
+))
+
+_ = (
+    "contains + plus" +
+    "another string"
+)
+
+_ = (
+    "start"
+		# leading comment
+    + "end"
+)
+
+_ = (
+    "start" +
+		# leading comment
+    "end"
+)

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1364,11 +1364,8 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
             op: Operator::Add, ..
         }) => {
             if checker.enabled(Rule::ExplicitStringConcatenation) {
-                if let Some(diagnostic) = flake8_implicit_str_concat::rules::explicit(
-                    expr,
-                    checker.locator,
-                    checker.settings,
-                ) {
+                if let Some(diagnostic) = flake8_implicit_str_concat::rules::explicit(expr, checker)
+                {
                     checker.report_diagnostic(diagnostic);
                 }
             }

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

@@ -1,11 +1,12 @@
-use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_diagnostics::AlwaysFixableViolation;
+use ruff_diagnostics::{Diagnostic, Edit, Fix};
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 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;
+use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
 
-use crate::Locator;
-use crate::settings::LinterSettings;
+use crate::checkers::ast::Checker;
 
 /// ## What it does
 /// Checks for string literals that are explicitly concatenated (using the
@@ -34,46 +35,76 @@ use crate::settings::LinterSettings;
 #[derive(ViolationMetadata)]
 pub(crate) struct ExplicitStringConcatenation;
 
-impl Violation for ExplicitStringConcatenation {
+impl AlwaysFixableViolation for ExplicitStringConcatenation {
     #[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()
+    }
 }
 
 /// ISC003
-pub(crate) fn explicit(
-    expr: &Expr,
-    locator: &Locator,
-    settings: &LinterSettings,
-) -> Option<Diagnostic> {
+pub(crate) fn explicit(expr: &Expr, checker: &Checker) -> Option<Diagnostic> {
     // If the user sets `allow-multiline` to `false`, then we should allow explicitly concatenated
     // strings that span multiple lines even if this rule is enabled. Otherwise, there's no way
     // for the user to write multiline strings, and that setting is "more explicit" than this rule
     // being enabled.
-    if !settings.flake8_implicit_str_concat.allow_multiline {
+    if !checker.settings.flake8_implicit_str_concat.allow_multiline {
         return None;
     }
 
-    if let Expr::BinOp(ast::ExprBinOp {
-        left,
-        op,
-        right,
-        range,
-    }) = expr
-    {
-        if matches!(op, Operator::Add) {
-            if matches!(
-                left.as_ref(),
-                Expr::FString(_) | Expr::StringLiteral(_) | Expr::BytesLiteral(_)
-            ) && matches!(
-                right.as_ref(),
-                Expr::FString(_) | Expr::StringLiteral(_) | Expr::BytesLiteral(_)
-            ) && locator.contains_line_break(*range)
+    if let Expr::BinOp(bin_op) = expr {
+        if let ast::ExprBinOp {
+            left,
+            right,
+            op: Operator::Add,
+            ..
+        } = bin_op
+        {
+            let concatable = matches!(
+                (left.as_ref(), right.as_ref()),
+                (
+                    Expr::StringLiteral(_) | Expr::FString(_),
+                    Expr::StringLiteral(_) | Expr::FString(_)
+                ) | (Expr::BytesLiteral(_), Expr::BytesLiteral(_))
+            );
+            if concatable
+                && checker
+                    .locator()
+                    .contains_line_break(TextRange::new(left.end(), right.start()))
             {
-                return Some(Diagnostic::new(ExplicitStringConcatenation, expr.range()));
+                let mut diagnostic = Diagnostic::new(ExplicitStringConcatenation, expr.range());
+                diagnostic.set_fix(generate_fix(checker, bin_op));
+                return Some(diagnostic);
             }
         }
     }
     None
 }
+
+fn generate_fix(checker: &Checker, expr_bin_op: &ast::ExprBinOp) -> Fix {
+    let ast::ExprBinOp { left, right, .. } = expr_bin_op;
+
+    let between_operands_range = TextRange::new(left.end(), right.start());
+    let between_operands = checker.locator().slice(between_operands_range);
+    let (before_plus, after_plus) = between_operands.split_once('+').unwrap();
+
+    let linebreak_before_operator =
+        before_plus.contains_line_break(TextRange::at(TextSize::new(0), before_plus.text_len()));
+
+    // If removing `+` from first line trim trailing spaces
+    // Preserve indentation when removing `+` from second line
+    let before_plus = if linebreak_before_operator {
+        before_plus
+    } else {
+        before_plus.trim_end_matches(is_python_whitespace)
+    };
+
+    Fix::safe_edit(Edit::range_replacement(
+        format!("{before_plus}{after_plus}"),
+        between_operands_range,
+    ))
+}

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

@@ -461,6 +461,7 @@ ISC.py:91:5: ISC001 [*] Implicitly concatenated string literals on one line
    91 |+_ = "\128" # fix should be "\128"
 92 92 | _ = "\12""foo" # fix should be "\12foo"
 93 93 | _ = "\12" ""  # fix should be "\12"
+94 94 | 
 
 ISC.py:92:5: ISC001 [*] Implicitly concatenated string literals on one line
    |
@@ -479,6 +480,8 @@ ISC.py:92:5: ISC001 [*] Implicitly concatenated string literals on one line
 92    |-_ = "\12""foo" # fix should be "\12foo"
    92 |+_ = "\12foo" # fix should be "\12foo"
 93 93 | _ = "\12" ""  # fix should be "\12"
+94 94 | 
+95 95 | 
 
 ISC.py:93:5: ISC001 [*] Implicitly concatenated string literals on one line
    |
@@ -495,3 +498,6 @@ ISC.py:93:5: ISC001 [*] Implicitly concatenated string literals on one line
 92 92 | _ = "\12""foo" # fix should be "\12foo"
 93    |-_ = "\12" ""  # fix should be "\12"
    93 |+_ = "\12"  # fix should be "\12"
+94 94 | 
+95 95 | 
+96 96 | # Mixed literal + non-literal scenarios