feat(rome_js_formatter): Parenthesize Types

This PR implements the rules for when parentheses around TypeScript types are needed or can be removed without changing the semantics of the program.

The majority of the PR is to implement `NeedsParentheses` for every `TsType`.

This PR further fixes an instability issue with the syntax rewriter. The rewriter used to remove all whitespace between the `(` paren and the token (or first comment and skipped token trivia). This is necessary or the formatter otherwise inserts an extra blank line before nodes that are parenthesized:

```
a
(
  Long &&
  Longer &&
)
```

becomes

```
a

(
  Long &&
  Longer &&
)
```

Notice, the added new line. What this logic didn't account for is that it should not remove a leading new line before a comment because we want to keep the comment on its own line and this requires that there's a leading new line trivia.

The last fix is in the source map that so far assumed that all ranges are added in increasing `end` order

```
correct: 2..3, 2..4, 2..5 (notice how the ranges are sorted by end)
incorrect: 2..4, 2..3, 2..5
```

This PR updates the sorting of the text ranges to also account the end ranges.

I added unit tests for all rules where parentheses are required and reviewed the updated snapshots.

There are a few new changes but these unrelated to parentheses but instead problems with the formatting of the specific syntax.

Average compatibility: 83.70 -> 84.11
Compatible lines: 80.79 -> 81.42

crates/rome_js_formatter/src/js/declarations/variable_declaration.rs

@@ -1,5 +1,5 @@
 use crate::prelude::*;
-use rome_formatter::write;
+use rome_formatter::{format_args, write};
 
 use rome_js_syntax::JsVariableDeclaration;
 use rome_js_syntax::JsVariableDeclarationFields;
@@ -11,6 +11,13 @@ impl FormatNodeRule<JsVariableDeclaration> for FormatJsVariableDeclaration {
     fn fmt_fields(&self, node: &JsVariableDeclaration, f: &mut JsFormatter) -> FormatResult<()> {
         let JsVariableDeclarationFields { kind, declarators } = node.as_fields();
 
-        write!(f, [kind.format(), space(), declarators.format()])
+        write!(
+            f,
+            [group(&format_args![
+                kind.format(),
+                space(),
+                declarators.format()
+            ])]
+        )
     }
 }

crates/rome_js_formatter/src/js/lists/variable_declarator_list.rs

@@ -11,60 +11,56 @@ impl FormatRule<JsVariableDeclaratorList> for FormatJsVariableDeclaratorList {
     type Context = JsFormatContext;
 
     fn fmt(&self, node: &JsVariableDeclaratorList, f: &mut JsFormatter) -> FormatResult<()> {
-        let format_inner = format_with(|f| {
-            let length = node.len();
+        let length = node.len();
 
-            let is_parent_for_loop = node.syntax().grand_parent().map_or(false, |grand_parent| {
-                matches!(
-                    grand_parent.kind(),
-                    JsSyntaxKind::JS_FOR_STATEMENT
-                        | JsSyntaxKind::JS_FOR_OF_STATEMENT
-                        | JsSyntaxKind::JS_FOR_IN_STATEMENT
-                )
-            });
-
-            let has_any_initializer = node.iter().any(|declarator| {
-                declarator.map_or(false, |declarator| declarator.initializer().is_some())
-            });
+        let is_parent_for_loop = node.syntax().grand_parent().map_or(false, |grand_parent| {
+            matches!(
+                grand_parent.kind(),
+                JsSyntaxKind::JS_FOR_STATEMENT
+                    | JsSyntaxKind::JS_FOR_OF_STATEMENT
+                    | JsSyntaxKind::JS_FOR_IN_STATEMENT
+            )
+        });
 
-            let format_separator = format_with(|f| {
-                if !is_parent_for_loop && has_any_initializer {
-                    write!(f, [hard_line_break()])
-                } else {
-                    write!(f, [soft_line_break_or_space()])
-                }
-            });
+        let has_any_initializer = node.iter().any(|declarator| {
+            declarator.map_or(false, |declarator| declarator.initializer().is_some())
+        });
 
-            let mut declarators = node.iter().zip(
-                node.format_separated(JsSyntaxKind::COMMA)
-                    .with_trailing_separator(TrailingSeparator::Disallowed),
-            );
+        let format_separator = format_with(|f| {
+            if !is_parent_for_loop && has_any_initializer {
+                write!(f, [hard_line_break()])
+            } else {
+                write!(f, [soft_line_break_or_space()])
+            }
+        });
 
-            let (first_declarator, format_first_declarator) = match declarators.next() {
-                Some((syntax, format_first_declarator)) => (syntax?, format_first_declarator),
-                None => return Err(FormatError::SyntaxError),
-            };
+        let mut declarators = node.iter().zip(
+            node.format_separated(JsSyntaxKind::COMMA)
+                .with_trailing_separator(TrailingSeparator::Disallowed),
+        );
 
-            if length == 1 && !first_declarator.syntax().has_leading_comments() {
-                return write!(f, [format_first_declarator]);
-            }
+        let (first_declarator, format_first_declarator) = match declarators.next() {
+            Some((syntax, format_first_declarator)) => (syntax?, format_first_declarator),
+            None => return Err(FormatError::SyntaxError),
+        };
 
-            write!(
-                f,
-                [indent(&format_once(|f| {
-                    write!(f, [format_first_declarator])?;
+        if length == 1 && !first_declarator.syntax().has_leading_comments() {
+            return write!(f, [format_first_declarator]);
+        }
 
-                    if length > 1 {
-                        write!(f, [format_separator])?;
-                    }
+        write!(
+            f,
+            [indent(&format_once(|f| {
+                write!(f, [format_first_declarator])?;
 
-                    f.join_with(&format_separator)
-                        .entries(declarators.map(|(_, format)| format))
-                        .finish()
-                }))]
-            )
-        });
+                if length > 1 {
+                    write!(f, [format_separator])?;
+                }
 
-        write!(f, [group(&format_inner)])
+                f.join_with(&format_separator)
+                    .entries(declarators.map(|(_, format)| format))
+                    .finish()
+            }))]
+        )
     }
 }

crates/rome_js_formatter/src/parentheses.rs

@@ -44,10 +44,11 @@ use rome_js_syntax::{
     JsAnyLiteralExpression, JsArrowFunctionExpression, JsAssignmentExpression, JsBinaryExpression,
     JsBinaryOperator, JsComputedMemberAssignment, JsComputedMemberExpression,
     JsConditionalExpression, JsLanguage, JsParenthesizedAssignment, JsParenthesizedExpression,
-    JsSequenceExpression, JsStaticMemberAssignment, JsStaticMemberExpression, JsSyntaxKind,
-    JsSyntaxNode, JsSyntaxToken,
+    JsSequenceExpression, JsSyntaxKind, JsSyntaxNode, JsSyntaxToken, TsConditionalType,
+    TsIndexedAccessType, TsIntersectionTypeElementList, TsParenthesizedType, TsType,
+    TsUnionTypeVariantList,
 };
-use rome_rowan::{declare_node_union, match_ast, AstNode, SyntaxResult};
+use rome_rowan::{declare_node_union, match_ast, AstNode, AstSeparatedList, SyntaxResult};
 
 /// Node that may be parenthesized to ensure it forms valid syntax or to improve readability
 pub trait NeedsParentheses: AstNode<Language = JsLanguage> {
@@ -597,15 +598,84 @@ pub(crate) fn is_spread(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool {
     )
 }
 
+/// Returns `true` if a TS primary type needs parentheses
+pub(crate) fn operator_type_or_higher_needs_parens(
+    node: &JsSyntaxNode,
+    parent: &JsSyntaxNode,
+) -> bool {
+    debug_assert_is_parent(node, parent);
+
+    match parent.kind() {
+        JsSyntaxKind::TS_ARRAY_TYPE
+        | JsSyntaxKind::TS_TYPE_OPERATOR_TYPE
+        | JsSyntaxKind::TS_REST_TUPLE_TYPE_ELEMENT
+        | JsSyntaxKind::TS_OPTIONAL_TUPLE_TYPE_ELEMENT => true,
+        JsSyntaxKind::TS_INDEXED_ACCESS_TYPE => {
+            let indexed = TsIndexedAccessType::unwrap_cast(parent.clone());
+
+            indexed.object_type().map(AstNode::into_syntax).as_ref() == Ok(node)
+        }
+        _ => false,
+    }
+}
+
+/// Tests if `node` is the check type of a [TsConditionalType]
+///
+/// ```javascript
+/// type s = A extends string ? string : number //  true for `A`, false for `string` and `number`
+/// ```
+pub(crate) fn is_check_type(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool {
+    debug_assert_is_parent(node, parent);
+
+    match parent.kind() {
+        JsSyntaxKind::TS_CONDITIONAL_TYPE => {
+            let conditional = TsConditionalType::unwrap_cast(parent.clone());
+
+            conditional.check_type().map(AstNode::into_syntax).as_ref() == Ok(node)
+        }
+        _ => false,
+    }
+}
+
+/// Returns `true` if node is in a union or intersection type with more than one variant
+///
+/// ```javascript
+/// type A = &string // -> false for `string` because `string` is the only variant
+/// type B = string & number // -> true for `string` or `number`
+/// type C = |string // -> false
+/// type D = string | number // -> true
+/// ```
+pub(crate) fn is_in_many_type_union_or_intersection_list(
+    node: &JsSyntaxNode,
+    parent: &JsSyntaxNode,
+) -> bool {
+    debug_assert_is_parent(node, parent);
+
+    match parent.kind() {
+        JsSyntaxKind::TS_UNION_TYPE_VARIANT_LIST => {
+            let list = TsUnionTypeVariantList::unwrap_cast(parent.clone());
+
+            list.len() > 1
+        }
+        JsSyntaxKind::TS_INTERSECTION_TYPE_ELEMENT_LIST => {
+            let list = TsIntersectionTypeElementList::unwrap_cast(parent.clone());
+
+            list.len() > 1
+        }
+        _ => false,
+    }
+}
+
 declare_node_union! {
-    pub(crate) JsAnyParenthesized = JsParenthesizedExpression | JsParenthesizedAssignment
+    pub(crate) JsAnyParenthesized = JsParenthesizedExpression | JsParenthesizedAssignment | TsParenthesizedType
 }
 
 impl JsAnyParenthesized {
     pub(crate) fn l_paren_token(&self) -> SyntaxResult<JsSyntaxToken> {
         match self {
             JsAnyParenthesized::JsParenthesizedExpression(expression) => expression.l_paren_token(),
             JsAnyParenthesized::JsParenthesizedAssignment(assignment) => assignment.l_paren_token(),
+            JsAnyParenthesized::TsParenthesizedType(ty) => ty.l_paren_token(),
         }
     }
 
@@ -617,13 +687,15 @@ impl JsAnyParenthesized {
             JsAnyParenthesized::JsParenthesizedAssignment(assignment) => {
                 assignment.assignment().map(AstNode::into_syntax)
             }
+            JsAnyParenthesized::TsParenthesizedType(ty) => ty.ty().map(AstNode::into_syntax),
         }
     }
 
     pub(crate) fn r_paren_token(&self) -> SyntaxResult<JsSyntaxToken> {
         match self {
             JsAnyParenthesized::JsParenthesizedExpression(expression) => expression.r_paren_token(),
             JsAnyParenthesized::JsParenthesizedAssignment(assignment) => assignment.r_paren_token(),
+            JsAnyParenthesized::TsParenthesizedType(ty) => ty.r_paren_token(),
         }
     }
 }
@@ -716,6 +788,86 @@ impl NeedsParentheses for JsAnyAssignmentPattern {
     }
 }
 
+impl NeedsParentheses for TsType {
+    fn needs_parentheses(&self) -> bool {
+        match self {
+            TsType::TsAnyType(ty) => ty.needs_parentheses(),
+            TsType::TsArrayType(ty) => ty.needs_parentheses(),
+            TsType::TsBigIntLiteralType(ty) => ty.needs_parentheses(),
+            TsType::TsBigintType(ty) => ty.needs_parentheses(),
+            TsType::TsBooleanLiteralType(ty) => ty.needs_parentheses(),
+            TsType::TsBooleanType(ty) => ty.needs_parentheses(),
+            TsType::TsConditionalType(ty) => ty.needs_parentheses(),
+            TsType::TsConstructorType(ty) => ty.needs_parentheses(),
+            TsType::TsFunctionType(ty) => ty.needs_parentheses(),
+            TsType::TsImportType(ty) => ty.needs_parentheses(),
+            TsType::TsIndexedAccessType(ty) => ty.needs_parentheses(),
+            TsType::TsInferType(ty) => ty.needs_parentheses(),
+            TsType::TsIntersectionType(ty) => ty.needs_parentheses(),
+            TsType::TsMappedType(ty) => ty.needs_parentheses(),
+            TsType::TsNeverType(ty) => ty.needs_parentheses(),
+            TsType::TsNonPrimitiveType(ty) => ty.needs_parentheses(),
+            TsType::TsNullLiteralType(ty) => ty.needs_parentheses(),
+            TsType::TsNumberLiteralType(ty) => ty.needs_parentheses(),
+            TsType::TsNumberType(ty) => ty.needs_parentheses(),
+            TsType::TsObjectType(ty) => ty.needs_parentheses(),
+            TsType::TsParenthesizedType(ty) => ty.needs_parentheses(),
+            TsType::TsReferenceType(ty) => ty.needs_parentheses(),
+            TsType::TsStringLiteralType(ty) => ty.needs_parentheses(),
+            TsType::TsStringType(ty) => ty.needs_parentheses(),
+            TsType::TsSymbolType(ty) => ty.needs_parentheses(),
+            TsType::TsTemplateLiteralType(ty) => ty.needs_parentheses(),
+            TsType::TsThisType(ty) => ty.needs_parentheses(),
+            TsType::TsTupleType(ty) => ty.needs_parentheses(),
+            TsType::TsTypeOperatorType(ty) => ty.needs_parentheses(),
+            TsType::TsTypeofType(ty) => ty.needs_parentheses(),
+            TsType::TsUndefinedType(ty) => ty.needs_parentheses(),
+            TsType::TsUnionType(ty) => ty.needs_parentheses(),
+            TsType::TsUnknownType(ty) => ty.needs_parentheses(),
+            TsType::TsVoidType(ty) => ty.needs_parentheses(),
+        }
+    }
+
+    fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
+        match self {
+            TsType::TsAnyType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsArrayType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsBigIntLiteralType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsBigintType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsBooleanLiteralType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsBooleanType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsConditionalType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsConstructorType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsFunctionType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsImportType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsIndexedAccessType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsInferType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsIntersectionType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsMappedType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsNeverType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsNonPrimitiveType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsNullLiteralType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsNumberLiteralType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsNumberType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsObjectType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsParenthesizedType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsReferenceType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsStringLiteralType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsStringType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsSymbolType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsTemplateLiteralType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsThisType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsTupleType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsTypeOperatorType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsTypeofType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsUndefinedType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsUnionType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsUnknownType(ty) => ty.needs_parentheses_with_parent(parent),
+            TsType::TsVoidType(ty) => ty.needs_parentheses_with_parent(parent),
+        }
+    }
+}
+
 fn debug_assert_is_expression(node: &JsSyntaxNode) {
     debug_assert!(
         JsAnyExpression::can_cast(node.kind()),

crates/rome_js_formatter/src/syntax_rewriter.rs

@@ -3,6 +3,7 @@ use crate::TextRange;
 use rome_formatter::{TransformSourceMap, TransformSourceMapBuilder};
 use rome_js_syntax::{
     JsAnyAssignment, JsAnyExpression, JsLanguage, JsLogicalExpression, JsSyntaxKind, JsSyntaxNode,
+    TsType,
 };
 use rome_rowan::syntax::SyntaxTrivia;
 use rome_rowan::{
@@ -157,6 +158,9 @@ impl JsFormatSyntaxRewriter {
                             .with_assignment(JsAnyAssignment::unwrap_cast(inner))
                             .into_syntax()
                     }
+                    JsAnyParenthesized::TsParenthesizedType(ty) => {
+                        ty.with_ty(TsType::unwrap_cast(inner)).into_syntax()
+                    }
                 };
 
                 VisitNodeSignal::Replace(updated)

crates/rome_js_formatter/src/ts/expressions/template_literal_type.rs

@@ -1,8 +1,9 @@
 use crate::prelude::*;
 
+use crate::parentheses::NeedsParentheses;
 use rome_formatter::write;
-use rome_js_syntax::TsTemplateLiteralType;
 use rome_js_syntax::TsTemplateLiteralTypeFields;
+use rome_js_syntax::{JsSyntaxNode, TsTemplateLiteralType};
 
 #[derive(Debug, Clone, Default)]
 pub struct FormatTsTemplateLiteralType;
@@ -24,4 +25,14 @@ impl FormatNodeRule<TsTemplateLiteralType> for FormatTsTemplateLiteralType {
             ]
         ]
     }
+
+    fn needs_parentheses(&self, item: &TsTemplateLiteralType) -> bool {
+        item.needs_parentheses()
+    }
+}
+
+impl NeedsParentheses for TsTemplateLiteralType {
+    fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool {
+        false
+    }
 }

crates/rome_js_formatter/src/ts/module/import_type.rs

@@ -1,9 +1,10 @@
 use crate::prelude::*;
 use crate::utils::{FormatLiteralStringToken, StringLiteralParentKind};
 
+use crate::parentheses::NeedsParentheses;
 use rome_formatter::write;
-use rome_js_syntax::TsImportType;
 use rome_js_syntax::TsImportTypeFields;
+use rome_js_syntax::{JsSyntaxNode, TsImportType};
 
 #[derive(Debug, Clone, Default)]
 pub struct FormatTsImportType;
@@ -39,4 +40,14 @@ impl FormatNodeRule<TsImportType> for FormatTsImportType {
             ]
         ]
     }
+
+    fn needs_parentheses(&self, item: &TsImportType) -> bool {
+        item.needs_parentheses()
+    }
+}
+
+impl NeedsParentheses for TsImportType {
+    fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool {
+        false
+    }
 }