Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_formatter): Parenthesize TypeScript types #3108

Merged
merged 4 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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()
])]
)
}
}
88 changes: 42 additions & 46 deletions crates/rome_js_formatter/src/js/lists/variable_declarator_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the group outside into the FormatJsVariableDeclaration to match prettier's formatting


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()
}))]
)
}
}
158 changes: 155 additions & 3 deletions crates/rome_js_formatter/src/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ use rome_js_syntax::{
JsBinaryOperator, JsComputedMemberAssignment, JsComputedMemberExpression,
JsConditionalExpression, JsLanguage, JsParenthesizedAssignment, JsParenthesizedExpression,
JsSequenceExpression, JsStaticMemberAssignment, JsStaticMemberExpression, JsSyntaxKind,
JsSyntaxNode, JsSyntaxToken,
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> {
Expand Down Expand Up @@ -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(),
}
}

Expand All @@ -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(),
}
}
}
Expand Down Expand Up @@ -716,6 +788,86 @@ impl NeedsParentheses for JsAnyAssignmentPattern {
}
}

impl NeedsParentheses for TsType {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These union implementation isn't used anywhere but I think it will be useful to remind everyone adding a new TsType to also implement NeedsParentheses

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()),
Expand Down
4 changes: 4 additions & 0 deletions crates/rome_js_formatter/src/syntax_rewriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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
}
}
13 changes: 12 additions & 1 deletion crates/rome_js_formatter/src/ts/module/import_type.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
}
}
Loading