From 0b189ea55206208d3b05769136f589c48ee977f8 Mon Sep 17 00:00:00 2001 From: shulaoda Date: Sat, 15 Feb 2025 00:24:41 +0800 Subject: [PATCH 1/2] refactor(linter): improve eslint/no-template-curly-in-string --- .../eslint/no_template_curly_in_string.rs | 79 +++++++++++-------- .../eslint_no_template_curly_in_string.snap | 35 ++++---- 2 files changed, 68 insertions(+), 46 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 6e1a301fcc8b6..2935ff1521bec 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 @@ -1,6 +1,8 @@ +use oxc_allocator::Allocator; use oxc_ast::AstKind; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; +use oxc_parser::{ParseOptions, Parser}; use oxc_span::Span; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -16,30 +18,37 @@ pub struct NoTemplateCurlyInString; declare_oxc_lint!( /// ### What it does + /// /// Disallow template literal placeholder syntax 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. + /// 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 + /// ### 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 + conditional_fix ); impl Rule for NoTemplateCurlyInString { @@ -49,34 +58,28 @@ 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; + let Some(start) = text.find("${") else { return }; - 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; - } - } - } + if text[start + 2..].contains('}') { + let template = format!("`{text}`"); + let allocator = Allocator::new(); + let ret = Parser::new(&allocator, template.as_str(), *ctx.source_type()) + .with_options(ParseOptions { + parse_regular_expression: true, + allow_return_outside_function: true, + ..ParseOptions::default() + }) + .parse(); - 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 !ret.panicked { + ctx.diagnostic_with_fix( + no_template_curly_in_string_diagnostic(literal.span), + |fixer| fixer.replace(literal.span, template), + ); + return; } + + ctx.diagnostic(no_template_curly_in_string_diagnostic(literal.span)); } } } @@ -105,6 +108,7 @@ fn test() { let fail = vec![ "'Hello, ${name}'", + "'Hello, ${{name}'", r#""Hello, ${name}""#, "'${greeting}, ${name}'", "'Hello, ${index + 1}'", @@ -113,6 +117,17 @@ fn test() { r#"'Hello, ${{foo: "bar"}.foo}'"#, ]; + let fix = vec![ + ("'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? From 06b82279017c22be16cc60ac13ca4651879543f5 Mon Sep 17 00:00:00 2001 From: shulaoda Date: Tue, 25 Feb 2025 01:29:34 +0800 Subject: [PATCH 2/2] refactor: use dangerous_fix instead --- .../eslint/no_template_curly_in_string.rs | 42 +++++++------------ 1 file changed, 14 insertions(+), 28 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 2935ff1521bec..86e38c3a631ef 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 @@ -1,8 +1,6 @@ -use oxc_allocator::Allocator; use oxc_ast::AstKind; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; -use oxc_parser::{ParseOptions, Parser}; use oxc_span::Span; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -19,16 +17,17 @@ 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. + /// 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 /// @@ -48,7 +47,7 @@ declare_oxc_lint!( NoTemplateCurlyInString, eslint, style, - conditional_fix + dangerous_fix ); impl Rule for NoTemplateCurlyInString { @@ -62,24 +61,10 @@ impl Rule for NoTemplateCurlyInString { if text[start + 2..].contains('}') { let template = format!("`{text}`"); - let allocator = Allocator::new(); - let ret = Parser::new(&allocator, template.as_str(), *ctx.source_type()) - .with_options(ParseOptions { - parse_regular_expression: true, - allow_return_outside_function: true, - ..ParseOptions::default() - }) - .parse(); - - if !ret.panicked { - ctx.diagnostic_with_fix( - no_template_curly_in_string_diagnostic(literal.span), - |fixer| fixer.replace(literal.span, template), - ); - return; - } - - ctx.diagnostic(no_template_curly_in_string_diagnostic(literal.span)); + ctx.diagnostic_with_dangerous_fix( + no_template_curly_in_string_diagnostic(literal.span), + |fixer| fixer.replace(literal.span, template), + ); } } } @@ -119,6 +104,7 @@ fn test() { 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}`"),