Skip to content

[Master] Emit parenthesis around propert/element access expression of casted object literal expression #15006

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

Merged
merged 4 commits into from
Apr 5, 2017
Merged
Show file tree
Hide file tree
Changes from 3 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
3 changes: 2 additions & 1 deletion src/compiler/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3492,7 +3492,8 @@ namespace ts {

export function parenthesizeConciseBody(body: ConciseBody): ConciseBody {
const emittedBody = skipPartiallyEmittedExpressions(body);
if (emittedBody.kind === SyntaxKind.ObjectLiteralExpression) {
const leftMostExpression = isExpression(emittedBody) ? getLeftmostExpression(emittedBody) : undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could just be:

if (!isBlock(body) && getLeftmostExpression(body).kind === SyntaxKind.ObjectLiteralExpresion)

I suggest using !isBlock(body) here as there are fewer comparisons necessary than a full isExpression test.

if (leftMostExpression && leftMostExpression.kind === SyntaxKind.ObjectLiteralExpression) {
return setTextRange(createParen(<Expression>body), body);
}

Expand Down
6 changes: 3 additions & 3 deletions src/compiler/transformers/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2324,13 +2324,13 @@ namespace ts {
// code if the casted expression has a lower precedence than the rest of the
// expression.
//
// To preserve comments, we return a "PartiallyEmittedExpression" here which will
// preserve the position information of the original expression.
//
// Due to the auto-parenthesization rules used by the visitor and factory functions
// we can safely elide the parentheses here, as a new synthetic
// ParenthesizedExpression will be inserted if we remove parentheses too
// aggressively.
//
// To preserve comments, we return a "PartiallyEmittedExpression" here which will
// preserve the position information of the original expression.
return createPartiallyEmittedExpression(expression, node);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//// [emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES5.ts]
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x];
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x;

//// [emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES5.js]
(function (x) { return ({ "1": "one", "2": "two" }[x]); });
(function (x) { return ({ "1": "one", "2": "two" }.x); });
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
=== tests/cases/compiler/emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES5.ts ===
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x];
>x : Symbol(x, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES5.ts, 0, 1))
>key : Symbol(key, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES5.ts, 0, 41))
>x : Symbol(x, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES5.ts, 0, 1))

(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x;
>x : Symbol(x, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES5.ts, 1, 1))
>key : Symbol(key, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES5.ts, 1, 41))

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
=== tests/cases/compiler/emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES5.ts ===
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x];
>(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x] : (x: any) => string
>x : any
>({ "1": "one", "2": "two" } as { [key: string]: string })[x] : string
>({ "1": "one", "2": "two" } as { [key: string]: string }) : { [key: string]: string; }
>{ "1": "one", "2": "two" } as { [key: string]: string } : { [key: string]: string; }
>{ "1": "one", "2": "two" } : { "1": string; "2": string; }
>"one" : "one"
>"two" : "two"
>key : string
>x : any

(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x;
>(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x : (x: any) => string
>x : any
>({ "1": "one", "2": "two" } as { [key: string]: string }).x : string
>({ "1": "one", "2": "two" } as { [key: string]: string }) : { [key: string]: string; }
>{ "1": "one", "2": "two" } as { [key: string]: string } : { [key: string]: string; }
>{ "1": "one", "2": "two" } : { "1": string; "2": string; }
>"one" : "one"
>"two" : "two"
>key : string
>x : string

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//// [emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES6.ts]
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x];
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x;

//// [emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES6.js]
(x) => ({ "1": "one", "2": "two" }[x]);
(x) => ({ "1": "one", "2": "two" }.x);
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
=== tests/cases/compiler/emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES6.ts ===
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x];
>x : Symbol(x, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES6.ts, 0, 1))
>key : Symbol(key, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES6.ts, 0, 41))
>x : Symbol(x, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES6.ts, 0, 1))

(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x;
>x : Symbol(x, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES6.ts, 1, 1))
>key : Symbol(key, Decl(emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES6.ts, 1, 41))

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
=== tests/cases/compiler/emitAccessExpressionOfCastedObjectLiteralExpressionInArrowFunctionES6.ts ===
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x];
>(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x] : (x: any) => string
>x : any
>({ "1": "one", "2": "two" } as { [key: string]: string })[x] : string
>({ "1": "one", "2": "two" } as { [key: string]: string }) : { [key: string]: string; }
>{ "1": "one", "2": "two" } as { [key: string]: string } : { [key: string]: string; }
>{ "1": "one", "2": "two" } : { "1": string; "2": string; }
>"one" : "one"
>"two" : "two"
>key : string
>x : any

(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x;
>(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x : (x: any) => string
>x : any
>({ "1": "one", "2": "two" } as { [key: string]: string }).x : string
>({ "1": "one", "2": "two" } as { [key: string]: string }) : { [key: string]: string; }
>{ "1": "one", "2": "two" } as { [key: string]: string } : { [key: string]: string; }
>{ "1": "one", "2": "two" } : { "1": string; "2": string; }
>"one" : "one"
>"two" : "two"
>key : string
>x : string

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// @target: es5

(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x];
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// @target: es6

(x) => ({ "1": "one", "2": "two" } as { [key: string]: string })[x];
(x) => ({ "1": "one", "2": "two" } as { [key: string]: string }).x;