fix(js_formatter): don't duplicate dangling comments for mapped types…

…, also fix break mode test

crates/biome_js_formatter/src/comments.rs

@@ -16,7 +16,7 @@ use biome_js_syntax::{
     JsBlockStatement, JsCallArguments, JsCatchClause, JsEmptyStatement, JsFinallyClause,
     JsFormalParameter, JsFunctionBody, JsIdentifierBinding, JsIdentifierExpression, JsIfStatement,
     JsLanguage, JsParameters, JsSyntaxKind, JsSyntaxNode, JsVariableDeclarator, JsWhileStatement,
-    TsInterfaceDeclaration,
+    TsInterfaceDeclaration, TsMappedType,
 };
 use biome_rowan::{AstNode, SyntaxNodeOptionExt, SyntaxTriviaPieceComments, TextLen};
 
@@ -1166,11 +1166,31 @@ fn handle_parameter_comment(comment: DecoratedComment<JsLanguage>) -> CommentPla
 fn handle_mapped_type_comment(
     comment: DecoratedComment<JsLanguage>,
 ) -> CommentPlacement<JsLanguage> {
-    if let (JsSyntaxKind::TS_MAPPED_TYPE, Some(following)) =
-        (comment.enclosing_node().kind(), comment.following_node())
-    {
-        if following.kind() == JsSyntaxKind::TS_TYPE_PARAMETER_NAME {
-            return CommentPlacement::dangling(comment.enclosing_node().clone(), comment);
+    if !matches!(
+        comment.enclosing_node().kind(),
+        JsSyntaxKind::TS_MAPPED_TYPE
+    ) {
+        return CommentPlacement::Default(comment);
+    }
+
+    if matches!(
+        comment.following_node().kind(),
+        Some(
+            JsSyntaxKind::TS_TYPE_PARAMETER_NAME
+                | JsSyntaxKind::TS_MAPPED_TYPE_READONLY_MODIFIER_CLAUSE,
+        )
+    ) {
+        return CommentPlacement::dangling(comment.enclosing_node().clone(), comment);
+    }
+
+    if matches!(
+        comment.preceding_node().kind(),
+        Some(JsSyntaxKind::TS_TYPE_PARAMETER_NAME)
+    ) {
+        if let Some(enclosing) = TsMappedType::cast_ref(comment.enclosing_node()) {
+            if let Ok(keys_type) = enclosing.keys_type() {
+                return CommentPlacement::trailing(keys_type.into_syntax(), comment);
+            }
         }
     }
 

crates/biome_js_formatter/src/ts/lists/union_type_variant_list.rs

@@ -146,7 +146,46 @@ impl Format<JsFormatContext> for FormatTypeVariant<'_> {
             write!(f, [align(2, &format_node)])?;
         }
 
-        if !is_suppressed {
+        // There are a few special cases for nodes which handle their own
+        // dangling comments:
+        // - Mapped types place the dangling comments as _leading_ comments for
+        // the type:
+        //     {
+        //       // This is a dangling comment, formatted as a leading comment
+        //       [foo in keyof Foo]: T
+        //     }
+        // - Other object like types format their own comments _only when there
+        // are no members in the type_:
+        //     {
+        //       // This is a dangling comment, formatted inside JsObjectLike
+        //     }
+        // - Empty tuple types also format their own dangling comments
+        //     [
+        //       // This is a dangling comment, formatted inside TsTupleType
+        //     ]
+        // Attempting to format any of these dangling comments again results
+        // in double printing, and often invalid syntax, such as:
+        //     const t: {
+        //       // Hello
+        //       [foo in keyof Foo]: T
+        //     } = 1;
+        // Would get formatted to:
+        //     const t: {
+        //       // Hello
+        //       [foo in keyof Foo]: T
+        //     }// Hello = 1;
+        // This check prevents that double printing from happening. There may
+        // be a better place for it in the long term (maybe just always ensure
+        // that a comment hasn't already been formatted before writing it?),
+        // but this covers the majority of these cases already.
+        let has_already_formatted_dangling_comments = match node {
+            AnyTsType::TsMappedType(_) => true,
+            AnyTsType::TsObjectType(object) => object.members().is_empty(),
+            AnyTsType::TsTupleType(tuple) => tuple.elements().is_empty(),
+            _ => false,
+        };
+
+        if !is_suppressed && has_already_formatted_dangling_comments {
             write!(f, [format_dangling_comments(node.syntax())])?;
         }
 

crates/biome_js_formatter/src/ts/types/mapped_type.rs

@@ -5,6 +5,7 @@ use crate::utils::FormatOptionalSemicolon;
 use biome_formatter::trivia::FormatLeadingComments;
 use biome_formatter::{format_args, write};
 use biome_js_syntax::{JsSyntaxNode, TsMappedType, TsMappedTypeFields};
+use biome_rowan::Direction;
 
 #[derive(Debug, Clone, Default)]
 pub struct FormatTsMappedType;
@@ -27,39 +28,29 @@ impl FormatNodeRule<TsMappedType> for FormatTsMappedType {
         } = node.as_fields();
 
         let property_name = property_name?;
-
-        // Check if the user introduced a new line inside the node.
-        let should_expand = node
-            .syntax()
-            .tokens()
-            // Skip the first token to avoid formatter instability. See #4165.
-            // This also makes sense since leading trivia of the first token
-            // are not part of the interior of the node.
-            .skip(1)
-            .flat_map(|token| {
-                token
-                    .leading_trivia()
-                    .pieces()
-                    .chain(token.trailing_trivia().pieces())
-            })
-            .any(|piece| piece.is_newline());
+        let should_expand = has_line_break_before_property_name(node)?;
 
         let comments = f.comments().clone();
-        let dangling_comments = comments.dangling_comments(node.syntax());
         let type_annotation_has_leading_comment =
             mapped_type.as_ref().map_or(false, |annotation| {
                 comments.has_leading_comments(annotation.syntax())
             });
 
         let format_inner = format_with(|f| {
+            write!(
+                f,
+                [FormatLeadingComments::Comments(
+                    comments.dangling_comments(node.syntax())
+                )]
+            )?;
+
             if let Some(readonly_modifier) = &readonly_modifier {
                 write!(f, [readonly_modifier.format(), space()])?;
             }
 
             write!(
                 f,
                 [
-                    FormatLeadingComments::Comments(dangling_comments),
                     group(&format_args![
                         l_brack_token.format(),
                         property_name.format(),
@@ -109,3 +100,45 @@ impl NeedsParentheses for TsMappedType {
         false
     }
 }
+
+/// Check if the user introduced a new line inside the node, but only if
+/// that new line occurs at or before the property name. For example,
+/// this would break:
+///   { [
+///     A in B]: T}
+/// Because the line break occurs before `A`, the property name. But this
+/// would _not_ break:
+///   { [A
+///     in B]: T}
+/// Because the break is _after_ the `A`.
+fn has_line_break_before_property_name(node: &TsMappedType) -> FormatResult<bool> {
+    let property_name = node.property_name()?;
+    let first_property_name_token = match property_name.syntax().first_token() {
+        Some(first_token) => first_token,
+        None => return Err(FormatError::SyntaxError),
+    };
+
+    let result = node
+        .syntax()
+        .descendants_tokens(Direction::Next)
+        // Skip the first token to avoid formatter instability. See #4165.
+        // This also makes sense since leading trivia of the first token
+        // are not part of the interior of the node.
+        .skip(1)
+        // Only process up until the first token of the property name
+        .take_while(|token| first_property_name_token != *token)
+        .flat_map(|token| {
+            token
+                .leading_trivia()
+                .pieces()
+                .chain(token.trailing_trivia().pieces())
+        })
+        // Add only the leading trivia of the first property name token to
+        // ensure that any newline in front of it is included. Otherwise
+        // the `take_while` stops at the previous token, and the trailing
+        // trivia won't include the newline.
+        .chain(first_property_name_token.leading_trivia().pieces())
+        .any(|piece| piece.is_newline());
+
+    Ok(result)
+}

crates/biome_js_formatter/tests/quick_test.rs

@@ -14,9 +14,8 @@ mod language {
 // use this test check if your snippet prints as you wish, without using a snapshot
 fn quick_test() {
     let src = r#"
-    setTimeout(() => {
-      updateDebouncedQuery(query);
-    }, debounceTime ?? 500);
+    type A2 = {
+        readonly [A in B]: T}
     "#;
     let source_type = JsFileSource::tsx();
     let tree = parse(