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

feat(rome_js_formatter): type alias as assignment like #2787

Merged
merged 3 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
38 changes: 2 additions & 36 deletions crates/rome_js_formatter/src/js/expressions/object_expression.rs
Original file line number Diff line number Diff line change
@@ -1,47 +1,13 @@
use crate::prelude::*;
use crate::utils::node_has_leading_newline;

use crate::utils::JsObjectLike;
use rome_formatter::write;
use rome_js_syntax::JsObjectExpression;
use rome_js_syntax::JsObjectExpressionFields;

#[derive(Debug, Clone, Default)]
pub struct FormatJsObjectExpression;

impl FormatNodeRule<JsObjectExpression> for FormatJsObjectExpression {
fn fmt_fields(&self, node: &JsObjectExpression, f: &mut JsFormatter) -> FormatResult<()> {
let JsObjectExpressionFields {
l_curly_token,
members,
r_curly_token,
} = node.as_fields();

let has_newline = node_has_leading_newline(members.syntax());

if members.is_empty() {
write!(
f,
[
format_delimited(&l_curly_token?, &members.format(), &r_curly_token?)
.soft_block_indent()
]
)
} else if has_newline {
write!(
f,
[
format_delimited(&l_curly_token?, &members.format(), &r_curly_token?)
.block_indent()
]
)
} else {
write!(
f,
[
format_delimited(&l_curly_token?, &members.format(), &r_curly_token?)
.soft_block_spaces()
]
)
}
write!(f, [JsObjectLike::from(node.clone())])
}
}
11 changes: 8 additions & 3 deletions crates/rome_js_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,10 +457,15 @@ mod test {
// use this test check if your snippet prints as you wish, without using a snapshot
fn quick_test() {
let src = r#"
export type a =
// foo
| foo1 & foo2
// bar
| bar1 & bar2
// prettier-ignore
| qux1 & qux2;


it(`does something really long and complicated so I have to write a very long name for the test`, function () {
console.log("hello!");
});

"#;
let syntax = SourceType::tsx();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,37 +1,25 @@
use crate::prelude::*;
use crate::utils::FormatWithSemicolon;

use crate::utils::{FormatWithSemicolon, JsAnyAssignmentLike};
use rome_formatter::{format_args, write};
use rome_js_syntax::{TsTypeAliasDeclaration, TsTypeAliasDeclarationFields};
use rome_js_syntax::TsTypeAliasDeclaration;

#[derive(Debug, Clone, Default)]
pub struct FormatTsTypeAliasDeclaration;

impl FormatNodeRule<TsTypeAliasDeclaration> for FormatTsTypeAliasDeclaration {
fn fmt_fields(&self, node: &TsTypeAliasDeclaration, f: &mut JsFormatter) -> FormatResult<()> {
let TsTypeAliasDeclarationFields {
type_token,
binding_identifier,
type_parameters,
eq_token,
ty,
semicolon_token,
} = node.as_fields();

let type_token = node.type_token()?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@ematipico ematipico Jun 27, 2022

Choose a reason for hiding this comment

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

I maintained the same logic we have introduced for variable declarations. There, the kind (var/let/const) doesn't belong to left. Hence, here type_token doesn't belong to left.

let semicolon = node.semicolon_token();
let assignment_like = format_with(|f| write!(f, [JsAnyAssignmentLike::from(node.clone())]));
write!(
f,
[FormatWithSemicolon::new(
&format_args!(
&format_args![
type_token.format(),
space_token(),
binding_identifier.format(),
type_parameters.format(),
space_token(),
eq_token.format(),
space_token(),
ty.format(),
),
semicolon_token.as_ref()
group_elements(&assignment_like)
],
semicolon.as_ref()
)]
)
}
Expand Down
29 changes: 3 additions & 26 deletions crates/rome_js_formatter/src/ts/types/object_type.rs
Original file line number Diff line number Diff line change
@@ -1,36 +1,13 @@
use crate::prelude::*;
use crate::utils::node_has_leading_newline;

use crate::utils::JsObjectLike;
use rome_formatter::write;
use rome_js_syntax::{TsObjectType, TsObjectTypeFields};
use rome_js_syntax::TsObjectType;

#[derive(Debug, Clone, Default)]
pub struct FormatTsObjectType;

impl FormatNodeRule<TsObjectType> for FormatTsObjectType {
fn fmt_fields(&self, node: &TsObjectType, f: &mut JsFormatter) -> FormatResult<()> {
let TsObjectTypeFields {
l_curly_token,
members,
r_curly_token,
} = node.as_fields();

if node_has_leading_newline(members.syntax()) {
write!(
f,
[
format_delimited(&l_curly_token?, &members.format(), &r_curly_token?)
.block_indent()
]
)
} else {
write!(
f,
[
format_delimited(&l_curly_token?, &members.format(), &r_curly_token?,)
.soft_block_spaces()
]
)
}
write!(f, [JsObjectLike::from(node.clone())])
}
}
95 changes: 86 additions & 9 deletions crates/rome_js_formatter/src/utils/assignment_like.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use rome_js_syntax::{
JsAnyObjectAssignmentPatternMember, JsAnyObjectBindingPatternMember, JsAnyObjectMemberName,
JsAssignmentExpression, JsInitializerClause, JsObjectAssignmentPattern,
JsObjectAssignmentPatternProperty, JsObjectBindingPattern, JsPropertyObjectMember,
JsSyntaxKind, JsVariableDeclarator, TsAnyVariableAnnotation, TsType,
JsSyntaxKind, JsVariableDeclarator, TsAnyVariableAnnotation, TsIdentifierBinding, TsType,
TsTypeAliasDeclaration,
};
use rome_js_syntax::{JsAnyLiteralExpression, JsSyntaxNode};
use rome_rowan::{declare_node_union, AstNode, SyntaxResult};
Expand All @@ -17,15 +18,16 @@ declare_node_union! {
JsPropertyObjectMember |
JsAssignmentExpression |
JsObjectAssignmentPatternProperty |
JsVariableDeclarator
JsVariableDeclarator |
TsTypeAliasDeclaration
}

declare_node_union! {
pub(crate) LeftAssignmentLike = JsAnyAssignmentPattern | JsAnyObjectMemberName | JsAnyBindingPattern
pub(crate) LeftAssignmentLike = JsAnyAssignmentPattern | JsAnyObjectMemberName | JsAnyBindingPattern | TsIdentifierBinding
}

declare_node_union! {
pub(crate) RightAssignmentLike = JsAnyExpression | JsAnyAssignmentPattern | JsInitializerClause
pub(crate) RightAssignmentLike = JsAnyExpression | JsAnyAssignmentPattern | JsInitializerClause | TsType
}

declare_node_union! {
Expand Down Expand Up @@ -151,6 +153,8 @@ impl RightAssignmentLike {
RightAssignmentLike::JsAnyExpression(expression) => Some(expression.clone()),
RightAssignmentLike::JsInitializerClause(initializer) => initializer.expression().ok(),
RightAssignmentLike::JsAnyAssignmentPattern(_) => None,
// TODO: check here
ematipico marked this conversation as resolved.
Show resolved Hide resolved
RightAssignmentLike::TsType(_) => None,
}
}
}
Expand All @@ -167,6 +171,9 @@ impl Format<JsFormatContext> for RightAssignmentLike {
RightAssignmentLike::JsInitializerClause(initializer) => {
write!(f, [space_token(), initializer.format()])
}
RightAssignmentLike::TsType(ty) => {
write!(f, [space_token(), ty.format()])
}
}
}
}
Expand Down Expand Up @@ -294,6 +301,9 @@ impl JsAnyAssignmentLike {
// SAFETY: Calling `unwrap` here is safe because we check `should_only_left` variant at the beginning of the `layout` function
Ok(variable_declarator.initializer().unwrap().into())
}
JsAnyAssignmentLike::TsTypeAliasDeclaration(type_alias_declaration) => {
Ok(type_alias_declaration.ty()?.into())
}
}
}

Expand All @@ -309,6 +319,9 @@ impl JsAnyAssignmentLike {
JsAnyAssignmentLike::JsVariableDeclarator(variable_declarator) => {
Ok(variable_declarator.id()?.into())
}
JsAnyAssignmentLike::TsTypeAliasDeclaration(type_alias_declaration) => {
Ok(type_alias_declaration.binding_identifier()?.into())
}
}
}

Expand Down Expand Up @@ -347,9 +360,20 @@ impl JsAnyAssignmentLike {
JsAnyAssignmentLike::JsVariableDeclarator(variable_declarator) => {
let id = variable_declarator.id()?;
let variable_annotation = variable_declarator.variable_annotation();

write!(f, [id.format(), variable_annotation.format()])?;
Ok(false)
}
JsAnyAssignmentLike::TsTypeAliasDeclaration(type_alias_declaration) => {
let binding_identifier = type_alias_declaration.binding_identifier()?;
let type_parameters = type_alias_declaration.type_parameters();

write!(f, [binding_identifier.format()])?;
if let Some(type_parameters) = type_parameters {
write!(f, [type_parameters.format(),])?;
}
Ok(false)
}
}
}

Expand All @@ -374,6 +398,10 @@ impl JsAnyAssignmentLike {
}
Ok(())
}
JsAnyAssignmentLike::TsTypeAliasDeclaration(type_alias_declaration) => {
let eq_token = type_alias_declaration.eq_token()?;
write!(f, [space_token(), eq_token.format()])
}
}
}

Expand Down Expand Up @@ -403,6 +431,10 @@ impl JsAnyAssignmentLike {
}
Ok(())
}
JsAnyAssignmentLike::TsTypeAliasDeclaration(type_alias_declaration) => {
let ty = type_alias_declaration.ty()?;
write!(f, [space_token(), ty.format()])
}
}
}

Expand All @@ -423,10 +455,8 @@ impl JsAnyAssignmentLike {
return Ok(AssignmentLikeLayout::BreakLeftHandSide);
}

if let Some(expression) = &right {
if should_break_after_operator(expression)? {
return Ok(AssignmentLikeLayout::BreakAfterOperator);
}
if self.should_break_after_operator()? {
return Ok(AssignmentLikeLayout::BreakAfterOperator);
}

if is_left_short {
Expand Down Expand Up @@ -559,6 +589,35 @@ impl JsAnyAssignmentLike {
Ok(false)
}

fn is_complex_type_alias(&self) -> SyntaxResult<bool> {
let result = if let JsAnyAssignmentLike::TsTypeAliasDeclaration(type_alias_declaration) =
self
{
let type_parameters = type_alias_declaration.type_parameters();

if let Some(type_parameters) = type_parameters {
let items = type_parameters.items();
if items.len() <= 1 {
return Ok(false);
};
for type_parameter in type_parameters.items() {
let type_parameter = type_parameter?;

if type_parameter.constraint().is_some() || type_parameter.default().is_some() {
return Ok(true);
}
}
return Ok(false);
} else {
false
}
} else {
false
};

Ok(result)
}

/// Particular function that checks if the left hand side of a [JsAnyAssignmentLike] should
/// be broken on multiple lines
fn should_break_left_hand_side(&self) -> SyntaxResult<bool> {
Expand All @@ -573,7 +632,25 @@ impl JsAnyAssignmentLike {
.and_then(|annotation| is_complex_type_annotation(annotation).ok())
.unwrap_or(false);

Ok(is_complex_destructuring || has_complex_type_annotation)
let is_complex_type_alias = self.is_complex_type_alias()?;

Ok(is_complex_destructuring || has_complex_type_annotation || is_complex_type_alias)
}

/// Checks if the the current assignment is eligible for [AssignmentLikeLayout::BreakAfterOperator]
///
/// This function is small wrapper around [should_break_after_operator] because it has to work
/// for nodes that belong to TypeScript too.
fn should_break_after_operator(&self) -> SyntaxResult<bool> {
let right = self.right()?;

let result = if let Some(expression) = right.as_expression() {
should_break_after_operator(&expression)?
} else {
has_new_line_before_comment(right.syntax())
};

Ok(result)
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/rome_js_formatter/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub(crate) mod format_class;
pub mod jsx_utils;
mod member_chain;
mod object;
mod object_like;
mod object_pattern_like;
#[cfg(test)]
mod quickcheck_utils;
Expand All @@ -18,6 +19,7 @@ pub(crate) use assignment_like::{should_break_after_operator, JsAnyAssignmentLik
pub(crate) use binary_like_expression::{format_binary_like_expression, JsAnyBinaryLikeExpression};
pub(crate) use format_conditional::{format_conditional, Conditional};
pub(crate) use member_chain::format_call_expression;
pub(crate) use object_like::JsObjectLike;
pub(crate) use object_pattern_like::JsObjectPatternLike;
use rome_formatter::{format_args, normalize_newlines, write, Buffer, VecBuffer};
use rome_js_syntax::suppression::{has_suppressions_category, SuppressionCategory};
Expand Down
Loading