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

Commit

Permalink
feat(rome_js_formatter): Parenthesize TypeScript types (#3108)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser authored Aug 30, 2022
1 parent bb31e72 commit 575b4d9
Show file tree
Hide file tree
Showing 61 changed files with 1,288 additions and 1,248 deletions.
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();

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 {
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

0 comments on commit 575b4d9

Please sign in to comment.