Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(js_formatter): move NeedsParentheses trait to biome_js_syntax #3541

Merged
merged 2 commits into from
Jul 30, 2024
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,6 +1,6 @@
use crate::parentheses::NeedsParentheses;
use crate::prelude::*;
use biome_formatter::write;
use biome_js_syntax::parentheses::NeedsParentheses;
use biome_js_syntax::JsArrayAssignmentPattern;
use biome_js_syntax::JsArrayAssignmentPatternFields;

Expand Down Expand Up @@ -42,10 +42,3 @@ impl FormatNodeRule<JsArrayAssignmentPattern> for FormatJsArrayAssignmentPattern
Ok(())
}
}

impl NeedsParentheses for JsArrayAssignmentPattern {
#[inline]
fn needs_parentheses(&self) -> bool {
false
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::prelude::*;

use crate::parentheses::NeedsParentheses;
use biome_js_syntax::parentheses::NeedsParentheses;
use biome_js_syntax::{AnyJsComputedMember, JsComputedMemberAssignment};

#[derive(Debug, Clone, Default)]
Expand All @@ -19,10 +19,3 @@ impl FormatNodeRule<JsComputedMemberAssignment> for FormatJsComputedMemberAssign
item.needs_parentheses()
}
}

impl NeedsParentheses for JsComputedMemberAssignment {
#[inline]
fn needs_parentheses(&self) -> bool {
false
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use crate::prelude::*;
use biome_formatter::write;

use crate::parentheses::NeedsParentheses;
use biome_js_syntax::JsIdentifierAssignment;
use biome_js_syntax::{JsForOfStatement, JsIdentifierAssignmentFields, JsSyntaxKind};
use biome_js_syntax::parentheses::NeedsParentheses;
use biome_js_syntax::{JsIdentifierAssignment, JsIdentifierAssignmentFields};

#[derive(Debug, Clone, Default)]
pub(crate) struct FormatJsIdentifierAssignment;
Expand All @@ -20,25 +19,6 @@ impl FormatNodeRule<JsIdentifierAssignment> for FormatJsIdentifierAssignment {
}
}

impl NeedsParentheses for JsIdentifierAssignment {
#[inline]
fn needs_parentheses(&self) -> bool {
let Ok(name) = self.name_token() else {
return false;
};
match name.text_trimmed() {
"async" => self
.parent::<JsForOfStatement>()
.is_some_and(|for_of| for_of.await_token().is_none()),
"let" => self
.syntax()
.parent()
.is_some_and(|parent| parent.kind() == JsSyntaxKind::JS_FOR_OF_STATEMENT),
_ => false,
}
}
}

#[cfg(test)]
mod tests {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::parentheses::NeedsParentheses;
use crate::prelude::*;
use crate::utils::JsObjectPatternLike;

use biome_formatter::write;
use biome_js_syntax::parentheses::NeedsParentheses;
use biome_js_syntax::JsObjectAssignmentPattern;

#[derive(Debug, Clone, Default)]
Expand Down Expand Up @@ -29,10 +30,3 @@ impl FormatNodeRule<JsObjectAssignmentPattern> for FormatJsObjectAssignmentPatte
Ok(())
}
}

impl NeedsParentheses for JsObjectAssignmentPattern {
#[inline]
fn needs_parentheses(&self) -> bool {
false
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::parentheses::NeedsParentheses;
use crate::prelude::*;

use biome_formatter::write;
use biome_js_syntax::JsParenthesizedAssignment;
use biome_js_syntax::JsParenthesizedAssignmentFields;
Expand Down Expand Up @@ -28,15 +28,4 @@ impl FormatNodeRule<JsParenthesizedAssignment> for FormatJsParenthesizedAssignme
]
]
}

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

impl NeedsParentheses for JsParenthesizedAssignment {
#[inline]
fn needs_parentheses(&self) -> bool {
false
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::js::expressions::static_member_expression::AnyJsStaticMemberLike;
use crate::parentheses::NeedsParentheses;
use crate::prelude::*;

use biome_js_syntax::parentheses::NeedsParentheses;
use biome_js_syntax::JsStaticMemberAssignment;

#[derive(Debug, Clone, Default)]
Expand All @@ -15,10 +16,3 @@ impl FormatNodeRule<JsStaticMemberAssignment> for FormatJsStaticMemberAssignment
item.needs_parentheses()
}
}

impl NeedsParentheses for JsStaticMemberAssignment {
#[inline]
fn needs_parentheses(&self) -> bool {
false
}
}
8 changes: 0 additions & 8 deletions crates/biome_js_formatter/src/js/bogus/bogus_assignment.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@
use crate::parentheses::NeedsParentheses;
use crate::FormatBogusNodeRule;
use biome_js_syntax::JsBogusAssignment;

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

impl FormatBogusNodeRule<JsBogusAssignment> for FormatJsBogusAssignment {}

impl NeedsParentheses for JsBogusAssignment {
#[inline]
fn needs_parentheses(&self) -> bool {
false
}
}
8 changes: 0 additions & 8 deletions crates/biome_js_formatter/src/js/bogus/bogus_expression.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@
use crate::parentheses::NeedsParentheses;
use crate::FormatBogusNodeRule;
use biome_js_syntax::JsBogusExpression;

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

impl FormatBogusNodeRule<JsBogusExpression> for FormatJsBogusExpression {}

impl NeedsParentheses for JsBogusExpression {
#[inline]
fn needs_parentheses(&self) -> bool {
false
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::prelude::*;

use crate::parentheses::NeedsParentheses;
use biome_formatter::{write, FormatRuleWithOptions};
use biome_js_syntax::parentheses::NeedsParentheses;
use biome_js_syntax::JsArrayExpression;
use biome_js_syntax::{
AnyJsArrayElement, AnyJsExpression, JsArrayElementList, JsArrayExpressionFields,
Expand Down Expand Up @@ -122,10 +122,3 @@ fn should_break(elements: &JsArrayElementList) -> SyntaxResult<bool> {
Ok(true)
}
}

impl NeedsParentheses for JsArrayExpression {
#[inline(always)]
fn needs_parentheses(&self) -> bool {
false
}
}
Original file line number Diff line number Diff line change
@@ -1,24 +1,22 @@
use crate::context::trailing_commas::FormatTrailingCommas;
use crate::js::bindings::parameters::has_only_simple_parameters;
use crate::js::expressions::call_arguments::GroupedCallArgumentLayout;
use crate::prelude::*;
use crate::utils::function_body::{FormatMaybeCachedFunctionBody, FunctionBodyCacheMode};
use crate::utils::AssignmentLikeLayout;

use biome_formatter::{
format_args, write, CstFormatContext, FormatRuleWithOptions, RemoveSoftLinesBuffer,
};
use std::iter::once;

use crate::context::trailing_commas::FormatTrailingCommas;
use crate::js::expressions::call_arguments::GroupedCallArgumentLayout;
use crate::parentheses::{
is_callee, is_conditional_test, update_or_lower_expression_needs_parentheses,
AnyJsExpressionLeftSide, NeedsParentheses,
};
use crate::utils::function_body::{FormatMaybeCachedFunctionBody, FunctionBodyCacheMode};
use crate::utils::{resolve_left_most_expression, AnyJsBinaryLikeExpression, AssignmentLikeLayout};
use biome_js_syntax::expression_left_side::AnyJsExpressionLeftSide;
use biome_js_syntax::parentheses::NeedsParentheses;
use biome_js_syntax::{
is_test_call_argument, AnyJsArrowFunctionParameters, AnyJsBindingPattern, AnyJsExpression,
AnyJsFormalParameter, AnyJsFunctionBody, AnyJsParameter, AnyJsTemplateElement,
JsArrowFunctionExpression, JsFormalParameter, JsSyntaxKind, JsTemplateExpression,
};
use biome_rowan::{SyntaxNodeOptionExt, SyntaxResult};
use std::iter::once;

#[derive(Debug, Copy, Clone, Default)]
pub(crate) struct FormatJsArrowFunctionExpression {
Expand Down Expand Up @@ -368,14 +366,15 @@ fn should_add_parens(body: &AnyJsFunctionBody) -> bool {
AnyJsFunctionBody::AnyJsExpression(
expression @ AnyJsExpression::JsConditionalExpression(_),
) => {
let are_parentheses_mandatory = matches!(
resolve_left_most_expression(expression.clone()),
let var_name = matches!(
AnyJsExpressionLeftSide::leftmost(expression.clone()),
AnyJsExpressionLeftSide::AnyJsExpression(
AnyJsExpression::JsObjectExpression(_)
| AnyJsExpression::JsFunctionExpression(_)
| AnyJsExpression::JsClassExpression(_)
)
);
let are_parentheses_mandatory = var_name;

!are_parentheses_mandatory
}
Expand Down Expand Up @@ -504,9 +503,12 @@ impl Format<JsFormatContext> for ArrowChain {
// () => () =>
// a
// )();
let is_callee = head_parent
.as_ref()
.map_or(false, |parent| is_callee(head.syntax(), parent));
let is_callee = head_parent.as_ref().map_or(false, |parent| {
matches!(
parent.kind(),
JsSyntaxKind::JS_CALL_EXPRESSION | JsSyntaxKind::JS_NEW_EXPRESSION
)
});

// With arrays, objects, sequence expressions, and block function bodies,
// the opening brace gives a convenient boundary to insert a line break,
Expand Down Expand Up @@ -820,24 +822,6 @@ impl ArrowFunctionLayout {
}
}

impl NeedsParentheses for JsArrowFunctionExpression {
fn needs_parentheses(&self) -> bool {
let Some(parent) = self.syntax().parent() else {
return false;
};
matches!(
parent.kind(),
JsSyntaxKind::TS_AS_EXPRESSION
| JsSyntaxKind::TS_SATISFIES_EXPRESSION
| JsSyntaxKind::JS_UNARY_EXPRESSION
| JsSyntaxKind::JS_AWAIT_EXPRESSION
| JsSyntaxKind::TS_TYPE_ASSERTION_EXPRESSION
) || is_conditional_test(self.syntax(), &parent)
|| update_or_lower_expression_needs_parentheses(self.syntax(), &parent)
|| AnyJsBinaryLikeExpression::can_cast(parent.kind())
}
}

/// Returns `true` if the template contains any new lines inside of its text chunks.
fn template_literal_contains_new_line(template: &JsTemplateExpression) -> bool {
template.elements().iter().any(|element| match element {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
use crate::prelude::*;
use crate::utils::AnyJsAssignmentLike;

use crate::parentheses::{
is_arrow_function_body, is_first_in_statement, FirstInStatementMode, NeedsParentheses,
};
use biome_formatter::write;

use biome_js_syntax::{
AnyJsAssignmentPattern, AnyJsForInitializer, JsArrowFunctionExpression, JsAssignmentExpression,
JsComputedMemberName, JsExpressionStatement, JsForStatement, JsSequenceExpression,
JsSyntaxKind,
};
use biome_rowan::{match_ast, AstNode};
use biome_js_syntax::parentheses::NeedsParentheses;
use biome_js_syntax::JsAssignmentExpression;

#[derive(Debug, Clone, Default)]
pub(crate) struct FormatJsAssignmentExpression;
Expand All @@ -26,84 +18,6 @@ impl FormatNodeRule<JsAssignmentExpression> for FormatJsAssignmentExpression {
}
}

impl NeedsParentheses for JsAssignmentExpression {
fn needs_parentheses(&self) -> bool {
let Some(parent) = self.syntax().parent() else {
return false;
};
match_ast! {
match &parent {
JsAssignmentExpression(_) => false,
// `[a = b]`
JsComputedMemberName(_) => false,

JsArrowFunctionExpression(_) => {
is_arrow_function_body(self.syntax(), parent)
},

JsForStatement(for_statement) => {
let is_initializer = match for_statement.initializer() {
Some(AnyJsForInitializer::AnyJsExpression(expression)) => {
expression.syntax() == self.syntax()
}
None | Some(_) => false,
};

let is_update = for_statement
.update()
.map(AstNode::into_syntax)
.as_ref()
== Some(self.syntax());

!(is_initializer || is_update)
},
JsExpressionStatement(_) => {
// Parenthesize `{ a } = { a: 5 }`
is_first_in_statement(
self.clone().into(),
FirstInStatementMode::ExpressionStatementOrArrow,
) && matches!(
self.left(),
Ok(AnyJsAssignmentPattern::JsObjectAssignmentPattern(_))
)
},
JsSequenceExpression(_) => {
let mut child = parent.clone();

for ancestor in parent.ancestors().skip(1) {
match ancestor.kind() {
JsSyntaxKind::JS_SEQUENCE_EXPRESSION
| JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION => child = ancestor,
JsSyntaxKind::JS_FOR_STATEMENT => {
let for_statement = JsForStatement::unwrap_cast(ancestor);

let is_initializer = match for_statement.initializer() {
Some(AnyJsForInitializer::AnyJsExpression(expression)) => {
expression.syntax() == &child
}
None | Some(_) => false,
};

let is_update =
for_statement.update().map(AstNode::into_syntax).as_ref()
== Some(&child);

return !(is_initializer || is_update);
}
_ => break,
}
}

true
},
_ => {
true
}
}
}
}
}

#[cfg(test)]
mod tests {

Expand Down
Loading
Loading