fix(unicorn/prefer-string-slice): make substring autofix safe

camc314 · camc314 · commit 7aecae2801f5 · 2026-03-04T22:15:56.000Z
diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_string_slice.rs b/crates/oxc_linter/src/rules/unicorn/prefer_string_slice.rs
@@ -1,4 +1,7 @@
-use oxc_ast::{AstKind, ast::MemberExpression};
+use oxc_ast::{
+    AstKind,
+    ast::{Argument, Expression, MemberExpression},
+};
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
 use oxc_span::Span;
@@ -54,11 +57,16 @@ impl Rule for PreferStringSlice {
                 return;
             }
             let method_name = v.property.name.as_str();
-            let has_unsafe_substr_arguments = method_name == "substr"
-                && (call_expr.arguments.len() >= 2
-                    || call_expr.arguments.iter().any(oxc_ast::ast::Argument::is_spread));
+            let has_spread_arguments = call_expr.arguments.iter().any(Argument::is_spread);
+            let has_unsafe_arguments = match method_name {
+                "substr" => has_spread_arguments || call_expr.arguments.len() >= 2,
+                "substring" => {
+                    has_spread_arguments || !is_safe_substring_arguments(&call_expr.arguments)
+                }
+                _ => unreachable!(),
+            };
 
-            if has_unsafe_substr_arguments {
+            if has_unsafe_arguments {
                 ctx.diagnostic(prefer_string_slice_diagnostic(v.property.span, method_name));
                 return;
             }
@@ -71,6 +79,35 @@ impl Rule for PreferStringSlice {
     }
 }
 
+fn is_safe_substring_arguments(arguments: &[Argument<'_>]) -> bool {
+    match arguments {
+        [] => true,
+        [start] => get_non_negative_integer_argument(start).is_some(),
+        [start, end] => {
+            let Some(start_value) = get_non_negative_integer_argument(start) else {
+                return false;
+            };
+            let Some(end_value) = get_non_negative_integer_argument(end) else {
+                return false;
+            };
+            start_value <= end_value
+        }
+        _ => false,
+    }
+}
+
+fn get_non_negative_integer_argument(argument: &Argument<'_>) -> Option<f64> {
+    if !argument.is_expression() {
+        return None;
+    }
+
+    let Expression::NumericLiteral(number) = argument.to_expression().get_inner_expression() else {
+        return None;
+    };
+
+    if number.value >= 0.0 && number.value.fract() == 0.0 { Some(number.value) } else { None }
+}
+
 #[test]
 fn test() {
     use crate::tester::Tester;
@@ -202,7 +239,6 @@ fn test() {
         // (r#""foo".substring(-1, -5)"#, r#""foo".slice(0, 0)"#),
         // (r#""foo".substring(-1, 2)"#, r#""foo".slice(0, 2)"#),
         // (r#""foo".substring(length)"#, r#""foo".slice(Math.max(0, length))"#),
-        (r#""foo".substring("fo".length)"#, r#""foo".slice("fo".length)"#), // spellchecker:disable-line
         // TODO: Get this passing.
         // (r#""foo".substring(0, length)"#, r#""foo".slice(0, Math.max(0, length))"#),
         // (r#""foo".substring(length, 0)"#, r#""foo".slice(0, Math.max(0, length))"#),
diff --git a/crates/oxc_linter/src/snapshots/unicorn_prefer_string_slice.snap b/crates/oxc_linter/src/snapshots/unicorn_prefer_string_slice.snap
@@ -219,56 +219,48 @@ source: crates/oxc_linter/src/tester.rs
  1 │ "foo".substring(2, 1)
    ·       ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:7]
  1 │ "foo".substring(-1, -5)
    ·       ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:7]
  1 │ "foo".substring(-1, 2)
    ·       ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:7]
  1 │ "foo".substring(length)
    ·       ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:10]
  1 │ "foobar".substring("foo".length)
    ·          ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:7]
  1 │ "foo".substring(0, length)
    ·       ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:7]
  1 │ "foo".substring(length, 0)
    ·       ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:5]
  1 │ foo.substring(start)
    ·     ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:7]
@@ -282,7 +274,6 @@ source: crates/oxc_linter/src/tester.rs
  1 │ foo.substring(start, end)
    ·     ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:7]
@@ -296,7 +287,6 @@ source: crates/oxc_linter/src/tester.rs
  1 │ foo.substring(1, 2, 3)
    ·     ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr()
    ╭─[prefer_string_slice.tsx:2:40]
@@ -323,7 +313,6 @@ source: crates/oxc_linter/src/tester.rs
    ·                           ─────────
  3 │                     /* 9 */ (( /* 10 */ bar /* 11 */ )) /* 12 */,
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr()
    ╭─[prefer_string_slice.tsx:1:5]
@@ -360,28 +349,24 @@ source: crates/oxc_linter/src/tester.rs
  1 │ foo.substring((10, 1), 0)
    ·     ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:5]
  1 │ foo.substring(0, (10, 1))
    ·     ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:5]
  1 │ foo.substring(0, await 1)
    ·     ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring()
    ╭─[prefer_string_slice.tsx:1:5]
  1 │ foo.substring((10, bar))
    ·     ─────────
    ╰────
-  help: Replace `substring` with `slice`.
 
   ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr()
    ╭─[prefer_string_slice.tsx:2:35]
