fix(linter) fix incorrect report in prefer-string-replace-all (#1796)

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

@@ -4,17 +4,22 @@ use oxc_ast::{
 };
 use oxc_diagnostics::{
     miette::{self, Diagnostic},
-    thiserror::Error,
+    thiserror::{self, Error},
 };
 use oxc_macros::declare_oxc_lint;
-use oxc_span::Span;
+use oxc_span::{Atom, 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()`.")]
+enum PreferStringReplaceAllDiagnostic {
+    #[error("eslint-plugin-unicorn(prefer-string-replace-all): This pattern can be replaced with `{1}`.")]
     #[diagnostic(severity(warning))]
-struct PreferStringReplaceAllDiagnostic(#[label] pub Span);
+    StringLiteral(#[label] Span, Atom),
+    #[error("eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()` when using a regex with the global flag.")]
+    #[diagnostic(severity(warning))]
+    UseReplaceAll(#[label] Span),
+}
 
 #[derive(Debug, Default, Clone)]
 pub struct PreferStringReplaceAll;
@@ -45,7 +50,9 @@ impl Rule for PreferStringReplaceAll {
             return;
         };
 
-        if !matches!(static_member_expr.property.name.as_str(), "replace" | "replaceAll") {
+        let method_name_str = static_member_expr.property.name.as_str();
+
+        if !matches!(method_name_str, "replace" | "replaceAll") {
             return;
         }
 
@@ -55,11 +62,22 @@ impl Rule for PreferStringReplaceAll {
 
         let Argument::Expression(pattern) = &call_expr.arguments[0] else { return };
 
-        if !is_reg_exp_with_global_flag(pattern) {
+        match method_name_str {
-            return;
+            "replaceAll" => {
+                if let Some(k) = get_pattern_replacement(pattern) {
+                    ctx.diagnostic(PreferStringReplaceAllDiagnostic::StringLiteral(
+                        static_member_expr.property.span,
+                        k,
+                    ));
+                }
+            }
+            "replace" if is_reg_exp_with_global_flag(pattern) => {
+                ctx.diagnostic(PreferStringReplaceAllDiagnostic::UseReplaceAll(
+                    static_member_expr.property.span,
+                ));
+            }
+            _ => {}
         }
-
-        ctx.diagnostic(PreferStringReplaceAllDiagnostic(static_member_expr.property.span));
     }
 }
 
@@ -81,6 +99,25 @@ fn is_reg_exp_with_global_flag<'a>(expr: &'a Expression<'a>) -> bool {
     false
 }
 
+fn get_pattern_replacement<'a>(expr: &'a Expression<'a>) -> Option<Atom> {
+    let Expression::RegExpLiteral(reg_exp_literal) = expr else { return None };
+
+    if !reg_exp_literal.regex.flags.contains(RegExpFlags::G) {
+        return None;
+    }
+
+    if !is_simple_string(&reg_exp_literal.regex.pattern) {
+        return None;
+    }
+
+    Some(reg_exp_literal.regex.pattern.clone())
+}
+
+fn is_simple_string(str: &str) -> bool {
+    str.chars()
+        .all(|c| !matches!(c, '^' | '$' | '+' | '[' | '{' | '(' | '\\' | '.' | '?' | '*' | '|'))
+}
+
 #[test]
 fn test() {
     use crate::tester::Tester;
@@ -158,10 +195,14 @@ fn test() {
         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, _)",
+        // we need a regex parser to handle this
-        r"foo.replaceAll(/\r\n\u{1f600}/gv, _)",
+        // r"foo.replaceAll(/\r\n\u{1f600}/gu, _)",
-        r"foo.replaceAll(/a",
+        // r"foo.replaceAll(/\r\n\u{1f600}/gv, _)",
+        r"foo.replaceAll(/a very very very very very very very very very very very very very very very very very very very very very very very very very very very very very long string/g, _)",
         r#"foo.replace(/(?!a)+/g, "")"#,
+        // https://github.com/oxc-project/oxc/issues/1790
+        // report error as `/world/g` can be replaced with string literal
+        r#""Hello world".replaceAll(/world/g, 'world!');"#,
     ];
 
     Tester::new_without_config(PreferStringReplaceAll::NAME, pass, fail).test_and_snapshot();

crates/oxc_linter/src/snapshots/prefer_string_replace_all.snap

@@ -2,244 +2,239 @@
 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()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a/g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/"'/g, '\'')
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\./g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\\\./g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\|/g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a/gu, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a/ug, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/[a]/g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a?/g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/.*/g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a|b/g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\W/g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\u{61}/g, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\u{61}/gu, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\u{61}/gv, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/]/g, "bar")
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a/gi, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a/gui, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a/uig, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a/vig, bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(new RegExp("foo", "g"), bar)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a]/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/[a]/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a{1/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/a{1}/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\u0022/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\u0027/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\cM\cj/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\x22/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\x27/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\uD83D\ude00/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\u{1f600}/gu, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\n/g, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\u{20}/gu, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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:1]
  1 │ foo.replace(/\u{20}/gv, _)
    ·     ───────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): This pattern can be replaced with `a]`.
    ╭─[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()`.
+  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): This pattern can be replaced with `a very very very very very very very very very very very very very very very very very very very very very
+  │ very very very very very very very very long string`.
    ╭─[prefer_string_replace_all.tsx:1:1]
- 1 │ foo.replaceAll(/\r\n\u{1f600}/gu, _)
+ 1 │ foo.replaceAll(/a very very very very very very very very very very very very very very very very very very very very very very very very very very very very very long string/g, _)
    ·     ──────────
    ╰────
 
-  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): Prefer `String#replaceAll()` over `String#replace()`.
+  ⚠ eslint-plugin-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: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, "")
    ·     ───────
    ╰────
 
+  ⚠ eslint-plugin-unicorn(prefer-string-replace-all): This pattern can be replaced with `world`.
+   ╭─[prefer_string_replace_all.tsx:1:1]
+ 1 │ "Hello world".replaceAll(/world/g, 'world!');
+   ·               ──────────
+   ╰────
+