Do not omit parentheses for numeric literal with ".property" if it can not be followed by dot operator directly. #4641
Conversation
|
Hi @vilic, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
|
|
||
| if (languageVersion < ScriptTarget.ES6 && isBinaryOrOctalIntegerLiteral(node, text)) { | ||
| return node.text; | ||
| } else { |
|
Personally, I'd rather we just always emit the parens. Thoughts @mhegazy? |
|
We try to emit clean code that you would write by hand. and i do not think you will always parenthesize your numeric literals. |
| @@ -2352,7 +2362,8 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi | |||
| operand.kind !== SyntaxKind.PostfixUnaryExpression && | |||
| operand.kind !== SyntaxKind.NewExpression && | |||
| !(operand.kind === SyntaxKind.CallExpression && node.parent.kind === SyntaxKind.NewExpression) && | |||
| !(operand.kind === SyntaxKind.FunctionExpression && node.parent.kind === SyntaxKind.CallExpression)) { | |||
| !(operand.kind === SyntaxKind.FunctionExpression && node.parent.kind === SyntaxKind.CallExpression) && | |||
| !(operand.kind === SyntaxKind.NumericLiteral && node.parent.kind === SyntaxKind.PropertyAccessExpression && !/^0[box]|[e.]/i.test(getEmittingNumericLiteralText(<LiteralExpression>operand)))) { | |||
There was a problem hiding this comment.
This doesn't cover when your numeric literal consists of escape sequences.
There was a problem hiding this comment.
I didn't notice there could be escape sequences in a numeric literal. http://www.ecma-international.org/ecma-262/6.0/#sec-literals-numeric-literals
There was a problem hiding this comment.
@DanielRosenwasser So what do you mean by "numeric literal consists of escape sequences"?
There was a problem hiding this comment.
You can literally write var x = \u0032\u002e\u0030 instead of var x = 2.0 and both are valid and ECMAScript code.
There was a problem hiding this comment.
Actually, I stand corrected - the escapes can't be used in numeric literals. From 10.1 - Source Text:
In string literals, regular expression literals, template literals and identifiers, any Unicode code point may also be expressed using Unicode escape sequences that explicitly express a code point’s numeric value. Within a comment, such an escape sequence is effectively ignored as part of the comment.
But as you mentioned, it doesn't cover numeric literals.
|
@mhegazy I'd rather we just didn't special case this. |
|
so @DanielRosenwasser what is the proposal, just check if the expression is numeric literal, and the parent is property access expression? this sound fine by me. @vilic, any concerns? |
|
@mhegazy that sounds fine, or maybe even just don't strip parens from a numeric literal in a type assertion because it's likely going to have a property access for whatever reason anyway. Either is fine with me. |
|
@mhegazy @DanielRosenwasser I am cool with both. But I prefer keeping property access as a condition though other situations might be rare. |
|
👍 |
…numeric-literal Do not omit parentheses for numeric literal with ".property" if it can not be followed by dot operator directly.
|
Thanks @vilic! |
vilicvane
deleted the
cast-expression-parentheses-with-numeric-literal
branch
Fix issue #4603.
Added a function
getEmittingNumericLiteralTextas a helper of determine whether we want to keep the parentheses, as the emitted numeric literal maybe different under different target.E.g.,
0o123emits83(which needs the parentheses) under ES3/5, but emits0o123under ES6.