fix(linter/unicorn/prefer-string-replace-all): resolve regex variables (#27769)

Unicorn reports `s.replace(RE, "b")` when `RE` holds a global regex, but Oxlint only checked inline patterns. Resolve stable regex bindings and aliases while preserving variable arguments in the fix. Use `run` for calls and variable declarations, following regex bindings' references without walking every AST node. Skip reassigned, redeclared, or property-mutated bindings.

Addresses report 17 (`unicorn/prefer-string-replace-all`) of #27746.

crates/oxc_linter/src/generated/rule_runner_impls.rs

@@ -3923,7 +3923,7 @@ impl RuleRunner for crate::rules::unicorn::prefer_string_raw::PreferStringRaw {
 
 impl RuleRunner for crate::rules::unicorn::prefer_string_replace_all::PreferStringReplaceAll {
     const NODE_TYPES: Option<&AstTypesBitset> =
-        Some(&AstTypesBitset::from_types(&[AstType::CallExpression]));
+        Some(&AstTypesBitset::from_types(&[AstType::CallExpression, AstType::VariableDeclarator]));
     const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run;
 }
 

crates/oxc_linter/src/rule.rs

@@ -775,6 +775,10 @@ mod test {
             &unicorn::consistent_assert::ConsistentAssert,
             &[ImportDeclaration],
         );
+        assert_rule_runs_on_node_types(
+            &unicorn::prefer_string_replace_all::PreferStringReplaceAll,
+            &[CallExpression, VariableDeclarator],
+        );
         assert_rule_runs_on_node_types(
             &promise::always_return::AlwaysReturn::default(),
             &[Function, ArrowFunctionExpression],

crates/oxc_linter/src/rules/unicorn/prefer_string_replace_all.rs

@@ -1,16 +1,24 @@
 use oxc_ast::{
     AstKind,
-    ast::{Argument, MemberExpression, RegExpFlags},
+    ast::{
+        Argument, CallExpression, Expression, MemberExpression, RegExpFlags, VariableDeclarator,
+    },
 };
 use oxc_codegen::CodegenOptions;
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
 use oxc_regular_expression::ast::Term;
+use oxc_semantic::SymbolId;
 use oxc_span::{GetSpan, Span};
 use oxc_str::CompactStr;
 
 use crate::{
-    AstNode, ast_util::extract_regex_flags, context::LintContext, fixer::RuleFixer, rule::Rule,
+    AstNode,
+    ast_util::{extract_regex_flags, iter_outer_expressions},
+    context::LintContext,
+    fixer::RuleFixer,
+    rule::Rule,
+    utils::is_regexp_callee,
 };
 
 fn string_literal(span: Span, replacement: &str) -> OxcDiagnostic {
@@ -62,10 +70,26 @@ declare_oxc_lint!(
 
 impl Rule for PreferStringReplaceAll {
     fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
-        let AstKind::CallExpression(call_expr) = node.kind() else {
-            return;
-        };
+        match node.kind() {
+            AstKind::CallExpression(call) => Self::check_call(call, ctx, false),
+            AstKind::VariableDeclarator(declarator) => {
+                if let Some(init) = &declarator.init
+                    && is_reg_exp_with_global_flag(init, ctx)
+                {
+                    Self::check_variable(declarator, ctx);
+                }
+            }
+            _ => {}
+        }
+    }
+}
 
+impl PreferStringReplaceAll {
+    fn check_call<'a>(
+        call_expr: &CallExpression<'a>,
+        ctx: &LintContext<'a>,
+        resolved_pattern: bool,
+    ) {
         let Some(member_expr) = call_expr.callee.get_member_expr() else {
             return;
         };
@@ -94,7 +118,12 @@ impl Rule for PreferStringReplaceAll {
                     });
                 }
             }
-            "replace" if is_reg_exp_with_global_flag(pattern) => {
+            "replace"
+                if resolved_pattern
+                    || pattern
+                        .as_expression()
+                        .is_some_and(|expr| is_reg_exp_with_global_flag(expr, ctx)) =>
+            {
                 let diagnostic = use_replace_all(static_member_expr.property.span);
 
                 if let Some(k) = get_pattern_replacement(pattern) {
@@ -116,6 +145,40 @@ impl Rule for PreferStringReplaceAll {
             _ => {}
         }
     }
+
+    fn check_variable<'a>(declarator: &VariableDeclarator<'a>, ctx: &LintContext<'a>) {
+        let Some(identifier) = declarator.id.get_binding_identifier() else {
+            return;
+        };
+        let mut pending: smallvec::SmallVec<[SymbolId; 4]> =
+            smallvec::smallvec![identifier.symbol_id()];
+        while let Some(symbol_id) = pending.pop() {
+            if binding_has_writes(symbol_id, ctx) {
+                continue;
+            }
+            for reference in ctx.scoping().get_resolved_references(symbol_id) {
+                let reference_span = ctx.nodes().get_node(reference.node_id()).span();
+                match iter_outer_expressions(ctx.nodes(), reference.node_id()).next() {
+                    Some(AstKind::CallExpression(call)) => {
+                        if call.arguments.first().and_then(Argument::as_expression).is_some_and(
+                            |pattern| pattern.get_inner_expression().span() == reference_span,
+                        ) {
+                            Self::check_call(call, ctx, true);
+                        }
+                    }
+                    Some(AstKind::VariableDeclarator(alias)) => {
+                        if alias.init.as_ref().is_some_and(|init| {
+                            init.get_inner_expression().span() == reference_span
+                        }) && let Some(identifier) = alias.id.get_binding_identifier()
+                        {
+                            pending.push(identifier.symbol_id());
+                        }
+                    }
+                    _ => {}
+                }
+            }
+        }
+    }
 }
 
 fn generate_string_literal(fixer: RuleFixer<'_, '_>, value: &str) -> String {
@@ -125,22 +188,47 @@ fn generate_string_literal(fixer: RuleFixer<'_, '_>, value: &str) -> String {
     codegen.into_source_text()
 }
 
-fn is_reg_exp_with_global_flag<'a>(expr: &'a Argument<'a>) -> bool {
-    if let Argument::RegExpLiteral(reg_exp_literal) = expr {
-        return reg_exp_literal.regex.flags.contains(RegExpFlags::G);
-    }
-
-    if let Argument::NewExpression(new_expr) = expr {
-        if !new_expr.callee.is_specific_id("RegExp") {
-            return false;
+fn is_reg_exp_with_global_flag<'a>(expr: &Expression<'a>, ctx: &LintContext<'a>) -> bool {
+    match expr.get_inner_expression() {
+        Expression::RegExpLiteral(reg_exp_literal) => {
+            reg_exp_literal.regex.flags.contains(RegExpFlags::G)
         }
-
-        if let Some(flags) = extract_regex_flags(&new_expr.arguments) {
-            return flags.contains(RegExpFlags::G);
+        Expression::NewExpression(new_expr) => {
+            is_regexp_callee(&new_expr.callee, ctx)
+                && !matches!(new_expr.arguments.first(), Some(Argument::SpreadElement(_)))
+                && extract_regex_flags(&new_expr.arguments)
+                    .is_some_and(|flags| flags.contains(RegExpFlags::G))
         }
+        _ => false,
     }
+}
 
-    false
+fn binding_has_writes(symbol_id: SymbolId, ctx: &LintContext<'_>) -> bool {
+    !ctx.scoping().symbol_redeclarations(symbol_id).is_empty()
+        || ctx.scoping().get_resolved_references(symbol_id).any(|reference| {
+            let reference_span = ctx.nodes().get_node(reference.node_id()).span();
+            reference.is_write()
+                || iter_outer_expressions(ctx.nodes(), reference.node_id())
+                    .find(|kind| match kind {
+                        AstKind::StaticMemberExpression(member) => {
+                            !member.object.span().contains_inclusive(reference_span)
+                        }
+                        AstKind::ComputedMemberExpression(member) => {
+                            !member.object.span().contains_inclusive(reference_span)
+                        }
+                        AstKind::PrivateFieldExpression(member) => {
+                            !member.object.span().contains_inclusive(reference_span)
+                        }
+                        _ => true,
+                    })
+                    .is_some_and(|kind| match kind {
+                        AstKind::AssignmentExpression(assignment) => {
+                            assignment.left.span().contains_inclusive(reference_span)
+                        }
+                        AstKind::UpdateExpression(_) => true,
+                        _ => false,
+                    })
+        })
 }
 
 fn get_pattern_replacement<'a>(expr: &'a Argument<'a>) -> Option<CompactStr> {
@@ -229,6 +317,22 @@ fn test() {
         r"foo.replaceAll(/foo/gm, bar)",
         r"foo.replaceAll(/foo/gs, bar)",
         r"foo.replaceAll(/foo/gim, bar)",
+        r"const pattern = /a/; foo.replace(pattern, bar)",
+        r"let pattern = /a/g; pattern = /a/; foo.replace(pattern, bar)",
+        r"const pattern = /a/g; foo.replaceAll(pattern, bar)",
+        r"const pattern = /a/g; function f(pattern) { foo.replace(pattern, bar); }",
+        r"const a = b; const b = a; foo.replace(a, bar)",
+        r"const {pattern} = /a/g; foo.replace(pattern, bar)",
+        r#"function f(RegExp) { const pattern = new RegExp("foo", "g"); foo.replace(pattern, bar); }"#,
+        r"let pattern = /a/g; const alias = pattern; pattern = /a/; foo.replace(alias, bar)",
+        r#"var RE = /a/g; var RE = /b/; "aba".replace(RE, "x")"#,
+        r#"const RE = /a/g; RE[Symbol.replace] = undefined; "x/a/gx/a/g".replace(RE, "b")"#,
+        r"const RE = /a/g; RE.lastIndex++; foo.replace(RE, bar)",
+        r"const RE = /a/g; const alias = RE; RE[Symbol.replace] = undefined; foo.replace(alias, bar)",
+        r#"const RE = new RegExp(...patterns, "g"); foo.replace(RE, bar)"#,
+        r"const RE = /a/g; foo.replace(unknown, RE)",
+        r"const RE = /a/g; foo.replace(...RE, bar)",
+        r"const RE = /a/g; const alias = RE.source; foo.replace(alias, bar)",
     ];
 
     let fail = vec![
@@ -252,7 +356,6 @@ fn test() {
         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, _)",
@@ -281,6 +384,16 @@ fn test() {
         // u/v flags are allowed, so replaceAll should still suggest string replacement
         r"foo.replaceAll(/foo/gu, bar)",
         r"foo.replaceAll(/foo/gv, bar)",
+        r#"const RE = /a/g;
+export const out = s.replace(RE, "b");
+export const out2 = s.replace(/a/g, "b");"#,
+        r#"const pattern = new RegExp("foo", "g"); foo.replace(pattern, bar)"#,
+        r"let pattern = /a/g; foo.replace(pattern, bar)",
+        r"var pattern = /a/g; foo.replace(pattern, bar)",
+        r"const pattern = (/a/g); foo.replace((pattern), bar)",
+        r"const pattern = /a/g; const alias = pattern; foo.replace(alias, bar)",
+        r"const pattern = /a/g; obj[pattern] = value; foo.replace(pattern, bar)",
+        r"const RE = /a/g; const first = RE; const second = (first); foo.replace(first, bar); foo.replace(second, bar); foo.replace(RE, bar)",
     ];
 
     let fix = vec![
@@ -410,10 +523,18 @@ fn test() {
         //     r"foo.replaceAll(/a/gms, bar)",
         //     r"foo.replaceAll('a', bar)",
         // ),
-        // (
-        //     r#"const pattern = new RegExp("foo", "g"); foo.replace(pattern, bar)"#,
-        //     r#"const pattern = new RegExp("foo", "g"); foo.replaceAll(pattern, bar)"#,
-        // ),
+        (
+            r#"const pattern = new RegExp("foo", "g"); foo.replace(pattern, bar)"#,
+            r#"const pattern = new RegExp("foo", "g"); foo.replaceAll(pattern, bar)"#,
+        ),
+        (
+            r"const pattern = /a/g; foo.replace(pattern, bar)",
+            r"const pattern = /a/g; foo.replaceAll(pattern, bar)",
+        ),
+        (
+            r"const pattern = /a/g; const alias = pattern; foo.replace(alias, bar)",
+            r"const pattern = /a/g; const alias = pattern; foo.replaceAll(alias, bar)",
+        ),
         // (
         //     r"foo.replace(/a{1}/g, _)",
         //     r"foo.replaceAll('a', _)",
@@ -574,6 +695,10 @@ fn test() {
         //     r#"foo.split(/a/gi).join("b")"#,
         //     r"foo.replaceAll(/a/gi, 'b')",
         // ),
+        (
+            r"const RE = /a/g; const first = RE; const second = (first); foo.replace(first, bar); foo.replace(second, bar); foo.replace(RE, bar)",
+            r"const RE = /a/g; const first = RE; const second = (first); foo.replaceAll(first, bar); foo.replaceAll(second, bar); foo.replaceAll(RE, bar)",
+        ),
     ];
 
     Tester::new(PreferStringReplaceAll::NAME, PreferStringReplaceAll::PLUGIN, pass, fail)

crates/oxc_linter/src/snapshots/unicorn_prefer_string_replace_all.snap

@@ -288,3 +288,83 @@ source: crates/oxc_linter/src/tester.rs
    ·                ───────
    ╰────
   help: Replace `/foo/gv` with `'foo'`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:2:22]
+ 1 │ const RE = /a/g;
+ 2 │ export const out = s.replace(RE, "b");
+   ·                      ───────
+ 3 │ export const out2 = s.replace(/a/g, "b");
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:3:23]
+ 2 │ export const out = s.replace(RE, "b");
+ 3 │ export const out2 = s.replace(/a/g, "b");
+   ·                       ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:1:45]
+ 1 │ const pattern = new RegExp("foo", "g"); foo.replace(pattern, bar)
+   ·                                             ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:1:25]
+ 1 │ let pattern = /a/g; foo.replace(pattern, bar)
+   ·                         ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:1:25]
+ 1 │ var pattern = /a/g; foo.replace(pattern, bar)
+   ·                         ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:1:29]
+ 1 │ const pattern = (/a/g); foo.replace((pattern), bar)
+   ·                             ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:1:50]
+ 1 │ const pattern = /a/g; const alias = pattern; foo.replace(alias, bar)
+   ·                                                  ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:1:49]
+ 1 │ const pattern = /a/g; obj[pattern] = value; foo.replace(pattern, bar)
+   ·                                                 ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:1:115]
+ 1 │ const RE = /a/g; const first = RE; const second = (first); foo.replace(first, bar); foo.replace(second, bar); foo.replace(RE, bar)
+   ·                                                                                                                   ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:1:64]
+ 1 │ const RE = /a/g; const first = RE; const second = (first); foo.replace(first, bar); foo.replace(second, bar); foo.replace(RE, bar)
+   ·                                                                ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.
+
+  ⚠ unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.
+   ╭─[prefer_string_replace_all.tsx:1:89]
+ 1 │ const RE = /a/g; const first = RE; const second = (first); foo.replace(first, bar); foo.replace(second, bar); foo.replace(RE, bar)
+   ·                                                                                         ───────
+   ╰────
+  help: Replace `replace` with `replaceAll`.

tasks/track_linter_timings/linter_timings.snap

@@ -663,7 +663,7 @@ unicorn/prefer-set-size                                    |  76337
 unicorn/prefer-single-call                                 |  24802
 unicorn/prefer-spread                                      |  62606
 unicorn/prefer-string-raw                                  |  62460
-unicorn/prefer-string-replace-all                          |  62606
+unicorn/prefer-string-replace-all                          | 100105
 unicorn/prefer-string-slice                                |  62606
 unicorn/prefer-string-starts-ends-with                     |  62606
 unicorn/prefer-string-trim-start-end                       |  62606