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

Add missing parentheses to let #14000

Merged
merged 19 commits into from Dec 22, 2022
19 changes: 19 additions & 0 deletions changelog_unreleased/javascript/14000.md
@@ -0,0 +1,19 @@
#### Fix missing parentheses when an expression statement starts with `let[` (#14000 by @fisker, @thorn0)

<!-- prettier-ignore -->
```jsx
// Input
(let[0] = 2);

// Prettier stable
let[0] = 2;

// Prettier stable (second format)
SyntaxError: Unexpected token (1:5)
> 1 | let[0] = 2;
| ^
2 |

// Prettier main
(let)[0] = 2;
```
4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -28,7 +28,7 @@
"@glimmer/syntax": "0.84.2",
"@iarna/toml": "2.2.5",
"@typescript-eslint/typescript-estree": "5.45.0",
"acorn": "8.8.0",
"acorn": "8.8.1",
"acorn-jsx": "5.3.2",
"angular-estree-parser": "2.5.1",
"angular-html-parser": "1.8.0",
Expand All @@ -44,7 +44,7 @@
"editorconfig": "0.15.3",
"editorconfig-to-prettier": "0.2.0",
"escape-string-regexp": "5.0.0",
"espree": "9.4.0",
"espree": "9.4.1",
"esutils": "2.0.3",
"fast-glob": "3.2.11",
"fast-json-stable-stringify": "2.1.0",
Expand Down
41 changes: 39 additions & 2 deletions src/language-js/needs-parens.js
Expand Up @@ -77,6 +77,39 @@ function needsParens(path, options) {
return true;
}

// `(let)[a] = 1`
if (
name === "object" &&
node.name === "let" &&
parent.type === "MemberExpression" &&
parent.computed &&
!parent.optional
) {
const statement = path.findAncestor(
(node) =>
node.type === "ExpressionStatement" ||
node.type === "ForStatement" ||
node.type === "ForInStatement" ||
node.type === "ForOfStatement"
);
const expression = !statement
? undefined
: statement.type === "ExpressionStatement"
? statement.expression
: statement.type === "ForStatement"
? statement.init
: statement.left;
if (
expression &&
startsWithNoLookaheadToken(
expression,
(leftmostNode) => leftmostNode === node
)
) {
return true;
}
}

return false;
}

Expand Down Expand Up @@ -157,7 +190,11 @@ function needsParens(path, options) {
if (
startsWithNoLookaheadToken(
node,
/* forbidFunctionClassAndDoExpr */ true
(node) =>
node.type === "ObjectExpression" ||
node.type === "FunctionExpression" ||
node.type === "ClassExpression" ||
node.type === "DoExpression"
)
) {
return true;
Expand All @@ -170,7 +207,7 @@ function needsParens(path, options) {
node.type !== "SequenceExpression" && // these have parens added anyway
startsWithNoLookaheadToken(
node,
/* forbidFunctionClassAndDoExpr */ false
(node) => node.type === "ObjectExpression"
)
) {
return true;
Expand Down
5 changes: 4 additions & 1 deletion src/language-js/print/function.js
Expand Up @@ -373,7 +373,10 @@ function printArrowFunction(path, options, print, args) {
// a <= a ? a : a
const shouldAddParens =
node.body.type === "ConditionalExpression" &&
!startsWithNoLookaheadToken(node.body, /* forbidFunctionAndClass */ false);
!startsWithNoLookaheadToken(
node.body,
(node) => node.type === "ObjectExpression"
);

return group([
...parts,
Expand Down
72 changes: 25 additions & 47 deletions src/language-js/utils/index.js
Expand Up @@ -939,74 +939,59 @@ function shouldPrintComma(options, level = "es5") {
}

/**
* Tests if an expression starts with `{`, or (if forbidFunctionClassAndDoExpr
* holds) `function`, `class`, or `do {}`. Will be overzealous if there's
* already necessary grouping parentheses.
* Tests if the leftmost node of the expression matches the predicate. E.g.,
* used to check whether an expression statement needs to be wrapped in extra
* parentheses because it starts with:
*
* - `{`
* - `function`, `class`, or `do {}`
* - `let[`
*
* Will be overzealous if there already are necessary grouping parentheses.
*
* @param {Node} node
* @param {boolean} forbidFunctionClassAndDoExpr
* @param {(leftmostNode: Node) => boolean} predicate
* @returns {boolean}
*/
function startsWithNoLookaheadToken(node, forbidFunctionClassAndDoExpr) {
node = getLeftMost(node);
function startsWithNoLookaheadToken(node, predicate) {
switch (node.type) {
case "FunctionExpression":
case "ClassExpression":
case "DoExpression":
return forbidFunctionClassAndDoExpr;
case "ObjectExpression":
return true;
case "BinaryExpression":
case "LogicalExpression":
case "AssignmentExpression":
case "NGPipeExpression":
return startsWithNoLookaheadToken(node.left, predicate);
case "MemberExpression":
case "OptionalMemberExpression":
return startsWithNoLookaheadToken(
node.object,
forbidFunctionClassAndDoExpr
);
return startsWithNoLookaheadToken(node.object, predicate);
case "TaggedTemplateExpression":
if (node.tag.type === "FunctionExpression") {
// IIFEs are always already parenthesized
return false;
}
return startsWithNoLookaheadToken(node.tag, forbidFunctionClassAndDoExpr);
return startsWithNoLookaheadToken(node.tag, predicate);
case "CallExpression":
case "OptionalCallExpression":
if (node.callee.type === "FunctionExpression") {
// IIFEs are always already parenthesized
return false;
}
return startsWithNoLookaheadToken(
node.callee,
forbidFunctionClassAndDoExpr
);
return startsWithNoLookaheadToken(node.callee, predicate);
case "ConditionalExpression":
return startsWithNoLookaheadToken(
node.test,
forbidFunctionClassAndDoExpr
);
return startsWithNoLookaheadToken(node.test, predicate);
case "UpdateExpression":
return (
!node.prefix &&
startsWithNoLookaheadToken(node.argument, forbidFunctionClassAndDoExpr)
!node.prefix && startsWithNoLookaheadToken(node.argument, predicate)
);
case "BindExpression":
return (
node.object &&
startsWithNoLookaheadToken(node.object, forbidFunctionClassAndDoExpr)
);
return node.object && startsWithNoLookaheadToken(node.object, predicate);
case "SequenceExpression":
return startsWithNoLookaheadToken(
node.expressions[0],
forbidFunctionClassAndDoExpr
);
return startsWithNoLookaheadToken(node.expressions[0], predicate);
case "TSSatisfiesExpression":
case "TSAsExpression":
case "TSNonNullExpression":
return startsWithNoLookaheadToken(
node.expression,
forbidFunctionClassAndDoExpr
);
return startsWithNoLookaheadToken(node.expression, predicate);
default:
return false;
return predicate(node);
}
}

Expand Down Expand Up @@ -1092,13 +1077,6 @@ function getPrecedence(operator) {
return PRECEDENCE.get(operator);
}

function getLeftMost(node) {
while (node.left) {
node = node.left;
}
return node;
}

function isBitwiseOperator(operator) {
return (
Boolean(bitshiftOperators[operator]) ||
Expand Down
4 changes: 4 additions & 0 deletions tests/config/format-test.js
Expand Up @@ -41,6 +41,10 @@ const unstableTests = new Map(
"js/comments/html-like/comment.js",
"js/for/continue-and-break-comment-without-blocks.js",
"typescript/satisfies-operators/comments-unstable.ts",
[
"js/identifier/parentheses/let-in-assignment.js",
(options) => options.semi === false,
],
].map((fixture) => {
const [file, isUnstable = () => true] = Array.isArray(fixture)
? fixture
Expand Down