Skip to content

Commit

Permalink
fix(js_formatter): correctly handle placement of comments inside name…
Browse files Browse the repository at this point in the history
…d import clause (biomejs#2566)
  • Loading branch information
ah-yu authored Apr 23, 2024
1 parent 52029f6 commit 65a61d7
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
57 changes: 52 additions & 5 deletions crates/biome_js_formatter/src/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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),
}
}
}
Expand Down Expand Up @@ -1312,6 +1315,50 @@ fn handle_class_method_comment(
}
}

fn handle_import_named_clause_comments(
comment: DecoratedComment<JsLanguage>,
) -> CommentPlacement<JsLanguage> {
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<JsLanguage>,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import {} from /* comment*/ 'foo'
import {} from // comment
'foo'

import {a} from /* comment */'foo'
import {a} from // comment
'foo'
Original file line number Diff line number Diff line change
@@ -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";
```

0 comments on commit 65a61d7

Please sign in to comment.