From 67c6530527e6ba799b0f1dfd89a39108a00d6330 Mon Sep 17 00:00:00 2001 From: Zheyu Zhang Date: Tue, 23 Apr 2024 13:15:34 +0800 Subject: [PATCH 1/4] correctly handle comment placement for named import clause --- crates/biome_js_formatter/src/comments.rs | 55 ++++++++++++++++++++--- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/crates/biome_js_formatter/src/comments.rs b/crates/biome_js_formatter/src/comments.rs index 6dd3ffb9e70c..6841629d33d3 100644 --- a/crates/biome_js_formatter/src/comments.rs +++ b/crates/biome_js_formatter/src/comments.rs @@ -14,8 +14,8 @@ use biome_js_syntax::{ AnyJsClass, AnyJsName, AnyJsRoot, AnyJsStatement, JsArrayHole, JsArrowFunctionExpression, JsBlockStatement, JsCallArguments, JsCatchClause, JsEmptyStatement, JsFinallyClause, JsFormalParameter, JsFunctionBody, JsIdentifierBinding, JsIdentifierExpression, JsIfStatement, - JsLanguage, JsParameters, JsSyntaxKind, JsSyntaxNode, JsVariableDeclarator, JsWhileStatement, - TsInterfaceDeclaration, TsMappedType, + JsLanguage, JsNamedImportSpecifiers, JsParameters, JsSyntaxKind, JsSyntaxNode, + JsVariableDeclarator, JsWhileStatement, TsInterfaceDeclaration, TsMappedType, }; use biome_rowan::{AstNode, SyntaxNodeOptionExt, SyntaxTriviaPieceComments, TextLen}; use biome_suppression::parse_suppression_comment; @@ -118,7 +118,8 @@ impl CommentStyle for JsCommentStyle { .or_else(handle_mapped_type_comment) .or_else(handle_switch_default_case_comment) .or_else(handle_after_arrow_fat_arrow_comment) - .or_else(handle_import_export_specifier_comment), + .or_else(handle_import_export_specifier_comment) + .or_else(handle_import_named_clause_comments), CommentTextPosition::OwnLine => handle_member_expression_comment(comment) .or_else(handle_function_comment) .or_else(handle_if_statement_comment) @@ -135,7 +136,8 @@ impl CommentStyle for JsCommentStyle { .or_else(handle_continue_break_comment) .or_else(handle_union_type_comment) .or_else(handle_import_export_specifier_comment) - .or_else(handle_class_method_comment), + .or_else(handle_class_method_comment) + .or_else(handle_import_named_clause_comments), CommentTextPosition::SameLine => handle_if_statement_comment(comment) .or_else(handle_while_comment) .or_else(handle_for_comment) @@ -145,7 +147,8 @@ impl CommentStyle for JsCommentStyle { .or_else(handle_call_expression_comment) .or_else(handle_continue_break_comment) .or_else(handle_class_comment) - .or_else(handle_declare_comment), + .or_else(handle_declare_comment) + .or_else(handle_import_named_clause_comments), } } } @@ -1312,6 +1315,48 @@ fn handle_class_method_comment( } } +fn handle_import_named_clause_comments( + comment: DecoratedComment, +) -> CommentPlacement { + let enclosing_node = comment.enclosing_node(); + match enclosing_node.kind() { + JsSyntaxKind::JS_IMPORT_NAMED_CLAUSE => { + if let Some(import_specifiers) = comment + .preceding_node() + .and_then(JsNamedImportSpecifiers::cast_ref) + { + let specifier_list = import_specifiers.specifiers(); + // attach comments to the last specifier as trailing comments if comments are at the end of the line + // ```javascript + // import { a } from // comment + // "foo" + // ``` + if specifier_list.len() != 0 && comment.text_position() == CommentTextPosition::EndOfLine{ + if let Some(Ok(last_specifier)) = specifier_list.last() { + return CommentPlacement::trailing(last_specifier.into_syntax(), comment); + } + } else { + // attach comments to the import specifier as leading comments if comments are placed after the from keyword + // ```javascript + // import {} from // comment + // "foo" + // ``` + let is_after_from_keyword = comment + .following_token() + .map_or(true, |token| token.kind() != JsSyntaxKind::FROM_KW); + if is_after_from_keyword { + if let Some(following_node) = comment.following_node() { + return CommentPlacement::leading(following_node.clone(), comment); + } + } + } + } + CommentPlacement::Default(comment) + } + _ => CommentPlacement::Default(comment), + } +} + fn place_leading_statement_comment( statement: AnyJsStatement, comment: DecoratedComment, From e618c086c662f53e32b36543069e13dee7e24dd0 Mon Sep 17 00:00:00 2001 From: Zheyu Zhang Date: Tue, 23 Apr 2024 13:43:13 +0800 Subject: [PATCH 2/4] add test cases --- .../js/module/import/named_import_clause.js | 7 +++ .../module/import/named_import_clause.js.snap | 49 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 crates/biome_js_formatter/tests/specs/js/module/import/named_import_clause.js create mode 100644 crates/biome_js_formatter/tests/specs/js/module/import/named_import_clause.js.snap diff --git a/crates/biome_js_formatter/tests/specs/js/module/import/named_import_clause.js b/crates/biome_js_formatter/tests/specs/js/module/import/named_import_clause.js new file mode 100644 index 000000000000..55d55ea12207 --- /dev/null +++ b/crates/biome_js_formatter/tests/specs/js/module/import/named_import_clause.js @@ -0,0 +1,7 @@ +import {} from /* comment*/ 'foo' +import {} from // comment +'foo' + +import {a} from /* comment */'foo' +import {a} from // comment +'foo' \ No newline at end of file diff --git a/crates/biome_js_formatter/tests/specs/js/module/import/named_import_clause.js.snap b/crates/biome_js_formatter/tests/specs/js/module/import/named_import_clause.js.snap new file mode 100644 index 000000000000..042765c7f613 --- /dev/null +++ b/crates/biome_js_formatter/tests/specs/js/module/import/named_import_clause.js.snap @@ -0,0 +1,49 @@ +--- +source: crates/biome_formatter_test/src/snapshot_builder.rs +info: js/module/import/named_import_clause.js +--- +# Input + +```js +import {} from /* comment*/ 'foo' +import {} from // comment +'foo' + +import {a} from /* comment */'foo' +import {a} from // comment +'foo' +``` + + +============================= + +# Outputs + +## Output 1 + +----- +Indent style: Tab +Indent width: 2 +Line ending: LF +Line width: 80 +Quote style: Double Quotes +JSX quote style: Double Quotes +Quote properties: As needed +Trailing comma: All +Semicolons: Always +Arrow parentheses: Always +Bracket spacing: true +Bracket same line: false +Attribute Position: Auto +----- + +```js +import {} from /* comment*/ "foo"; +import {} from // comment +"foo"; + +import { a } from /* comment */ "foo"; +import { + a, // comment +} from "foo"; +``` From 0f784aacb5cfcc9c54e52d9ae2309776931b7def Mon Sep 17 00:00:00 2001 From: Zheyu Zhang Date: Tue, 23 Apr 2024 13:51:23 +0800 Subject: [PATCH 3/4] add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f29de93377e2..eb2e47f397b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b ### Formatter +#### Bug fixes + +- Correctly handle placement of comments inside named import clauses. [#2566](https://github.com/biomejs/biome/pull/2566). Contributed by @ah-yu + ### JavaScript APIs ### Linter From 57d861929dcb40d88358db10164878bbd657fa7c Mon Sep 17 00:00:00 2001 From: Zheyu Zhang Date: Tue, 23 Apr 2024 13:56:23 +0800 Subject: [PATCH 4/4] fmt --- crates/biome_js_formatter/src/comments.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/biome_js_formatter/src/comments.rs b/crates/biome_js_formatter/src/comments.rs index 6841629d33d3..e0242b91f5b8 100644 --- a/crates/biome_js_formatter/src/comments.rs +++ b/crates/biome_js_formatter/src/comments.rs @@ -1331,14 +1331,16 @@ fn handle_import_named_clause_comments( // import { a } from // comment // "foo" // ``` - if specifier_list.len() != 0 && comment.text_position() == CommentTextPosition::EndOfLine{ + if specifier_list.len() != 0 + && comment.text_position() == CommentTextPosition::EndOfLine + { if let Some(Ok(last_specifier)) = specifier_list.last() { return CommentPlacement::trailing(last_specifier.into_syntax(), comment); } } else { // attach comments to the import specifier as leading comments if comments are placed after the from keyword // ```javascript - // import {} from // comment + // import {} from // comment // "foo" // ``` let is_after_from_keyword = comment