Add Autofix for ISC003 (#18256)

Co-authored-by: Micha Reiser <micha@reiser.io>

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

@@ -91,3 +91,99 @@ _ = "\8""0" # fix should be "\80"
 _ = "\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(
+                if let Some(diagnostic) = flake8_implicit_str_concat::rules::explicit(expr, checker)
-                    expr,
+                {
-                    checker.locator,
-                    checker.settings,
-                ) {
                     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::checkers::ast::Checker;
-use crate::settings::LinterSettings;
 
 /// ## 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(
+pub(crate) fn explicit(expr: &Expr, checker: &Checker) -> Option<Diagnostic> {
-    expr: &Expr,
-    locator: &Locator,
-    settings: &LinterSettings,
-) -> 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 {
+    if let Expr::BinOp(bin_op) = expr {
-        left,
+        if let ast::ExprBinOp {
-        op,
+            left,
-        right,
+            right,
-        range,
+            op: Operator::Add,
-    }) = expr
+            ..
-    {
+        } = bin_op
-        if matches!(op, Operator::Add) {
+        {
-            if matches!(
+            let concatable = matches!(
-                left.as_ref(),
+                (left.as_ref(), right.as_ref()),
-                Expr::FString(_) | Expr::StringLiteral(_) | Expr::BytesLiteral(_)
+                (
-            ) && matches!(
+                    Expr::StringLiteral(_) | Expr::FString(_),
-                right.as_ref(),
+                    Expr::StringLiteral(_) | Expr::FString(_)
-                Expr::FString(_) | Expr::StringLiteral(_) | Expr::BytesLiteral(_)
+                ) | (Expr::BytesLiteral(_), Expr::BytesLiteral(_))
-            ) && locator.contains_line_break(*range)
+            );
+            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

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

@@ -1,7 +1,7 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_implicit_str_concat/mod.rs
 ---
-ISC.py:9:3: ISC003 Explicitly concatenated string should be implicitly concatenated
+ISC.py:9:3: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
    |
  8 |   _ = (
  9 | /   "abc" +
@@ -9,8 +9,19 @@ ISC.py:9:3: ISC003 Explicitly concatenated string should be implicitly concatena
    | |_______^ ISC003
 11 |   )
    |
+   = help: Remove redundant '+' operator to implicitly concatenate
 
-ISC.py:14:3: ISC003 Explicitly concatenated string should be implicitly concatenated
+ℹ Safe fix
+6  6  |     "def"
+7  7  | 
+8  8  | _ = (
+9     |-  "abc" +
+   9  |+  "abc"
+10 10 |   "def"
+11 11 | )
+12 12 | 
+
+ISC.py:14:3: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
    |
 13 |   _ = (
 14 | /   f"abc" +
@@ -18,8 +29,19 @@ ISC.py:14:3: ISC003 Explicitly concatenated string should be implicitly concaten
    | |_______^ ISC003
 16 |   )
    |
+   = help: Remove redundant '+' operator to implicitly concatenate
 
-ISC.py:19:3: ISC003 Explicitly concatenated string should be implicitly concatenated
+ℹ Safe fix
+11 11 | )
+12 12 | 
+13 13 | _ = (
+14    |-  f"abc" +
+   14 |+  f"abc"
+15 15 |   "def"
+16 16 | )
+17 17 | 
+
+ISC.py:19:3: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
    |
 18 |   _ = (
 19 | /   b"abc" +
@@ -27,8 +49,19 @@ ISC.py:19:3: ISC003 Explicitly concatenated string should be implicitly concaten
    | |________^ ISC003
 21 |   )
    |
+   = help: Remove redundant '+' operator to implicitly concatenate
 
-ISC.py:78:10: ISC003 Explicitly concatenated string should be implicitly concatenated
+ℹ Safe fix
+16 16 | )
+17 17 | 
+18 18 | _ = (
+19    |-  b"abc" +
+   19 |+  b"abc"
+20 20 |   b"def"
+21 21 | )
+22 22 | 
+
+ISC.py:78:10: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
    |
 77 |   # Explicitly concatenated nested f-strings
 78 |   _ = f"a {f"first"
@@ -38,8 +71,19 @@ ISC.py:78:10: ISC003 Explicitly concatenated string should be implicitly concate
 80 |   _ = f"a {f"first {f"middle"}"
 81 |       + f"second"} d"
    |
+   = help: Remove redundant '+' operator to implicitly concatenate
 
-ISC.py:80:10: ISC003 Explicitly concatenated string should be implicitly concatenated
+ℹ Safe fix
+76 76 | 
+77 77 | # Explicitly concatenated nested f-strings
+78 78 | _ = f"a {f"first"
+79    |-    + f"second"} d"
+   79 |+     f"second"} d"
+80 80 | _ = f"a {f"first {f"middle"}"
+81 81 |     + f"second"} d"
+82 82 | 
+
+ISC.py:80:10: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
    |
 78 |   _ = f"a {f"first"
 79 |       + f"second"} d"
@@ -50,3 +94,263 @@ ISC.py:80:10: ISC003 Explicitly concatenated string should be implicitly concate
 82 |
 83 |   # See https://github.com/astral-sh/ruff/issues/12936
    |
+   = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+78 78 | _ = f"a {f"first"
+79 79 |     + f"second"} d"
+80 80 | _ = f"a {f"first {f"middle"}"
+81    |-    + f"second"} d"
+   81 |+     f"second"} d"
+82 82 | 
+83 83 | # See https://github.com/astral-sh/ruff/issues/12936
+84 84 | _ = "\12""0" # fix should be "\0120"
+
+ISC.py:110:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+109 |   _ = (
+110 | /     rf"raw_f{x}" +
+111 | |     r"raw_normal"
+    | |_________________^ ISC003
+112 |   )
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+107 107 | )
+108 108 | 
+109 109 | _ = (
+110     |-    rf"raw_f{x}" +
+    110 |+    rf"raw_f{x}"
+111 111 |     r"raw_normal"
+112 112 | )
+113 113 | 
+
+ISC.py:117:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+115 |   # Different prefix combinations
+116 |   _ = (
+117 | /     u"unicode" +
+118 | |     r"raw"
+    | |__________^ ISC003
+119 |   )
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+114 114 | 
+115 115 | # Different prefix combinations
+116 116 | _ = (
+117     |-    u"unicode" +
+    117 |+    u"unicode"
+118 118 |     r"raw"
+119 119 | )
+120 120 | 
+
+ISC.py:122:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+121 |   _ = (
+122 | /     rb"raw_bytes" +
+123 | |     b"normal_bytes"
+    | |___________________^ ISC003
+124 |   )
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+119 119 | )
+120 120 | 
+121 121 | _ = (
+122     |-    rb"raw_bytes" +
+    122 |+    rb"raw_bytes"
+123 123 |     b"normal_bytes"
+124 124 | )
+125 125 | 
+
+ISC.py:127:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+126 |   _ = (
+127 | /     b"bytes" +
+128 | |     b"with_bytes"
+    | |_________________^ ISC003
+129 |   )
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+124 124 | )
+125 125 | 
+126 126 | _ = (
+127     |-    b"bytes" +
+    127 |+    b"bytes"
+128 128 |     b"with_bytes"
+129 129 | )
+130 130 | 
+
+ISC.py:133:6: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+131 |   # Repeated concatenation
+132 |
+133 |   _ = ("a" +
+    |  ______^
+134 | |     "b" +
+    | |_______^ ISC003
+135 |       "c" +
+136 |       "d" + "e"
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+130 130 | 
+131 131 | # Repeated concatenation
+132 132 | 
+133     |-_ = ("a" +
+    133 |+_ = ("a"
+134 134 |     "b" +
+135 135 |     "c" +
+136 136 |     "d" + "e"
+
+ISC.py:139:6: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+137 |   )
+138 |
+139 |   _ = ("a"
+    |  ______^
+140 | |     + "b"
+    | |_________^ ISC003
+141 |       + "c"
+142 |       + "d"
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+137 137 | )
+138 138 | 
+139 139 | _ = ("a"
+140     |-    + "b"
+    140 |+     "b"
+141 141 |     + "c"
+142 142 |     + "d"
+143 143 |     + "e"
+
+ISC.py:160:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+159 |   _ = (
+160 | /     "first"
+161 | |     +    "second"  # extra spaces around +
+    | |_________________^ ISC003
+162 |   )
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+158 158 | 
+159 159 | _ = (
+160 160 |     "first"
+161     |-    +    "second"  # extra spaces around +
+    161 |+        "second"  # extra spaces around +
+162 162 | )
+163 163 | 
+164 164 | _ = (
+
+ISC.py:165:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+164 |   _ = (
+165 | /     "first"    +  # trailing spaces before +
+166 | |     "second"
+    | |____________^ ISC003
+167 |   )
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+162 162 | )
+163 163 | 
+164 164 | _ = (
+165     |-    "first"    +  # trailing spaces before +
+    165 |+    "first"  # trailing spaces before +
+166 166 |     "second"
+167 167 | )
+168 168 | 
+
+ISC.py:170:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+169 |   _ = ((
+170 | /     "deep" +
+171 | |     "nesting"
+    | |_____________^ ISC003
+172 |   ))
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+167 167 | )
+168 168 | 
+169 169 | _ = ((
+170     |-    "deep" +
+    170 |+    "deep"
+171 171 |     "nesting"
+172 172 | ))
+173 173 | 
+
+ISC.py:175:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+174 |   _ = (
+175 | /     "contains + plus" +
+176 | |     "another string"
+    | |____________________^ ISC003
+177 |   )
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+172 172 | ))
+173 173 | 
+174 174 | _ = (
+175     |-    "contains + plus" +
+    175 |+    "contains + plus"
+176 176 |     "another string"
+177 177 | )
+178 178 | 
+
+ISC.py:180:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+179 |   _ = (
+180 | /     "start"
+181 | |         # leading comment
+182 | |     + "end"
+    | |___________^ ISC003
+183 |   )
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+179 179 | _ = (
+180 180 |     "start"
+181 181 | 		# leading comment
+182     |-    + "end"
+    182 |+     "end"
+183 183 | )
+184 184 | 
+185 185 | _ = (
+
+ISC.py:186:5: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
+    |
+185 |   _ = (
+186 | /     "start" +
+187 | |         # leading comment
+188 | |     "end"
+    | |_________^ ISC003
+189 |   )
+    |
+    = help: Remove redundant '+' operator to implicitly concatenate
+
+ℹ Safe fix
+183 183 | )
+184 184 | 
+185 185 | _ = (
+186     |-    "start" +
+    186 |+    "start"
+187 187 | 		# leading comment
+188 188 |     "end"
+189 189 | )

crates/ruff_linter/src/rules/flake8_implicit_str_concat/snapshots/ruff_linter__rules__flake8_implicit_str_concat__tests__multiline_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