From d6d4a4972113c7ff95530494831286e4304ea35d Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Fri, 1 Sep 2023 20:37:31 +0200 Subject: [PATCH] fix(lint/useTemplate): correctly handle comments --- CHANGELOG.md | 21 +++++++++++ .../src/analyzers/style/use_template.rs | 3 +- .../tests/specs/style/useTemplate/invalid.js | 5 +++ .../specs/style/useTemplate/invalid.js.snap | 35 ++++++++++++++++++- website/astro.config.ts | 15 ++++---- .../src/content/docs/internals/changelog.mdx | 21 +++++++++++ 6 files changed, 91 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 828d63d6d68d..ace11924a90f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,27 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom + `${a + b}px` ``` +- Fix [rome#4109](https://github.com/rome/tools/issues/4109) + + Previously, [useTemplate](https://biomejs.dev/lint/rules/useTemplate/) suggested an invalid code fix when a leading or trailing single-line comment was present: + + ```diff + // leading comment + - 1 /* inner comment */ + "+" + 2 // trailing comment + + `${// leading comment + + 1 /* inner comment */}+${2 //trailing comment}` // trailing comment + ``` + + Now, the rule correctly handle this case: + + ```diff + // leading comment + - 1 + "+" + 2 // trailing comment + + `${1}+${2}` // trailing comment + ``` + + As a sideeffect, the rule also suggests the removal of any inner comments. + - Fix [#106](https://github.com/biomejs/biome/issues/106) [noUndeclaredVariables](https://biomejs.dev/lint/rules/noUndeclaredVariables/) now correctly recognizes some TypeScript types such as `Uppercase`. diff --git a/crates/rome_js_analyze/src/analyzers/style/use_template.rs b/crates/rome_js_analyze/src/analyzers/style/use_template.rs index f86bce24cc95..4bf5993534a6 100644 --- a/crates/rome_js_analyze/src/analyzers/style/use_template.rs +++ b/crates/rome_js_analyze/src/analyzers/style/use_template.rs @@ -214,7 +214,8 @@ fn template_chuck_from(string_literal: &JsStringLiteralExpression) -> Option Option { Some(AnyJsTemplateElement::from(make::js_template_element( JsSyntaxToken::new_detached(JsSyntaxKind::DOLLAR_CURLY, "${", [], []), - expr.trim()?, + expr.with_leading_trivia_pieces([])? + .with_trailing_trivia_pieces([])?, make::token(T!['}']), ))) } diff --git a/crates/rome_js_analyze/tests/specs/style/useTemplate/invalid.js b/crates/rome_js_analyze/tests/specs/style/useTemplate/invalid.js index cf700d5635d6..4552eaea30bc 100644 --- a/crates/rome_js_analyze/tests/specs/style/useTemplate/invalid.js +++ b/crates/rome_js_analyze/tests/specs/style/useTemplate/invalid.js @@ -49,3 +49,8 @@ foo() + '\n'; const x = a + ("b") + c; ("a") + (b) + ("c"); + +//a +/*b*/ foo /*c*/ + /*d*/ 'baz' /*e*/ + /*f*/ 1 //g ++ //h +bar //i diff --git a/crates/rome_js_analyze/tests/specs/style/useTemplate/invalid.js.snap b/crates/rome_js_analyze/tests/specs/style/useTemplate/invalid.js.snap index efd1fe091446..bd1e452c8474 100644 --- a/crates/rome_js_analyze/tests/specs/style/useTemplate/invalid.js.snap +++ b/crates/rome_js_analyze/tests/specs/style/useTemplate/invalid.js.snap @@ -56,6 +56,11 @@ const x = a + ("b") + c; ("a") + (b) + ("c"); +//a +/*b*/ foo /*c*/ + /*d*/ 'baz' /*e*/ + /*f*/ 1 //g ++ //h +bar //i + ``` # Diagnostics @@ -408,7 +413,7 @@ invalid.js:27:1 lint/style/useTemplate FIXABLE ━━━━━━━━━━ 25 25 │ foo() + '\n'; 26 26 │ 27 │ - 1·*·/**leading*/'foo'····/**trailing·*/···················+·'bar'; - 27 │ + `${1·*·/**leading*/'foo'····/**trailing·*/}bar`; + 27 │ + `${1·*·/**leading*/'foo'}bar`; 28 28 │ 29 29 │ // strings including `${` @@ -641,6 +646,7 @@ invalid.js:51:1 lint/style/useTemplate FIXABLE ━━━━━━━━━━ > 51 │ ("a") + (b) + ("c"); │ ^^^^^^^^^^^^^^^^^^^ 52 │ + 53 │ //a i Suggested fix: Use a template literal. @@ -649,6 +655,33 @@ invalid.js:51:1 lint/style/useTemplate FIXABLE ━━━━━━━━━━ 51 │ - ("a")·+·(b)·+·("c"); 51 │ + `a${b}c`; 52 52 │ + 53 53 │ //a + + +``` + +``` +invalid.js:54:7 lint/style/useTemplate FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Template literals are preferred over string concatenation. + + 53 │ //a + > 54 │ /*b*/ foo /*c*/ + /*d*/ 'baz' /*e*/ + /*f*/ 1 //g + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 55 │ + //h + > 56 │ bar //i + │ ^^^ + 57 │ + + i Suggested fix: Use a template literal. + + 52 52 │ + 53 53 │ //a + 54 │ - /*b*/·foo·/*c*/·+·/*d*/·'baz'·/*e*/·+·/*f*/·1·//g + 55 │ - +·//h + 56 │ - bar·//i + 54 │ + /*b*/·`${foo}baz${1}${bar}`·//i + 57 55 │ ``` diff --git a/website/astro.config.ts b/website/astro.config.ts index 2e507bcb36da..94921f22ac2c 100644 --- a/website/astro.config.ts +++ b/website/astro.config.ts @@ -143,9 +143,7 @@ function inlineIntegration(): AstroIntegration { }; } - - -const site = "https://biomejs.dev" +const site = "https://biomejs.dev"; // https://astro.build/config export default defineConfig({ site, @@ -227,12 +225,15 @@ export default defineConfig({ }, }, { - tag: 'meta', - attrs: { property: 'og:image', content: site + '/img/og.png?v=1' }, + tag: "meta", + attrs: { property: "og:image", content: `${site}/img/og.png?v=1` }, }, { - tag: 'meta', - attrs: { property: 'twitter:image', content: site + '/img/og.png?v=1' }, + tag: "meta", + attrs: { + property: "twitter:image", + content: `${site}/img/og.png?v=1`, + }, }, ], customCss: [ diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index b6a40805bbd6..49289568afca 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -74,6 +74,27 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom + `${a + b}px` ``` +- Fix [rome#4109](https://github.com/rome/tools/issues/4109) + + Previously, [useTemplate](https://biomejs.dev/lint/rules/useTemplate/) suggested an invalid code fix when a leading or trailing single-line comment was present: + + ```diff + // leading comment + - 1 /* inner comment */ + "+" + 2 // trailing comment + + `${// leading comment + + 1 /* inner comment */}+${2 //trailing comment}` // trailing comment + ``` + + Now, the rule correctly handle this case: + + ```diff + // leading comment + - 1 + "+" + 2 // trailing comment + + `${1}+${2}` // trailing comment + ``` + + As a sideeffect, the rule also suggests the removal of any inner comments. + - Fix [#106](https://github.com/biomejs/biome/issues/106) [noUndeclaredVariables](https://biomejs.dev/lint/rules/noUndeclaredVariables/) now correctly recognizes some TypeScript types such as `Uppercase`.