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): member chain parentheses
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Aug 12, 2022
1 parent af3445f commit 80321df
Show file tree
Hide file tree
Showing 60 changed files with 1,574 additions and 1,472 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use crate::parentheses::{
NeedsParentheses,
};
use crate::utils::{
is_simple_expression, resolve_expression, resolve_left_most_expression,
JsAnyBinaryLikeLeftExpression,
resolve_expression, resolve_left_most_expression, JsAnyBinaryLikeLeftExpression,
};
use rome_js_syntax::{
JsAnyArrowFunctionParameters, JsAnyExpression, JsAnyFunctionBody, JsArrowFunctionExpression,
Expand Down Expand Up @@ -35,36 +34,37 @@ impl FormatNodeRule<JsArrowFunctionExpression> for FormatJsArrowFunctionExpressi
body,
} = node.as_fields();

if let Some(async_token) = async_token {
write!(f, [async_token.format(), space()])?;
}
let format_signature = format_with(|f| {
if let Some(async_token) = &async_token {
write!(f, [async_token.format(), space()])?;
}

write!(f, [type_parameters.format()])?;
write!(f, [type_parameters.format()])?;

match parameters? {
JsAnyArrowFunctionParameters::JsAnyBinding(binding) => write!(
f,
[format_parenthesize(
binding.syntax().first_token().as_ref(),
&format_args![binding.format(), if_group_breaks(&text(",")),],
binding.syntax().last_token().as_ref(),
)
.grouped_with_soft_block_indent()]
)?,
JsAnyArrowFunctionParameters::JsParameters(params) => {
write![f, [group(&params.format())]]?
match parameters.as_ref()? {
JsAnyArrowFunctionParameters::JsAnyBinding(binding) => write!(
f,
[format_parenthesize(
binding.syntax().first_token().as_ref(),
&format_args![binding.format(), if_group_breaks(&text(",")),],
binding.syntax().last_token().as_ref(),
)
.grouped_with_soft_block_indent()]
)?,
JsAnyArrowFunctionParameters::JsParameters(params) => {
write![f, [group(&params.format())]]?
}
}
}

write![
f,
[
return_type_annotation.format(),
space(),
fat_arrow_token.format(),
space()
write![
f,
[
return_type_annotation.format(),
space(),
fat_arrow_token.format(),
]
]
]?;
});

let body = body?;

Expand All @@ -87,13 +87,34 @@ impl FormatNodeRule<JsArrowFunctionExpression> for FormatJsArrowFunctionExpressi
// going to get broken anyways.
let body_has_soft_line_break = match &body {
JsFunctionBody(_) => true,
JsAnyExpression(expr) => match expr {
JsAnyExpression(expr) => match resolve_expression(expr.clone()) {
JsArrowFunctionExpression(_)
| JsArrayExpression(_)
| JsObjectExpression(_)
| JsParenthesizedExpression(_)
| JsxTagExpression(_) => true,
expr => is_simple_expression(expr)?,
JsSequenceExpression(_) => {
return write!(
f,
[group(&format_args![
format_signature,
group(&format_args![
space(),
text("("),
soft_block_indent(&body.format()),
text(")")
])
])]
);
// // We handle sequence expressions as the body of arrows specially,
// // so that the required parentheses end up on their own lines.
// if (node.body.type === "SequenceExpression") {
// return group([
// ...parts,
// group([" (", indent([softline, body]), softline, ")"]),
// ]);
// }
}
_ => false,
},
};

Expand All @@ -118,23 +139,26 @@ impl FormatNodeRule<JsArrowFunctionExpression> for FormatJsArrowFunctionExpressi
};

if body_has_soft_line_break && !should_add_parens {
write![f, [body.format()]]
write![f, [format_signature, space(), body.format()]]
} else {
write!(
f,
[group(&soft_line_indent_or_space(&format_with(|f| {
if should_add_parens {
write!(f, [if_group_fits_on_line(&text("("))])?;
}

write!(f, [body.format()])?;

if should_add_parens {
write!(f, [if_group_fits_on_line(&text(")"))])?;
}

Ok(())
})))]
[
format_signature,
group(&soft_line_indent_or_space(&format_with(|f| {
if should_add_parens {
write!(f, [if_group_fits_on_line(&text("("))])?;
}

write!(f, [body.format()])?;

if should_add_parens {
write!(f, [if_group_fits_on_line(&text(")"))])?;
}

Ok(())
})))
]
)
}
}
Expand Down Expand Up @@ -163,7 +187,7 @@ impl NeedsParentheses for JsArrowFunctionExpression {

#[cfg(test)]
mod tests {
use crate::parentheses::NeedsParentheses;

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl NeedsParentheses for JsAssignmentExpression {
JsSyntaxKind::JS_EXPRESSION_STATEMENT => {
// Parenthesize `{ a } = { a: 5 }`
is_first_in_statement(
self.syntax(),
self.clone().into(),
FirstInStatementMode::ExpressionStatementOrArrow,
) && matches!(
self.left(),
Expand Down Expand Up @@ -95,7 +95,7 @@ impl NeedsParentheses for JsAssignmentExpression {

#[cfg(test)]
mod tests {
use crate::parentheses::NeedsParentheses;

use crate::{assert_needs_parentheses, assert_not_needs_parentheses};
use rome_js_syntax::JsAssignmentExpression;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(super) fn await_or_yield_needs_parens(parent: &JsSyntaxNode, node: &JsSyntax

#[cfg(test)]
mod tests {
use crate::parentheses::NeedsParentheses;

use crate::{assert_needs_parentheses, assert_not_needs_parentheses};
use rome_js_syntax::JsAwaitExpression;

Expand Down
86 changes: 38 additions & 48 deletions crates/rome_js_formatter/src/js/expressions/call_expression.rs
Original file line number Diff line number Diff line change
@@ -1,59 +1,49 @@
use crate::prelude::*;
use crate::utils::format_call_expression;

use crate::parentheses::{resolve_left_most_expression, NeedsParentheses};
use crate::parentheses::NeedsParentheses;
use crate::utils::get_member_chain;
use rome_js_syntax::{JsCallExpression, JsSyntaxKind, JsSyntaxNode};
use rome_rowan::AstNode;

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

impl FormatNodeRule<JsCallExpression> for FormatJsCallExpression {
fn fmt_fields(&self, node: &JsCallExpression, formatter: &mut JsFormatter) -> FormatResult<()> {
format_call_expression(node.syntax(), formatter)
fn fmt_fields(&self, node: &JsCallExpression, f: &mut JsFormatter) -> FormatResult<()> {
let member_chain = get_member_chain(node, f)?;

member_chain.fmt(f)
}

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

impl NeedsParentheses for JsCallExpression {
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
match parent.kind() {
JsSyntaxKind::JS_NEW_EXPRESSION => true,

_ => false,
}
}
}

// impl NeedsParentheses for JsCallExpression {
// fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
// match parent.kind() {
// JsSyntaxKind::JS_EXPRESSION_STATEMENT | JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION => {
// self.callee()
// .map_or(false, |callee| starts_with_no_lookahead_token(&callee))
// }
// _ => {}
// }
//
// // case "CallExpression":
// // case "MemberExpression":
// // case "TaggedTemplateExpression":
// // case "TSNonNullExpression":
// // if (
// // name === "callee" &&
// // (parent.type === "BindExpression" || parent.type === "NewExpression")
// // ) {
// // let object = node;
// // while (object) {
// // switch (object.type) {
// // case "CallExpression":
// // case "OptionalCallExpression":
// // return true;
// // case "MemberExpression":
// // case "OptionalMemberExpression":
// // case "BindExpression":
// // object = object.object;
// // break;
// // // tagged templates are basically member expressions from a grammar perspective
// // // see https://tc39.github.io/ecma262/#prod-MemberExpression
// // case "TaggedTemplateExpression":
// // object = object.tag;
// // break;
// // case "TSNonNullExpression":
// // object = object.expression;
// // break;
// // default:
// // return false;
// // }
// // }
// }
// }
#[cfg(test)]
mod tests {

use crate::{assert_needs_parentheses, assert_not_needs_parentheses};
use rome_js_syntax::JsCallExpression;

#[test]
fn needs_parentheses() {
assert_needs_parentheses!("new (call())()", JsCallExpression);

assert_not_needs_parentheses!("a?.()!.c", JsCallExpression);
assert_not_needs_parentheses!("(a?.())!.c", JsCallExpression);

assert_not_needs_parentheses!("(call())()", JsCallExpression[1]);
assert_not_needs_parentheses!("getLogger().error(err);", JsCallExpression[0]);
assert_not_needs_parentheses!("getLogger().error(err);", JsCallExpression[1]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ impl NeedsParentheses for JsClassExpression {
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
is_callee(self.syntax(), parent)
|| is_first_in_statement(
self.syntax(),
self.clone().into(),
FirstInStatementMode::ExpressionOrExportDefault,
)
}
}

#[cfg(test)]
mod tests {
use crate::parentheses::NeedsParentheses;

use crate::{assert_needs_parentheses, assert_not_needs_parentheses};
use rome_js_syntax::JsClassExpression;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use crate::prelude::*;

use crate::js::expressions::static_member_expression::memberish_needs_parens;
use crate::parentheses::NeedsParentheses;
use rome_formatter::{format_args, write};
use rome_js_syntax::JsComputedMemberExpression;
use rome_js_syntax::JsComputedMemberExpressionFields;
use rome_js_syntax::{JsComputedMemberExpression, JsSyntaxNode};
use rome_js_syntax::{JsComputedMemberExpressionFields, JsSyntaxKind};

#[derive(Debug, Clone, Default)]
pub struct FormatJsComputedMemberExpression;
Expand Down Expand Up @@ -35,4 +37,38 @@ impl FormatNodeRule<JsComputedMemberExpression> for FormatJsComputedMemberExpres
]
]
}

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

impl NeedsParentheses for JsComputedMemberExpression {
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
if self.is_optional_chain() && matches!(parent.kind(), JsSyntaxKind::JS_NEW_EXPRESSION) {
return true;
}

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

#[cfg(test)]
mod tests {

use crate::{assert_needs_parentheses, assert_not_needs_parentheses};
use rome_js_syntax::JsComputedMemberExpression;

#[test]
fn needs_parentheses() {
assert_needs_parentheses!("new (test()[a])()", JsComputedMemberExpression);
assert_needs_parentheses!("new (test().a[b])()", JsComputedMemberExpression);
assert_needs_parentheses!(
"new (test()`template`[index])()",
JsComputedMemberExpression
);
assert_needs_parentheses!("new (test()![member])()", JsComputedMemberExpression);

assert_not_needs_parentheses!("new (test[a])()", JsComputedMemberExpression);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::utils::JsAnyConditional;

use crate::parentheses::{
is_binary_like_left_or_right, is_conditional_test, is_in_left_hand_side_position,
resolve_left_most_expression, NeedsParentheses,
NeedsParentheses,
};
use rome_js_syntax::{JsConditionalExpression, JsSyntaxKind, JsSyntaxNode};

Expand Down Expand Up @@ -45,7 +45,7 @@ impl NeedsParentheses for JsConditionalExpression {

#[cfg(test)]
mod tests {
use crate::parentheses::NeedsParentheses;

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

Expand Down
Loading

0 comments on commit 80321df

Please sign in to comment.