diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_string_slice.rs b/crates/oxc_linter/src/rules/unicorn/prefer_string_slice.rs index 20aace2afc958..6e0bd73a589d8 100644 --- 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; @@ -36,7 +39,7 @@ declare_oxc_lint!( PreferStringSlice, unicorn, pedantic, - fix + conditional_fix ); impl Rule for PreferStringSlice { @@ -53,14 +56,58 @@ impl Rule for PreferStringSlice { if !matches!(v.property.name.as_str(), "substr" | "substring") { return; } + let method_name = v.property.name.as_str(); + 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_arguments { + ctx.diagnostic(prefer_string_slice_diagnostic(v.property.span, method_name)); + return; + } + ctx.diagnostic_with_fix( - prefer_string_slice_diagnostic(v.property.span, v.property.name.as_str()), + prefer_string_slice_diagnostic(v.property.span, method_name), |fixer| fixer.replace(v.property.span, "slice"), ); } } } +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 { + 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; @@ -90,6 +137,7 @@ fn test() { r#""foo".substr()"#, r#""foo".substr(1)"#, r#""foo".substr(1, 2)"#, + r#""foo".substr(11, 8)"#, r#""foo".substr(bar.length, Math.min(baz, 100))"#, r#""foo".substr(1, length)"#, r#""foo".substr(1, "abc".length)"#, @@ -168,7 +216,6 @@ fn test() { // "foo".slice(0, Math.max(0, length))"#, // ), // (r#""foo".substr(0, -1)"#, r#""foo".slice(0, 0)"#), - (r#""foo".substr(0, "foo".length)"#, r#""foo".slice(0, "foo".length)"#), ( "const uri = 'foo'; ((uri || '')).substr(1)", @@ -192,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 index ec5fc10f468fd..59cac93900970 100644 --- a/crates/oxc_linter/src/snapshots/unicorn_prefer_string_slice.snap +++ b/crates/oxc_linter/src/snapshots/unicorn_prefer_string_slice.snap @@ -84,49 +84,48 @@ source: crates/oxc_linter/src/tester.rs 1 │ "foo".substr(1, 2) · ────── ╰──── - help: Replace `substr` with `slice`. + + ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() + ╭─[prefer_string_slice.tsx:1:7] + 1 │ "foo".substr(11, 8) + · ────── + ╰──── ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:7] 1 │ "foo".substr(bar.length, Math.min(baz, 100)) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:7] 1 │ "foo".substr(1, length) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:7] 1 │ "foo".substr(1, "abc".length) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:7] 1 │ "foo".substr("1", 2) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:7] 1 │ "foo".substr(0, -1) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:7] 1 │ "foo".substr(0, "foo".length) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:2:19] @@ -134,14 +133,12 @@ source: crates/oxc_linter/src/tester.rs 2 │ "foo".substr(1, length - 4) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:7] 1 │ "foo".substr(1, length) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:2:27] @@ -170,28 +167,24 @@ source: crates/oxc_linter/src/tester.rs 1 │ foo.substr(start, length) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:7] 1 │ "foo".substr(1, 2) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:5] 1 │ foo.substr(1, 2, 3) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:10] 1 │ "Sample".substr(0, "Sample".lastIndexOf("/")) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring() ╭─[prefer_string_slice.tsx:1:5] @@ -226,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] @@ -289,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] @@ -303,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] @@ -330,70 +313,60 @@ 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] 1 │ foo.substr(0, ...bar) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:5] 1 │ foo.substr(...bar) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:5] 1 │ foo.substr(0, (100, 1)) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:5] 1 │ foo.substr(0, 1, extraArgument) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substr() ╭─[prefer_string_slice.tsx:1:5] 1 │ foo.substr((0, bar.length), (0, baz.length)) · ────── ╰──── - help: Replace `substr` with `slice`. ⚠ eslint-plugin-unicorn(prefer-string-slice): Prefer String#slice() over String#substring() ╭─[prefer_string_slice.tsx:1:5] 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] @@ -401,4 +374,3 @@ source: crates/oxc_linter/src/tester.rs 2 │ const output = string.substr(-2, 2); · ────── ╰──── - help: Replace `substr` with `slice`.