From 3102b22c3274b3149efc271cdcfb020fb9066ea3 Mon Sep 17 00:00:00 2001 From: Cameron Date: Sun, 24 Dec 2023 01:37:32 +0000 Subject: [PATCH] fix(linter) fix incorrect report in `prefer-string-replace-all` (#1796) --- .../unicorn/prefer_string_replace_all.rs | 67 ++++++++++--- .../snapshots/prefer_string_replace_all.snap | 97 +++++++++---------- 2 files changed, 100 insertions(+), 64 deletions(-) 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 index b87d60bc0..67dadbdca 100644 --- a/crates/oxc_linter/src/rules/unicorn/prefer_string_replace_all.rs +++ b/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()`.")] -#[diagnostic(severity(warning))] -struct PreferStringReplaceAllDiagnostic(#[label] pub Span); +enum PreferStringReplaceAllDiagnostic { + #[error("eslint-plugin-unicorn(prefer-string-replace-all): This pattern can be replaced with `{1}`.")] + #[diagnostic(severity(warning))] + 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) { - return; + match method_name_str { + "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 { + 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(®_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, _)", - r"foo.replaceAll(/\r\n\u{1f600}/gv, _)", - r"foo.replaceAll(/a", + // we need a regex parser to handle this + // r"foo.replaceAll(/\r\n\u{1f600}/gu, _)", + // 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(); diff --git a/crates/oxc_linter/src/snapshots/prefer_string_replace_all.snap b/crates/oxc_linter/src/snapshots/prefer_string_replace_all.snap index 55489ff84..1dbd66226 100644 --- a/crates/oxc_linter/src/snapshots/prefer_string_replace_all.snap +++ b/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()`. - ╭─[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()`. + ⚠ 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): This pattern can be replaced with `world`. + ╭─[prefer_string_replace_all.tsx:1:1] + 1 │ "Hello world".replaceAll(/world/g, 'world!'); + · ────────── + ╰──── +