Skip to content

Commit

Permalink
challenge(formatter): handle let variable name in non-strict mode (#814)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos committed Nov 21, 2023
1 parent d1e6290 commit 478074f
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 1,138 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::prelude::*;
use biome_formatter::write;

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

#[derive(Debug, Clone, Default)]
Expand All @@ -23,16 +23,14 @@ impl FormatNodeRule<JsIdentifierAssignment> for FormatJsIdentifierAssignment {
impl NeedsParentheses for JsIdentifierAssignment {
#[inline]
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
let is_async = self
.name_token()
.map_or(false, |name| name.text_trimmed() == "async");

if is_async && JsForOfStatement::can_cast(parent.kind()) {
let for_of = JsForOfStatement::unwrap_cast(parent.clone());

for_of.await_token().is_none()
} else {
false
let Ok(name) = self.name_token() else {
return false;
};
match name.text_trimmed() {
"async" => JsForOfStatement::cast_ref(parent)
.is_some_and(|for_of| for_of.await_token().is_none()),
"let" => parent.kind() == JsSyntaxKind::JS_FOR_OF_STATEMENT,
_ => false,
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::prelude::*;

use crate::parentheses::NeedsParentheses;
use crate::ts::expressions::as_expression::TsAsOrSatisfiesExpression;
use biome_formatter::write;
use biome_js_syntax::{JsIdentifierExpression, JsSyntaxNode};
use biome_js_syntax::assign_ext::AnyJsMemberAssignment;
use biome_js_syntax::{AnyJsExpression, JsIdentifierExpression, JsSyntaxNode};
use biome_js_syntax::{JsIdentifierExpressionFields, JsSyntaxKind};
use biome_rowan::SyntaxNodeOptionExt;

Expand All @@ -24,27 +24,72 @@ impl FormatNodeRule<JsIdentifierExpression> for FormatJsIdentifierExpression {

impl NeedsParentheses for JsIdentifierExpression {
fn needs_parentheses_with_parent(&self, parent: &JsSyntaxNode) -> bool {
// edge case: handle cases such as
// `(type) as unknown satisfies unknown`
if TsAsOrSatisfiesExpression::can_cast(parent.kind())
&& parent
.ancestors()
.skip(1)
.find(|x| !TsAsOrSatisfiesExpression::can_cast(x.kind()))
.kind()
== Some(JsSyntaxKind::JS_EXPRESSION_STATEMENT)
{
self.name()
.and_then(|x| x.value_token())
.map_or(false, |name| {
// These keywords are contextually reserved by TypeSCript in strict and sloppy modes.
matches!(
name.text_trimmed(),
"await" | "interface" | "let" | "module" | "type" | "yield" | "using"
)
})
} else {
false
let Ok(name) = self.name().and_then(|x| x.value_token()) else {
return false;
};
// In non-strict mode (sloppy mode), `let` may be a variable name.
// To disambiguate `let` from a let-declaration,
// we have to enclose `let` between parentheses in some dge cases.
match parent.kind() {
JsSyntaxKind::JS_COMPUTED_MEMBER_EXPRESSION
| JsSyntaxKind::JS_COMPUTED_MEMBER_ASSIGNMENT => {
// `(let)[0];`
// `(let)[0] = 0;`
// `for( (let)[0] = 0, b = 0;;;) {}`
// `for( (let)[0] of []) {}`
name.text_trimmed() == "let"
&& parent
.ancestors()
.find(|x| {
!AnyJsExpression::can_cast(x.kind())
&& !AnyJsMemberAssignment::can_cast(x.kind())
})
.filter(|x| {
matches!(
x.kind(),
JsSyntaxKind::JS_EXPRESSION_STATEMENT
| JsSyntaxKind::JS_FOR_IN_STATEMENT
| JsSyntaxKind::JS_FOR_OF_STATEMENT
| JsSyntaxKind::JS_FOR_STATEMENT
)
})
.and_then(|x| x.first_child()?.first_token())
.is_some_and(|token| token == name)
}
JsSyntaxKind::JS_CALL_EXPRESSION
| JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION
| JsSyntaxKind::JS_STATIC_MEMBER_ASSIGNMENT => {
// for ( (let).a of [] ) {}
name.text_trimmed() == "let"
&& parent
.ancestors()
.find(|x| {
!AnyJsExpression::can_cast(x.kind())
&& !AnyJsMemberAssignment::can_cast(x.kind())
})
.filter(|x| x.kind() == JsSyntaxKind::JS_FOR_OF_STATEMENT)
.and_then(|x| x.first_child()?.first_token())
.is_some_and(|token| token == name)
}
JsSyntaxKind::TS_AS_EXPRESSION | JsSyntaxKind::TS_SATISFIES_EXPRESSION => {
// `(let) as unknown satisfies unknown;`
// `(type) as unknown satisfies unknown;`
matches!(
name.text_trimmed(),
"await" | "interface" | "let" | "module" | "type" | "using" | "yield"
) && parent
.ancestors()
.skip(1)
.find(|x| {
!matches!(
x.kind(),
JsSyntaxKind::TS_AS_EXPRESSION | JsSyntaxKind::TS_SATISFIES_EXPRESSION
)
})
.kind()
== Some(JsSyntaxKind::JS_EXPRESSION_STATEMENT)
}
_ => false,
}
}
}
6 changes: 1 addition & 5 deletions crates/biome_js_formatter/tests/prettier_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,7 @@ fn test_snapshot(input: &'static str, _: &str, _: &str, _: &str) {
}

fn is_non_strict_mode(root_path: &Path, file_path: &Path) -> bool {
let test_cases_paths = [
"js/with",
"js/sloppy-mode",
"js/identifier/parentheses/const.js",
];
let test_cases_paths = ["js/with/", "js/sloppy-mode/", "js/identifier/"];

test_cases_paths.iter().any(|path| {
file_path
Expand Down

This file was deleted.

0 comments on commit 478074f

Please sign in to comment.