diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index acafcd78f..8798cdaea 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -296,3 +296,20 @@ pub fn get_declaration_of_variable<'a, 'b>( let symbol_id = reference.symbol_id()?; Some(ctx.nodes().get_node(symbol_table.get_declaration(symbol_id))) } + +pub fn extract_regex_flags<'a>( + args: &'a oxc_allocator::Vec<'a, Argument<'a>>, +) -> Option { + if args.len() <= 1 { + return None; + } + let Argument::Expression(Expression::StringLiteral(flag_arg)) = &args[1] else { + return None; + }; + let mut flags = RegExpFlags::empty(); + for ch in flag_arg.value.chars() { + let flag = RegExpFlags::try_from(ch).ok()?; + flags |= flag; + } + Some(flags) +} diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 3007ffd34..fd76ae622 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -171,6 +171,7 @@ mod unicorn { pub mod prefer_query_selector; pub mod prefer_regexp_test; pub mod prefer_spread; + pub mod prefer_string_replace_all; pub mod prefer_string_starts_ends_with; pub mod prefer_string_trim_start_end; pub mod prefer_type_error; @@ -316,6 +317,7 @@ oxc_macros::declare_all_lint_rules! { unicorn::prefer_query_selector, unicorn::prefer_regexp_test, unicorn::prefer_spread, + unicorn::prefer_string_replace_all, unicorn::prefer_string_trim_start_end, unicorn::prefer_string_starts_ends_with, unicorn::prefer_type_error, diff --git a/crates/oxc_linter/src/rules/eslint/no_control_regex.rs b/crates/oxc_linter/src/rules/eslint/no_control_regex.rs index 715618ba2..1e22771af 100644 --- a/crates/oxc_linter/src/rules/eslint/no_control_regex.rs +++ b/crates/oxc_linter/src/rules/eslint/no_control_regex.rs @@ -11,7 +11,7 @@ use oxc_macros::declare_oxc_lint; use oxc_span::{Atom, GetSpan, Span}; use regex::{Matches, Regex}; -use crate::{context::LintContext, rule::Rule, AstNode}; +use crate::{ast_util::extract_regex_flags, context::LintContext, rule::Rule, AstNode}; #[derive(Debug, Error, Diagnostic)] #[error("eslint(no-control-regex): Unexpected control character(s)")] @@ -141,21 +141,6 @@ impl Rule for NoControlRegex { } } -fn extract_flags<'a>(args: &'a oxc_allocator::Vec<'a, Argument<'a>>) -> Option { - if args.len() <= 1 { - return None; - } - let Argument::Expression(Expression::StringLiteral(flag_arg)) = &args[1] else { - return None; - }; - let mut flags = RegExpFlags::empty(); - for ch in flag_arg.value.chars() { - let flag = RegExpFlags::try_from(ch).ok()?; - flags |= flag; - } - Some(flags) -} - struct RegexPatternData<'a> { /// A regex pattern, either from a literal (`/foo/`) a RegExp constructor /// (`new RegExp("foo")`), or a RegExp function call (`RegExp("foo")) @@ -208,7 +193,7 @@ fn regex_pattern<'a>(node: &AstNode<'a>) -> Option> { // RegExp("pat") expression, not just "pat". Some(RegexPatternData { pattern: &pattern.value, - flags: extract_flags(&expr.arguments), + flags: extract_regex_flags(&expr.arguments), span: kind.span(), }) } else { @@ -237,7 +222,7 @@ fn regex_pattern<'a>(node: &AstNode<'a>) -> Option> { // RegExp("pat") expression, not just "pat". Some(RegexPatternData { pattern: &pattern.value, - flags: extract_flags(&expr.arguments), + flags: extract_regex_flags(&expr.arguments), span: kind.span(), }) } else { diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_string_replace_all.rs b/crates/oxc_linter/src/rules/unicorn/prefer_string_replace_all.rs new file mode 100644 index 000000000..ed2c8f9d7 --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/prefer_string_replace_all.rs @@ -0,0 +1,168 @@ +use oxc_ast::{ + ast::{Argument, Expression, MemberExpression, RegExpFlags}, + AstKind, +}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{ast_util::extract_regex_flags, context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.")] +#[diagnostic(severity(warning))] +struct PreferStringReplaceAllDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct PreferStringReplaceAll; + +declare_oxc_lint!( + /// ### What it does + /// + /// Prefers [`String#replaceAll()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll) over [`String#replace()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace) when using a regex with the global flag. + /// + /// ### Why is this bad? + /// + /// The [`String#replaceAll()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll) method is both faster and safer as you don't have to use a regex and remember to escape it if the string is not a literal. And when used with a regex, it makes the intent clearer. + /// + /// ### Example + /// ```javascript + /// ``` + PreferStringReplaceAll, + pedantic +); + +impl Rule for PreferStringReplaceAll { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::CallExpression(call_expr) = node.kind() else { return }; + + let Some(member_expr) = call_expr.callee.get_member_expr() else { return }; + + let MemberExpression::StaticMemberExpression(static_member_expr) = member_expr else { + return; + }; + + if !matches!(static_member_expr.property.name.as_str(), "replace" | "replaceAll") { + return; + } + + if call_expr.arguments.len() != 2 { + return; + } + + let Argument::Expression(pattern) = &call_expr.arguments[0] else { return }; + + if !is_reg_exp_with_global_flag(pattern) { + return; + } + + ctx.diagnostic(PreferStringReplaceAllDiagnostic(static_member_expr.property.span)); + } +} + +fn is_reg_exp_with_global_flag<'a>(expr: &'a Expression<'a>) -> bool { + if let Expression::RegExpLiteral(reg_exp_literal) = expr { + return reg_exp_literal.regex.flags.contains(RegExpFlags::G); + } + + if let Expression::NewExpression(new_expr) = expr { + if !new_expr.callee.is_specific_id("RegExp") { + return false; + } + + if let Some(flags) = extract_regex_flags(&new_expr.arguments) { + return flags.contains(RegExpFlags::G); + } + } + + false +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + r#"foo.replace(/a/, bar)"#, + r#"foo.replaceAll(/a/, bar)"#, + r#"foo.replace("string", bar)"#, + r#"foo.replaceAll("string", bar)"#, + r#"foo.replace(/a/g)"#, + r#"foo.replaceAll(/a/g)"#, + r"foo.replace(/\\./g)", + r"foo.replaceAll(/\\./g)", + r#"new foo.replace(/a/g, bar)"#, + r#"new foo.replaceAll(/a/g, bar)"#, + r#"replace(/a/g, bar)"#, + r#"replaceAll(/a/g, bar)"#, + r#"foo[replace](/a/g, bar);"#, + r#"foo[replaceAll](/a/g, bar);"#, + r#"foo.methodNotReplace(/a/g, bar);"#, + r#"foo['replace'](/a/g, bar)"#, + r#"foo['replaceAll'](/a/g, bar)"#, + r#"foo.replace(/a/g, bar, extra);"#, + r#"foo.replaceAll(/a/g, bar, extra);"#, + r#"foo.replace();"#, + r#"foo.replaceAll();"#, + r#"foo.replace(...argumentsArray, ...argumentsArray2)"#, + r#"foo.replaceAll(...argumentsArray, ...argumentsArray2)"#, + r#"foo.replace(unknown, bar)"#, + r#"const pattern = new RegExp("foo", unknown); foo.replace(pattern, bar)"#, + r#"const pattern = new RegExp("foo"); foo.replace(pattern, bar)"#, + r#"const pattern = new RegExp(); foo.replace(pattern, bar)"#, + r#"const pattern = "string"; foo.replace(pattern, bar)"#, + r#"const pattern = new RegExp("foo", "g"); foo.replace(...[pattern], bar)"#, + r#"const pattern = "not-a-regexp"; foo.replace(pattern, bar)"#, + r#"const pattern = new RegExp("foo", "i"); foo.replace(pattern, bar)"#, + r#"foo.replace(new NotRegExp("foo", "g"), bar)"#, + ]; + + let fail = vec![ + r#"foo.replace(/a/g, bar)"#, + r#"foo.replace(/"'/g, '\'')"#, + r"foo.replace(/\./g, bar)", + r"foo.replace(/\\\./g, bar)", + r"foo.replace(/\|/g, bar)", + r#"foo.replace(/a/gu, bar)"#, + r#"foo.replace(/a/ug, bar)"#, + r#"foo.replace(/[a]/g, bar)"#, + r#"foo.replace(/a?/g, bar)"#, + r#"foo.replace(/.*/g, bar)"#, + r#"foo.replace(/a|b/g, bar)"#, + r"foo.replace(/\W/g, bar)", + r"foo.replace(/\u{61}/g, bar)", + r"foo.replace(/\u{61}/gu, bar)", + r"foo.replace(/\u{61}/gv, bar)", + r#"foo.replace(/]/g, "bar")"#, + r#"foo.replace(/a/gi, bar)"#, + r#"foo.replace(/a/gui, bar)"#, + r#"foo.replace(/a/uig, bar)"#, + r#"foo.replace(/a/vig, bar)"#, + // r#"const pattern = new RegExp("foo", "g"); foo.replace(pattern, bar)"#, + r#"foo.replace(new RegExp("foo", "g"), bar)"#, + r#"foo.replace(/a]/g, _)"#, + r#"foo.replace(/[a]/g, _)"#, + r#"foo.replace(/a{1/g, _)"#, + r#"foo.replace(/a{1}/g, _)"#, + r"foo.replace(/\u0022/g, _)", + r"foo.replace(/\u0027/g, _)", + r"foo.replace(/\cM\cj/g, _)", + r"foo.replace(/\x22/g, _)", + r"foo.replace(/\x27/g, _)", + r"foo.replace(/\uD83D\ude00/g, _)", + r"foo.replace(/\u{1f600}/gu, _)", + r"foo.replace(/\n/g, _)", + r"foo.replace(/\u{20}/gu, _)", + r"foo.replace(/\u{20}/gv, _)", + r#"foo.replaceAll(/a]/g, _)"#, + r"foo.replaceAll(/\r\n\u{1f600}/gu, _)", + r"foo.replaceAll(/\r\n\u{1f600}/gv, _)", + r#"foo.replaceAll(/a"#, + r#"foo.replace(/(?!a)+/g, "")"#, + ]; + + Tester::new_without_config(PreferStringReplaceAll::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/prefer_string_replace_all.snap b/crates/oxc_linter/src/snapshots/prefer_string_replace_all.snap new file mode 100644 index 000000000..55489ff84 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/prefer_string_replace_all.snap @@ -0,0 +1,245 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: prefer_string_replace_all +--- + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a/g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/"'/g, '\'') + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\./g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\\\./g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\|/g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a/gu, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a/ug, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/[a]/g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a?/g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/.*/g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a|b/g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\W/g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\u{61}/g, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\u{61}/gu, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\u{61}/gv, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/]/g, "bar") + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a/gi, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a/gui, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a/uig, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a/vig, bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(new RegExp("foo", "g"), bar) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a]/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/[a]/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a{1/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/a{1}/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\u0022/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\u0027/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\cM\cj/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\x22/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\x27/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\uD83D\ude00/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\u{1f600}/gu, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\n/g, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\u{20}/gu, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/\u{20}/gv, _) + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replaceAll(/a]/g, _) + · ────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replaceAll(/\r\n\u{1f600}/gu, _) + · ────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replaceAll(/\r\n\u{1f600}/gv, _) + · ────────── + ╰──── + + × Unterminated regular expression + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replaceAll(/a + · ── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ foo.replace(/(?!a)+/g, "") + · ─────── + ╰──── + +