From b7390da1c155f0c66007af1171b691630cd3bdc8 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Thu, 11 Jan 2024 11:51:14 +0100 Subject: [PATCH] fix(js_formatter): placements of line comments between function parentheses and body (#1485) --- CHANGELOG.md | 4 + crates/biome_js_formatter/src/comments.rs | 42 +++-- .../js/module/function/function_comments.js | 26 ++- .../module/function/function_comments.js.snap | 43 ++++- .../between-parentheses-and-function-body.js | 32 ++-- ...ween-parentheses-and-function-body.js.snap | 152 ------------------ .../src/content/docs/internals/changelog.mdx | 4 + 7 files changed, 105 insertions(+), 198 deletions(-) delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fcb2e6419f1..7c9851fe2b8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom ### Formatter +#### Bug fixes + +- Fix [#1172](https://github.com/biomejs/biome/issues/1172). Fix placement of line comment after function expression parentheses, they are now attached to first statement in body. Contributed by @kalleep + ### JavaScript APIs ### Linter diff --git a/crates/biome_js_formatter/src/comments.rs b/crates/biome_js_formatter/src/comments.rs index 10dc33511702..1f4803259379 100644 --- a/crates/biome_js_formatter/src/comments.rs +++ b/crates/biome_js_formatter/src/comments.rs @@ -100,7 +100,7 @@ impl CommentStyle for JsCommentStyle { ) -> CommentPlacement { match comment.text_position() { CommentTextPosition::EndOfLine => handle_typecast_comment(comment) - .or_else(handle_function_declaration_comment) + .or_else(handle_function_comment) .or_else(handle_conditional_comment) .or_else(handle_if_statement_comment) .or_else(handle_while_comment) @@ -119,7 +119,7 @@ impl CommentStyle for JsCommentStyle { .or_else(handle_after_arrow_fat_arrow_comment) .or_else(handle_import_export_specifier_comment), CommentTextPosition::OwnLine => handle_member_expression_comment(comment) - .or_else(handle_function_declaration_comment) + .or_else(handle_function_comment) .or_else(handle_if_statement_comment) .or_else(handle_while_comment) .or_else(handle_try_comment) @@ -641,38 +641,32 @@ fn handle_member_expression_comment( } } -fn handle_function_declaration_comment( - comment: DecoratedComment, -) -> CommentPlacement { - let is_function_declaration = matches!( +fn handle_function_comment(comment: DecoratedComment) -> CommentPlacement { + if !matches!( comment.enclosing_node().kind(), JsSyntaxKind::JS_FUNCTION_DECLARATION | JsSyntaxKind::JS_FUNCTION_EXPORT_DEFAULT_DECLARATION - ); + | JsSyntaxKind::JS_FUNCTION_EXPRESSION + ) || !comment.kind().is_line() + { + return CommentPlacement::Default(comment); + }; - let following = match comment.following_node() { - Some(following) if is_function_declaration => following, - _ => return CommentPlacement::Default(comment), + let Some(body) = comment.following_node().and_then(JsFunctionBody::cast_ref) else { + return CommentPlacement::Default(comment); }; - // Make comments between the `)` token and the function body leading comments - // of the first non empty statement or dangling comments of the body. + // Make line comments between the `)` token and the function body leading comments + // of the first statement or dangling comments of the body. // ```javascript - // function test() /* comment */ { + // function test() // comment + // { // console.log("Hy"); // } // ``` - if let Some(body) = JsFunctionBody::cast_ref(following) { - match body - .statements() - .iter() - .find(|statement| !matches!(statement, AnyJsStatement::JsEmptyStatement(_))) - { - Some(first) => CommentPlacement::leading(first.into_syntax(), comment), - None => CommentPlacement::dangling(body.into_syntax(), comment), - } - } else { - CommentPlacement::Default(comment) + match body.statements().first() { + Some(first) => CommentPlacement::leading(first.into_syntax(), comment), + None => CommentPlacement::dangling(body.into_syntax(), comment), } } diff --git a/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js b/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js index 996c1ab6901d..a39c7ca35c5c 100644 --- a/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js +++ b/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js @@ -11,7 +11,29 @@ function b() // leading comment } - function c( //some comment foo, bar, -) {} \ No newline at end of file +) {} + + +(function d() +// a +{ + return 42 +}); + +function e() +// a +{ + ; +}; + +function f() +// a +{ + a; +}; + +function h() /* a */ { + a; +}; diff --git a/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js.snap b/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js.snap index 196967aa18ae..724e07fe5fb6 100644 --- a/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js.snap +++ b/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js.snap @@ -19,10 +19,33 @@ function b() // leading comment } - function c( //some comment foo, bar, ) {} + + +(function d() +// a +{ + return 42 +}); + +function e() +// a +{ + ; +}; + +function f() +// a +{ + a; +}; + +function h() /* a */ { + a; +}; + ``` @@ -65,6 +88,24 @@ function c( foo, bar, ) {} + +(function d() { + // a + return 42; +}); + +function e() { + // a +} + +function f() { + // a + a; +} + +function h() /* a */ { + a; +} ``` diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js b/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js index b5c47f27f542..c3553778f752 100644 --- a/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js +++ b/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js @@ -4,22 +4,18 @@ function function_declaration() return 42 } -// FIXME -// TODO: reformat issue -// (function named() -// // this is a function -// { -// return 42 -// })(); +(function named() +// this is a function +{ + return 42 +})(); -// FIXME -// TODO: reformat issue -// (function () -// // this is a function -// { -// return 42 -// })(); +(function () +// this is a function +{ + return 42 +})(); /* anonymous declaration */ export default function () @@ -28,14 +24,12 @@ export default function () return 42 } -// FIXME -// TODO: reformat issue a = { foo() // this is a function {}, -// bar: function() -// // this is a function -// {}, + bar: function() + // this is a function + {}, } diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js.snap deleted file mode 100644 index a09cbbd0c72a..000000000000 --- a/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js.snap +++ /dev/null @@ -1,152 +0,0 @@ ---- -source: crates/biome_formatter_test/src/snapshot_builder.rs -info: js/comments/function/between-parentheses-and-function-body.js ---- - -# Input - -```js -function function_declaration() -// this is a function -{ - return 42 -} - -// FIXME -// TODO: reformat issue -// (function named() -// // this is a function -// { -// return 42 -// })(); - - -// FIXME -// TODO: reformat issue -// (function () -// // this is a function -// { -// return 42 -// })(); - -/* anonymous declaration */ -export default function () -// this is a function -{ - return 42 -} - -// FIXME -// TODO: reformat issue -a = { - foo() - // this is a function - {}, - -// bar: function() -// // this is a function -// {}, -} - -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Biome -@@ -3,15 +3,21 @@ - return 42; - } - --(function named() { -- // this is a function -- return 42; --})(); -+// FIXME -+// TODO: reformat issue -+// (function named() -+// // this is a function -+// { -+// return 42 -+// })(); - --(function () { -- // this is a function -- return 42; --})(); -+// FIXME -+// TODO: reformat issue -+// (function () -+// // this is a function -+// { -+// return 42 -+// })(); - - /* anonymous declaration */ - export default function () { -@@ -19,12 +25,14 @@ - return 42; - } - -+// FIXME -+// TODO: reformat issue - a = { - foo() { - // this is a function - }, - -- bar: function () { -- // this is a function -- }, -+ // bar: function() -+ // // this is a function -+ // {}, - }; -``` - -# Output - -```js -function function_declaration() { - // this is a function - return 42; -} - -// FIXME -// TODO: reformat issue -// (function named() -// // this is a function -// { -// return 42 -// })(); - -// FIXME -// TODO: reformat issue -// (function () -// // this is a function -// { -// return 42 -// })(); - -/* anonymous declaration */ -export default function () { - // this is a function - return 42; -} - -// FIXME -// TODO: reformat issue -a = { - foo() { - // this is a function - }, - - // bar: function() - // // this is a function - // {}, -}; -``` - - diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index 4a8b9e1e6f3e..1296923288a9 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -29,6 +29,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom ### Formatter +#### Bug fixes + +- Fix [#1172](https://github.com/biomejs/biome/issues/1172). Fix placement of line comment after function expression parentheses, they are now attached to first statement in body. Contributed by @kalleep + ### JavaScript APIs ### Linter