From 17acece90b9ca769f5b4228a7e925311712df1ca Mon Sep 17 00:00:00 2001 From: dalaoshu Date: Tue, 25 Feb 2025 03:00:04 +0800 Subject: [PATCH] refactor(linter): improve `eslint/no-template-curly-in-string` (#9090) - Align rule logic with `eslint`, as `eslint` supports the detection of `"${{}"`. - Support dangerous fix. --- .../eslint/no_template_curly_in_string.rs | 77 ++++++++++--------- .../eslint_no_template_curly_in_string.snap | 35 +++++---- 2 files changed, 60 insertions(+), 52 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_template_curly_in_string.rs b/crates/oxc_linter/src/rules/eslint/no_template_curly_in_string.rs index 8f5d44eb96c9f..942d62a9af8d1 100644 --- a/crates/oxc_linter/src/rules/eslint/no_template_curly_in_string.rs +++ b/crates/oxc_linter/src/rules/eslint/no_template_curly_in_string.rs @@ -16,30 +16,38 @@ pub struct NoTemplateCurlyInString; declare_oxc_lint!( /// ### What it does - /// Disallow template literal placeholder syntax in regular strings + /// + /// Disallow template literal placeholder syntax in regular strings. This rule ensures that + /// expressions like `${variable}` are only used within template literals, avoiding incorrect + /// usage in regular strings. /// /// ### Why is this bad? - /// ECMAScript 6 allows programmers to create strings containing variable or - /// expressions using template literals, instead of string concatenation, by - /// writing expressions like `${variable}` between two backtick quotes. It - /// can be easy to use the wrong quotes when wanting to use template - /// literals, by writing `"${variable}"`, and end up with the literal value - /// `"${variable}"` instead of a string containing the value of the injected - /// expressions. /// - /// ### Example + /// ECMAScript 6 allows programmers to create strings containing variables or expressions using + /// template literals. This is done by embedding expressions like `${variable}` between backticks. + /// If regular quotes (`'` or `"`) are used with template literal syntax, it results in the literal + /// string `"${variable}"` instead of evaluating the expression. This rule helps to avoid this mistake, + /// ensuring that expressions are correctly evaluated inside template literals. + /// + /// ### Examples /// /// Examples of **incorrect** code for this rule: /// ```javascript - /// /*eslint no-template-curly-in-string: "error"*/ /// "Hello ${name}!"; /// 'Hello ${name}!'; /// "Time: ${12 * 60 * 60 * 1000}"; /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```javascript + /// `Hello ${name}!`; + /// `Time: ${12 * 60 * 60 * 1000}`; + /// templateFunction`Hello ${name}`; + /// ``` NoTemplateCurlyInString, eslint, style, - pending // TODO: conditional_fix + dangerous_fix ); impl Rule for NoTemplateCurlyInString { @@ -49,34 +57,14 @@ impl Rule for NoTemplateCurlyInString { }; let text = literal.value.as_str(); - if let Some(start_index) = text.find("${") { - let mut open_braces_count = 0; - let mut end_index = None; - - for (i, c) in text[start_index..].char_indices() { - let real_index = start_index + i; - if c == '{' { - open_braces_count += 1; - } else if c == '}' && open_braces_count > 0 { - open_braces_count -= 1; - if open_braces_count == 0 { - end_index = Some(real_index); - break; - } - } - } + let Some(start) = text.find("${") else { return }; - if let Some(end_index) = end_index { - let literal_span_start = literal.span.start + 1; - let match_start = u32::try_from(start_index) - .expect("Conversion from usize to u32 failed for match_start"); - let match_end = u32::try_from(end_index + 1) - .expect("Conversion from usize to u32 failed for match_end"); - ctx.diagnostic(no_template_curly_in_string_diagnostic(Span::new( - literal_span_start + match_start, - literal_span_start + match_end, - ))); - } + if text[start + 2..].contains('}') { + let template = format!("`{text}`"); + ctx.diagnostic_with_dangerous_fix( + no_template_curly_in_string_diagnostic(literal.span), + |fixer| fixer.replace(literal.span, template), + ); } } } @@ -105,6 +93,7 @@ fn test() { let fail = vec![ "'Hello, ${name}'", + "'Hello, ${{name}'", r#""Hello, ${name}""#, "'${greeting}, ${name}'", "'Hello, ${index + 1}'", @@ -113,6 +102,18 @@ fn test() { r#"'Hello, ${{foo: "bar"}.foo}'"#, ]; + let fix = vec![ + ("'Hello, ${name}'", "`Hello, ${name}`"), + ("'Hello, ${{name}'", "`Hello, ${{name}`"), + (r#""Hello, ${name}""#, r"`Hello, ${name}`"), + ("'${greeting}, ${name}'", "`${greeting}, ${name}`"), + ("'Hello, ${index + 1}'", "`Hello, ${index + 1}`"), + (r#"'Hello, ${name + " foo"}'"#, r#"`Hello, ${name + " foo"}`"#), + (r#"'Hello, ${name || "foo"}'"#, r#"`Hello, ${name || "foo"}`"#), + (r#"'Hello, ${{foo: "bar"}.foo}'"#, r#"`Hello, ${{foo: "bar"}.foo}`"#), + ]; + Tester::new(NoTemplateCurlyInString::NAME, NoTemplateCurlyInString::PLUGIN, pass, fail) + .expect_fix(fix) .test_and_snapshot(); } diff --git a/crates/oxc_linter/src/snapshots/eslint_no_template_curly_in_string.snap b/crates/oxc_linter/src/snapshots/eslint_no_template_curly_in_string.snap index 8374a34180c66..d1045349e3092 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_template_curly_in_string.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_template_curly_in_string.snap @@ -2,50 +2,57 @@ source: crates/oxc_linter/src/tester.rs --- ⚠ eslint(no-template-curly-in-string): Template placeholders will not interpolate in regular strings - ╭─[no_template_curly_in_string.tsx:1:9] + ╭─[no_template_curly_in_string.tsx:1:1] 1 │ 'Hello, ${name}' - · ─────── + · ──────────────── ╰──── help: Did you mean to use a template string literal? ⚠ eslint(no-template-curly-in-string): Template placeholders will not interpolate in regular strings - ╭─[no_template_curly_in_string.tsx:1:9] + ╭─[no_template_curly_in_string.tsx:1:1] + 1 │ 'Hello, ${{name}' + · ───────────────── + ╰──── + help: Did you mean to use a template string literal? + + ⚠ eslint(no-template-curly-in-string): Template placeholders will not interpolate in regular strings + ╭─[no_template_curly_in_string.tsx:1:1] 1 │ "Hello, ${name}" - · ─────── + · ──────────────── ╰──── help: Did you mean to use a template string literal? ⚠ eslint(no-template-curly-in-string): Template placeholders will not interpolate in regular strings - ╭─[no_template_curly_in_string.tsx:1:2] + ╭─[no_template_curly_in_string.tsx:1:1] 1 │ '${greeting}, ${name}' - · ─────────── + · ────────────────────── ╰──── help: Did you mean to use a template string literal? ⚠ eslint(no-template-curly-in-string): Template placeholders will not interpolate in regular strings - ╭─[no_template_curly_in_string.tsx:1:9] + ╭─[no_template_curly_in_string.tsx:1:1] 1 │ 'Hello, ${index + 1}' - · ──────────── + · ───────────────────── ╰──── help: Did you mean to use a template string literal? ⚠ eslint(no-template-curly-in-string): Template placeholders will not interpolate in regular strings - ╭─[no_template_curly_in_string.tsx:1:9] + ╭─[no_template_curly_in_string.tsx:1:1] 1 │ 'Hello, ${name + " foo"}' - · ──────────────── + · ───────────────────────── ╰──── help: Did you mean to use a template string literal? ⚠ eslint(no-template-curly-in-string): Template placeholders will not interpolate in regular strings - ╭─[no_template_curly_in_string.tsx:1:9] + ╭─[no_template_curly_in_string.tsx:1:1] 1 │ 'Hello, ${name || "foo"}' - · ──────────────── + · ───────────────────────── ╰──── help: Did you mean to use a template string literal? ⚠ eslint(no-template-curly-in-string): Template placeholders will not interpolate in regular strings - ╭─[no_template_curly_in_string.tsx:1:9] + ╭─[no_template_curly_in_string.tsx:1:1] 1 │ 'Hello, ${{foo: "bar"}.foo}' - · ─────────────────── + · ──────────────────────────── ╰──── help: Did you mean to use a template string literal?