Skip to content

Commit

Permalink
challenge(formatter): fix uneeded parens on decoaretd class expr (#746)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos committed Nov 16, 2023
1 parent 1c26f3f commit 430567f
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 507 deletions.
10 changes: 6 additions & 4 deletions crates/biome_js_formatter/src/js/expressions/class_expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ use biome_formatter::{format_args, write};
use crate::parentheses::{
is_callee, is_first_in_statement, FirstInStatementMode, NeedsParentheses,
};
use biome_js_syntax::{JsClassExpression, JsSyntaxNode};
use biome_js_syntax::{JsClassExpression, JsSyntaxKind, JsSyntaxNode};

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

impl FormatNodeRule<JsClassExpression> for FormatJsClassExpression {
fn fmt_fields(&self, node: &JsClassExpression, f: &mut JsFormatter) -> FormatResult<()> {
if node.decorators().is_empty() {
if node.decorators().is_empty() || !self.needs_parentheses(node) {
FormatClass::from(&node.clone().into()).fmt(f)
} else {
write!(
Expand All @@ -29,7 +29,8 @@ impl FormatNodeRule<JsClassExpression> for FormatJsClassExpression {
}

fn needs_parentheses(&self, item: &JsClassExpression) -> bool {
!item.decorators().is_empty() || item.needs_parentheses()
/*!item.decorators().is_empty() || */
item.needs_parentheses()
}

fn fmt_dangling_comments(
Expand All @@ -44,7 +45,8 @@ impl FormatNodeRule<JsClassExpression> for FormatJsClassExpression {

impl NeedsParentheses for JsClassExpression {
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
is_callee(self.syntax(), parent)
(parent.kind() == JsSyntaxKind::JS_EXTENDS_CLAUSE && !self.decorators().is_empty())
|| is_callee(self.syntax(), parent)
|| is_first_in_statement(
self.clone().into(),
FirstInStatementMode::ExpressionOrExportDefault,
Expand Down
19 changes: 18 additions & 1 deletion crates/biome_js_formatter/src/syntax_rewriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl JsFormatSyntaxRewriter {
// Keep parentheses around unknown expressions. Biome can't know the precedence.
if inner.kind().is_bogus()
// Don't remove parentheses if the expression is a decorator
|| inner.grand_parent().map_or(false, |node| node.kind() == JsSyntaxKind::JS_DECORATOR)
|| inner.grand_parent().map_or(false, |node| node.kind() == JsSyntaxKind::JS_DECORATOR && decorator_expression_needs_parens(&inner))
// Don't remove parentheses if they have skipped trivia. We don't know for certain what the intended syntax is.
// Nor if there's a leading type cast comment
|| has_type_cast_comment_or_skipped(&l_paren.leading_trivia())
Expand Down Expand Up @@ -403,6 +403,23 @@ impl JsFormatSyntaxRewriter {
}
}

// TODO: This should be handled with a `NeedsParentheses` impl.
fn decorator_expression_needs_parens(inner: &JsSyntaxNode) -> bool {
match AnyJsExpression::cast_ref(inner) {
Some(AnyJsExpression::JsCallExpression(call)) => call
.callee()
.map(|callee| decorator_expression_needs_parens(callee.syntax()))
.unwrap_or(true),
Some(AnyJsExpression::JsStaticMemberExpression(static_expr)) => {
static_expr.is_optional()
|| static_expr.object().map_or(true, |object| {
object.syntax().kind() != JsSyntaxKind::JS_IDENTIFIER_EXPRESSION
})
}
_ => true,
}
}

impl SyntaxRewriter for JsFormatSyntaxRewriter {
type Language = JsLanguage;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,28 +51,20 @@ Arrow parentheses: Always

```js
console.log(
(
@deco
class Foo {}
),
@deco
class Foo {},
);
console.log(
(
@deco
class {}
),
@deco
class {},
);

const a1 =
(
@deco
class Foo {}
);
@deco
class Foo {};
const a2 =
(
@deco
class {}
);
@deco
class {};

(
@deco
Expand All @@ -94,10 +86,8 @@ const b2 = [];
@deco
class {}
)(
(
@deco
class Foo {}
),
@deco
class Foo {},
).name;
(
@deco
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

0 comments on commit 430567f

Please sign in to comment.