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): parenthesise TsAsExpression
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Aug 17, 2022
1 parent cb1d677 commit 6dbe074
Show file tree
Hide file tree
Showing 19 changed files with 344 additions and 582 deletions.
336 changes: 178 additions & 158 deletions crates/rome_js_formatter/src/js/expressions/call_arguments.rs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::prelude::*;

use crate::js::expressions::static_member_expression::memberish_needs_parens;
use crate::js::expressions::static_member_expression::member_chain_callee_needs_parens;
use crate::parentheses::NeedsParentheses;
use rome_formatter::{format_args, write};
use rome_js_syntax::{
Expand Down Expand Up @@ -117,7 +117,7 @@ impl NeedsParentheses for JsComputedMemberExpression {
return true;
}

memberish_needs_parens(self.clone().into(), parent)
member_chain_callee_needs_parens(self.clone().into(), parent)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::prelude::*;
use crate::utils::JsAnyConditional;

use crate::parentheses::{
is_binary_like_left_or_right, is_conditional_test, is_in_left_hand_side_position,
is_binary_like_left_or_right, is_conditional_test, is_in_left_hand_side_position, is_spread,
NeedsParentheses,
};
use rome_js_syntax::{JsConditionalExpression, JsSyntaxKind, JsSyntaxNode};
Expand All @@ -28,16 +28,15 @@ impl NeedsParentheses for JsConditionalExpression {
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
match parent.kind() {
JsSyntaxKind::JS_UNARY_EXPRESSION
| JsSyntaxKind::JS_SPREAD
| JsSyntaxKind::JS_AWAIT_EXPRESSION
| JsSyntaxKind::JSX_SPREAD_ATTRIBUTE
| JsSyntaxKind::TS_TYPE_ASSERTION_EXPRESSION
| JsSyntaxKind::TS_AS_EXPRESSION => true,

_ => {
is_conditional_test(self.syntax(), parent)
|| is_in_left_hand_side_position(self.syntax(), parent)
|| is_binary_like_left_or_right(self.syntax(), parent)
|| is_spread(self.syntax(), parent)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ pub(crate) fn is_expression_handling_parens(expression: &JsAnyExpression) -> boo
| JsComputedMemberExpression(_)
| TsNonNullAssertionExpression(_)
| JsxTagExpression(_)
| TsAsExpression(_)
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,21 +155,15 @@ impl NeedsParentheses for JsStaticMemberExpression {
return true;
}

memberish_needs_parens(self.clone().into(), parent)
member_chain_callee_needs_parens(self.clone().into(), parent)
}
}

pub(crate) fn memberish_needs_parens(node: JsAnyExpression, parent: &JsSyntaxNode) -> bool {
pub(crate) fn member_chain_callee_needs_parens(
node: JsAnyExpression,
parent: &JsSyntaxNode,
) -> bool {
use JsAnyExpression::*;
debug_assert!(
matches!(
node,
JsStaticMemberExpression(_)
| JsComputedMemberExpression(_)
| TsNonNullAssertionExpression(_)
),
"Expected node to be a member expression"
);

match parent.kind() {
// `new (test().a)
Expand Down
13 changes: 7 additions & 6 deletions crates/rome_js_formatter/src/js/expressions/template.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::prelude::*;
use rome_formatter::write;

use crate::js::expressions::static_member_expression::member_chain_callee_needs_parens;
use crate::parentheses::NeedsParentheses;
use rome_js_syntax::{
JsAnyExpression, JsSyntaxNode, JsSyntaxToken, JsTemplate, TsTemplateLiteralType,
Expand Down Expand Up @@ -86,11 +87,11 @@ impl JsAnyTemplate {

/// `TemplateLiteral`'s are `PrimaryExpression's that never need parentheses.
impl NeedsParentheses for JsTemplate {
fn needs_parentheses(&self) -> bool {
false
}

fn needs_parentheses_with_parent(&self, _parent: &JsSyntaxNode) -> bool {
false
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
if self.tag().is_some() {
member_chain_callee_needs_parens(self.clone().into(), parent)
} else {
false
}
}
}
17 changes: 6 additions & 11 deletions crates/rome_js_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,17 +718,12 @@ mod test {
// use this test check if your snippet prints as you wish, without using a snapshot
fn quick_test() {
let src = r#"
let bifornCringerMoshedPerplexSawder=
askTrovenaBeenaDependsRowans=
glimseGlyphsHazardNoopsTieTie=
averredBathersBoxroomBuggyNurl;
let bifornCringerMoshedPerplexSawder =
(askTrovenaBeenaDependsRowans = glimseGlyphsHazardNoopsTieTie =
averredBathersBoxroomBuggyNurl);
it(`handles
some
newlines
does something really long and complicated so I have to write a very long name for the test`, () => {
console.log("hello!");
}, 2500);
"#;
let syntax = SourceType::tsx();
let tree = parse(src, 0, syntax);
Expand Down
24 changes: 15 additions & 9 deletions crates/rome_js_formatter/src/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,19 +299,11 @@ pub(crate) fn is_in_left_hand_side_position(
}
}

pub(crate) fn is_in_assignment_expression_position(
parent: &JsSyntaxNode,
_expression: &JsSyntaxNode,
) -> bool {
matches!(parent.kind(), JsSyntaxKind::JS_SPREAD)
}

pub(crate) fn unary_expression_needs_parentheses(
expression: &JsSyntaxNode,
parent: &JsSyntaxNode,
) -> bool {
is_in_left_hand_side_position(expression, parent)
|| is_in_assignment_expression_position(parent, expression)
is_in_left_hand_side_position(expression, parent) || is_spread(expression, parent)
}

/// Returns `true` if `node< is the `object` of a [JsStaticMemberExpression] or [JsComputedMemberExpression]
Expand Down Expand Up @@ -368,6 +360,8 @@ pub(crate) fn is_conditional_test(node: &JsSyntaxNode, parent: &JsSyntaxNode) ->

/// Returns `true` if `node` is the `tag` of a [JsTemplate] expression
pub(crate) fn is_tag(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool {
debug_assert_is_expression(node);

match parent.kind() {
JsSyntaxKind::JS_TEMPLATE => {
let template = JsTemplate::unwrap_cast(parent.clone());
Expand All @@ -378,6 +372,18 @@ pub(crate) fn is_tag(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool {
}
}

/// Returns `true` if `node` is a spread `...node`
pub(crate) fn is_spread(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool {
debug_assert_is_expression(node);

matches!(
parent.kind(),
JsSyntaxKind::JSX_SPREAD_CHILD
| JsSyntaxKind::JS_SPREAD
| JsSyntaxKind::JSX_SPREAD_ATTRIBUTE
)
}

pub(crate) fn is_binary_like_left_or_right(node: &JsSyntaxNode, parent: &JsSyntaxNode) -> bool {
debug_assert_is_expression(node);

Expand Down
86 changes: 85 additions & 1 deletion crates/rome_js_formatter/src/ts/expressions/as_expression.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use crate::prelude::*;

use crate::parentheses::{
is_binary_like_left_or_right, is_callee, is_member_object, is_spread, is_tag, NeedsParentheses,
};
use rome_formatter::write;
use rome_js_syntax::TsAsExpression;
use rome_js_syntax::TsAsExpressionFields;
use rome_js_syntax::{JsSyntaxKind, JsSyntaxNode, TsAsExpression};

#[derive(Debug, Clone, Default)]
pub struct FormatTsAsExpression;
Expand All @@ -26,4 +29,85 @@ impl FormatNodeRule<TsAsExpression> for FormatTsAsExpression {
]
]
}

fn needs_parentheses(&self, item: &TsAsExpression) -> bool {
item.needs_parentheses()
}
}

impl NeedsParentheses for TsAsExpression {
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
match parent.kind() {
JsSyntaxKind::JS_CONDITIONAL_EXPRESSION
| JsSyntaxKind::JS_EXTENDS_CLAUSE
| JsSyntaxKind::TS_TYPE_ASSERTION_EXPRESSION
| JsSyntaxKind::JS_UNARY_EXPRESSION
| JsSyntaxKind::JS_AWAIT_EXPRESSION
| JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION => true,

_ => {
is_callee(self.syntax(), parent)
|| is_tag(self.syntax(), parent)
|| is_spread(self.syntax(), parent)
|| is_member_object(self.syntax(), parent)
|| is_binary_like_left_or_right(self.syntax(), parent)
}
}
}
}

#[cfg(test)]
mod tests {

use crate::{assert_needs_parentheses, assert_not_needs_parentheses};
use rome_js_syntax::{SourceType, TsAsExpression};

#[test]
fn needs_parentheses() {
assert_needs_parentheses!("5 as number ? true : false", TsAsExpression);
assert_needs_parentheses!("cond ? x as number : false", TsAsExpression);
assert_needs_parentheses!("cond ? true : x as number", TsAsExpression);

assert_needs_parentheses!("class X extends (B as number) {}", TsAsExpression);

assert_needs_parentheses!("(x as Function)()", TsAsExpression);
assert_needs_parentheses!("(x as Function)?.()", TsAsExpression);
assert_needs_parentheses!("new (x as Function)()", TsAsExpression);

assert_needs_parentheses!("<number>(x as any)", TsAsExpression);
assert_needs_parentheses!("(x as any)`template`", TsAsExpression);
assert_needs_parentheses!("!(x as any)", TsAsExpression);
assert_needs_parentheses!("[...(x as any)]", TsAsExpression);
assert_needs_parentheses!("({...(x as any)})", TsAsExpression);
assert_needs_parentheses!(
"<test {...(x as any)} />",
TsAsExpression,
SourceType::tsx()
);
assert_needs_parentheses!(
"<test>{...(x as any)}</test>",
TsAsExpression,
SourceType::tsx()
);
assert_needs_parentheses!("await (x as any)", TsAsExpression);
assert_needs_parentheses!("(x as any)!", TsAsExpression);

assert_needs_parentheses!("(x as any).member", TsAsExpression);
assert_needs_parentheses!("(x as any)[member]", TsAsExpression);
assert_not_needs_parentheses!("object[x as any]", TsAsExpression);

assert_needs_parentheses!("(x as any) + (y as any)", TsAsExpression[0]);
assert_needs_parentheses!("(x as any) + (y as any)", TsAsExpression[1]);

assert_needs_parentheses!("(x as any) && (y as any)", TsAsExpression[0]);
assert_needs_parentheses!("(x as any) && (y as any)", TsAsExpression[1]);

assert_needs_parentheses!("(x as any) in (y as any)", TsAsExpression[0]);
assert_needs_parentheses!("(x as any) in (y as any)", TsAsExpression[1]);

assert_needs_parentheses!("(x as any) instanceof (y as any)", TsAsExpression[0]);
assert_needs_parentheses!("(x as any) instanceof (y as any)", TsAsExpression[1]);

assert_not_needs_parentheses!("x as number as string", TsAsExpression[1]);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::prelude::*;

use crate::js::expressions::static_member_expression::memberish_needs_parens;
use crate::js::expressions::static_member_expression::member_chain_callee_needs_parens;
use crate::parentheses::NeedsParentheses;
use rome_formatter::write;
use rome_js_syntax::{JsSyntaxKind, TsNonNullAssertionExpressionFields};
Expand Down Expand Up @@ -31,6 +31,6 @@ impl FormatNodeRule<TsNonNullAssertionExpression> for FormatTsNonNullAssertionEx
impl NeedsParentheses for TsNonNullAssertionExpression {
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
matches!(parent.kind(), JsSyntaxKind::JS_EXTENDS_CLAUSE)
|| memberish_needs_parens(self.clone().into(), parent)
|| member_chain_callee_needs_parens(self.clone().into(), parent)
}
}
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::TsTypeAssertionExpression;
use rome_js_syntax::TsTypeAssertionExpressionFields;
use rome_js_syntax::{JsSyntaxKind, TsAsExpression, TsTypeAssertionExpressionFields};
use rome_js_syntax::{JsSyntaxNode, TsTypeAssertionExpression};

#[derive(Debug, Clone, Default)]
pub struct FormatTsTypeAssertionExpression;
Expand Down
4 changes: 1 addition & 3 deletions crates/rome_js_formatter/src/utils/member_chain/groups.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ use crate::context::TabWidth;
use crate::parentheses::NeedsParentheses;
use crate::prelude::*;
use crate::utils::member_chain::chain_member::{ChainEntry, ChainMember};
use crate::utils::member_chain::simple_argument::SimpleArgument;
use crate::utils::member_chain::MemberChain;
use rome_formatter::write;
use rome_js_syntax::JsCallExpression;
use rome_rowan::{AstSeparatedList, SyntaxResult};
use rome_rowan::SyntaxResult;
use std::mem;

pub(super) struct MemberChainGroupsBuilder {
Expand Down
Loading

0 comments on commit 6dbe074

Please sign in to comment.