From 2950c05a1699ffbbc2b43050cba1c579a891724c Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Sat, 15 Jul 2023 22:52:56 +0300 Subject: [PATCH 1/7] fix(54992): parenthesize negative constant enums --- src/compiler/checker.ts | 12 ++ src/compiler/emitter.ts | 1 + src/compiler/transformers/ts.ts | 13 +- src/compiler/types.ts | 1 + .../reference/constEnumPropertyAccess3.js | 33 ++++++ .../constEnumPropertyAccess3.symbols | 80 +++++++++++++ .../reference/constEnumPropertyAccess3.types | 111 ++++++++++++++++++ .../reference/constEnumToStringNoComments.js | 12 +- .../constEnumToStringWithComments.js | 12 +- .../constEnums/constEnumPropertyAccess3.ts | 18 +++ 10 files changed, 276 insertions(+), 17 deletions(-) create mode 100644 tests/baselines/reference/constEnumPropertyAccess3.js create mode 100644 tests/baselines/reference/constEnumPropertyAccess3.symbols create mode 100644 tests/baselines/reference/constEnumPropertyAccess3.types create mode 100644 tests/cases/conformance/constEnums/constEnumPropertyAccess3.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 73a15fd578777..0d110bd6e616e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -46483,6 +46483,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { return undefined; } + function shouldParenthesizeConstantValue(node: AccessExpression) { + if (isAccessExpression(node.parent) && node.parent.expression === node) { + const value = getConstantValue(node); + return typeof value === "number" && value < 0; + } + return false; + } + function isFunctionType(type: Type): boolean { return !!(type.flags & TypeFlags.Object) && getSignaturesOfType(type, SignatureKind.Call).length > 0; } @@ -46805,6 +46813,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const node = getParseTreeNode(nodeIn, canHaveConstantValue); return node ? getConstantValue(node) : undefined; }, + shouldParenthesizeConstantValue: nodeIn => { + const node = getParseTreeNode(nodeIn, isAccessExpression); + return !!node && shouldParenthesizeConstantValue(node); + }, collectLinkedAliases, getReferencedValueDeclaration, getReferencedValueDeclarations, diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index a58223868f8b0..7199968109a54 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -1158,6 +1158,7 @@ export const notImplementedResolver: EmitResolver = { isEntityNameVisible: notImplemented, // Returns the constant value this property access resolves to: notImplemented, or 'undefined' for a non-constant getConstantValue: notImplemented, + shouldParenthesizeConstantValue: notImplemented, getReferencedValueDeclaration: notImplemented, getReferencedValueDeclarations: notImplemented, getTypeReferenceSerializationKind: notImplemented, diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 8b97637c07722..444334cce042c 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -110,6 +110,7 @@ import { isNamedExportBindings, isNamedImportBindings, isNamespaceExport, + isNumericLiteral, isObjectLiteralElementLike, isParameterPropertyDeclaration, isPrivateIdentifier, @@ -119,6 +120,7 @@ import { isSimpleInlineableExpression, isSourceFile, isStatement, + isStringLiteral, isTemplateLiteral, isTryStatement, JsxOpeningElement, @@ -2669,18 +2671,19 @@ export function transformTypeScript(context: TransformationContext) { function substituteConstantValue(node: PropertyAccessExpression | ElementAccessExpression): LeftHandSideExpression { const constantValue = tryGetConstEnumValue(node); if (constantValue !== undefined) { - // track the constant value on the node for the printer in needsDotDotForPropertyAccess - setConstantValue(node, constantValue); + const substitute = typeof constantValue === "string" ? factory.createStringLiteral(constantValue) : + resolver.shouldParenthesizeConstantValue(node) ? factory.createParenthesizedExpression(factory.createNumericLiteral(constantValue)) : factory.createNumericLiteral(constantValue); - const substitute = typeof constantValue === "string" ? factory.createStringLiteral(constantValue) : factory.createNumericLiteral(constantValue); + // track the constant value on the node for the printer in needsDotDotForPropertyAccess + if (isStringLiteral(substitute) || isNumericLiteral(substitute)) { + setConstantValue(node, constantValue); + } if (!compilerOptions.removeComments) { const originalNode = getOriginalNode(node, isAccessExpression); - addSyntheticTrailingComment(substitute, SyntaxKind.MultiLineCommentTrivia, ` ${safeMultiLineComment(getTextOfNode(originalNode))} `); } return substitute; } - return node; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 63ca3980ca64a..dc94361199d9a 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5700,6 +5700,7 @@ export interface EmitResolver { isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node): SymbolVisibilityResult; // Returns the constant value this property access resolves to, or 'undefined' for a non-constant getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): string | number | undefined; + shouldParenthesizeConstantValue(node: PropertyAccessExpression | ElementAccessExpression): boolean; getReferencedValueDeclaration(reference: Identifier): Declaration | undefined; getReferencedValueDeclarations(reference: Identifier): Declaration[] | undefined; getTypeReferenceSerializationKind(typeName: EntityName, location?: Node): TypeReferenceSerializationKind; diff --git a/tests/baselines/reference/constEnumPropertyAccess3.js b/tests/baselines/reference/constEnumPropertyAccess3.js new file mode 100644 index 0000000000000..ab6237c380cd3 --- /dev/null +++ b/tests/baselines/reference/constEnumPropertyAccess3.js @@ -0,0 +1,33 @@ +//// [tests/cases/conformance/constEnums/constEnumPropertyAccess3.ts] //// + +//// [constEnumPropertyAccess3.ts] +const enum E { + A = ~1, + B = -1, + C = ~(1 + 1), + D = -(1 + 2), + E = 1 - 10, +} + +E.A.toString(); +E.B.toString(); +E.C.toString(); +E.D.toString(); + +E["A"].toString(); +E["B"].toString(); +E["C"].toString(); +E["D"].toString(); +E["E"].toString(); + + +//// [constEnumPropertyAccess3.js] +(-2) /* E.A */.toString(); +(-1) /* E.B */.toString(); +(-3) /* E.C */.toString(); +(-3) /* E.D */.toString(); +(-2) /* E["A"] */.toString(); +(-1) /* E["B"] */.toString(); +(-3) /* E["C"] */.toString(); +(-3) /* E["D"] */.toString(); +(-9) /* E["E"] */.toString(); diff --git a/tests/baselines/reference/constEnumPropertyAccess3.symbols b/tests/baselines/reference/constEnumPropertyAccess3.symbols new file mode 100644 index 0000000000000..27fac59420724 --- /dev/null +++ b/tests/baselines/reference/constEnumPropertyAccess3.symbols @@ -0,0 +1,80 @@ +//// [tests/cases/conformance/constEnums/constEnumPropertyAccess3.ts] //// + +=== constEnumPropertyAccess3.ts === +const enum E { +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) + + A = ~1, +>A : Symbol(E.A, Decl(constEnumPropertyAccess3.ts, 0, 14)) + + B = -1, +>B : Symbol(E.B, Decl(constEnumPropertyAccess3.ts, 1, 11)) + + C = ~(1 + 1), +>C : Symbol(E.C, Decl(constEnumPropertyAccess3.ts, 2, 11)) + + D = -(1 + 2), +>D : Symbol(E.D, Decl(constEnumPropertyAccess3.ts, 3, 17)) + + E = 1 - 10, +>E : Symbol(E.E, Decl(constEnumPropertyAccess3.ts, 4, 17)) +} + +E.A.toString(); +>E.A.toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) +>E.A : Symbol(E.A, Decl(constEnumPropertyAccess3.ts, 0, 14)) +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) +>A : Symbol(E.A, Decl(constEnumPropertyAccess3.ts, 0, 14)) +>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) + +E.B.toString(); +>E.B.toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) +>E.B : Symbol(E.B, Decl(constEnumPropertyAccess3.ts, 1, 11)) +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) +>B : Symbol(E.B, Decl(constEnumPropertyAccess3.ts, 1, 11)) +>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) + +E.C.toString(); +>E.C.toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) +>E.C : Symbol(E.C, Decl(constEnumPropertyAccess3.ts, 2, 11)) +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) +>C : Symbol(E.C, Decl(constEnumPropertyAccess3.ts, 2, 11)) +>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) + +E.D.toString(); +>E.D.toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) +>E.D : Symbol(E.D, Decl(constEnumPropertyAccess3.ts, 3, 17)) +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) +>D : Symbol(E.D, Decl(constEnumPropertyAccess3.ts, 3, 17)) +>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) + +E["A"].toString(); +>E["A"].toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) +>"A" : Symbol(E.A, Decl(constEnumPropertyAccess3.ts, 0, 14)) +>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) + +E["B"].toString(); +>E["B"].toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) +>"B" : Symbol(E.B, Decl(constEnumPropertyAccess3.ts, 1, 11)) +>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) + +E["C"].toString(); +>E["C"].toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) +>"C" : Symbol(E.C, Decl(constEnumPropertyAccess3.ts, 2, 11)) +>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) + +E["D"].toString(); +>E["D"].toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) +>"D" : Symbol(E.D, Decl(constEnumPropertyAccess3.ts, 3, 17)) +>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) + +E["E"].toString(); +>E["E"].toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) +>E : Symbol(E, Decl(constEnumPropertyAccess3.ts, 0, 0)) +>"E" : Symbol(E.E, Decl(constEnumPropertyAccess3.ts, 4, 17)) +>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --)) + diff --git a/tests/baselines/reference/constEnumPropertyAccess3.types b/tests/baselines/reference/constEnumPropertyAccess3.types new file mode 100644 index 0000000000000..85d0460f9b600 --- /dev/null +++ b/tests/baselines/reference/constEnumPropertyAccess3.types @@ -0,0 +1,111 @@ +//// [tests/cases/conformance/constEnums/constEnumPropertyAccess3.ts] //// + +=== constEnumPropertyAccess3.ts === +const enum E { +>E : E + + A = ~1, +>A : E.A +>~1 : number +>1 : 1 + + B = -1, +>B : E.B +>-1 : -1 +>1 : 1 + + C = ~(1 + 1), +>C : E.C +>~(1 + 1) : number +>(1 + 1) : number +>1 + 1 : number +>1 : 1 +>1 : 1 + + D = -(1 + 2), +>D : E.C +>-(1 + 2) : number +>(1 + 2) : number +>1 + 2 : number +>1 : 1 +>2 : 2 + + E = 1 - 10, +>E : E.E +>1 - 10 : number +>1 : 1 +>10 : 10 +} + +E.A.toString(); +>E.A.toString() : string +>E.A.toString : (radix?: number) => string +>E.A : E.A +>E : typeof E +>A : E.A +>toString : (radix?: number) => string + +E.B.toString(); +>E.B.toString() : string +>E.B.toString : (radix?: number) => string +>E.B : E.B +>E : typeof E +>B : E.B +>toString : (radix?: number) => string + +E.C.toString(); +>E.C.toString() : string +>E.C.toString : (radix?: number) => string +>E.C : E.C +>E : typeof E +>C : E.C +>toString : (radix?: number) => string + +E.D.toString(); +>E.D.toString() : string +>E.D.toString : (radix?: number) => string +>E.D : E.C +>E : typeof E +>D : E.C +>toString : (radix?: number) => string + +E["A"].toString(); +>E["A"].toString() : string +>E["A"].toString : (radix?: number) => string +>E["A"] : E.A +>E : typeof E +>"A" : "A" +>toString : (radix?: number) => string + +E["B"].toString(); +>E["B"].toString() : string +>E["B"].toString : (radix?: number) => string +>E["B"] : E.B +>E : typeof E +>"B" : "B" +>toString : (radix?: number) => string + +E["C"].toString(); +>E["C"].toString() : string +>E["C"].toString : (radix?: number) => string +>E["C"] : E.C +>E : typeof E +>"C" : "C" +>toString : (radix?: number) => string + +E["D"].toString(); +>E["D"].toString() : string +>E["D"].toString : (radix?: number) => string +>E["D"] : E.C +>E : typeof E +>"D" : "D" +>toString : (radix?: number) => string + +E["E"].toString(); +>E["E"].toString() : string +>E["E"].toString : (radix?: number) => string +>E["E"] : E.E +>E : typeof E +>"E" : "E" +>toString : (radix?: number) => string + diff --git a/tests/baselines/reference/constEnumToStringNoComments.js b/tests/baselines/reference/constEnumToStringNoComments.js index cffb3b1cbcb61..4ab485ee16635 100644 --- a/tests/baselines/reference/constEnumToStringNoComments.js +++ b/tests/baselines/reference/constEnumToStringNoComments.js @@ -31,9 +31,9 @@ var y0 = 0.5.toString(); var y1 = 0.5.toString(); var z0 = 2..toString(); var z1 = 2..toString(); -var a0 = -1..toString(); -var a1 = -1..toString(); -var b0 = -1.5.toString(); -var b1 = -1.5.toString(); -var c0 = -1..toString(); -var c1 = -1..toString(); +var a0 = (-1).toString(); +var a1 = (-1).toString(); +var b0 = (-1.5).toString(); +var b1 = (-1.5).toString(); +var c0 = (-1).toString(); +var c1 = (-1).toString(); diff --git a/tests/baselines/reference/constEnumToStringWithComments.js b/tests/baselines/reference/constEnumToStringWithComments.js index 23f656658025d..ad5b3305cea7a 100644 --- a/tests/baselines/reference/constEnumToStringWithComments.js +++ b/tests/baselines/reference/constEnumToStringWithComments.js @@ -31,9 +31,9 @@ var y0 = 0.5 /* Foo.Y */.toString(); var y1 = 0.5 /* Foo["Y"] */.toString(); var z0 = 2 /* Foo.Z */.toString(); var z1 = 2 /* Foo["Z"] */.toString(); -var a0 = -1 /* Foo.A */.toString(); -var a1 = -1 /* Foo["A"] */.toString(); -var b0 = -1.5 /* Foo.B */.toString(); -var b1 = -1.5 /* Foo["B"] */.toString(); -var c0 = -1 /* Foo.C */.toString(); -var c1 = -1 /* Foo["C"] */.toString(); +var a0 = (-1) /* Foo.A */.toString(); +var a1 = (-1) /* Foo["A"] */.toString(); +var b0 = (-1.5) /* Foo.B */.toString(); +var b1 = (-1.5) /* Foo["B"] */.toString(); +var c0 = (-1) /* Foo.C */.toString(); +var c1 = (-1) /* Foo["C"] */.toString(); diff --git a/tests/cases/conformance/constEnums/constEnumPropertyAccess3.ts b/tests/cases/conformance/constEnums/constEnumPropertyAccess3.ts new file mode 100644 index 0000000000000..11eaaa10cbaed --- /dev/null +++ b/tests/cases/conformance/constEnums/constEnumPropertyAccess3.ts @@ -0,0 +1,18 @@ +const enum E { + A = ~1, + B = -1, + C = ~(1 + 1), + D = -(1 + 2), + E = 1 - 10, +} + +E.A.toString(); +E.B.toString(); +E.C.toString(); +E.D.toString(); + +E["A"].toString(); +E["B"].toString(); +E["C"].toString(); +E["D"].toString(); +E["E"].toString(); From 2ef9980dc8301410523fdd7683c8a28c7835d3dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Tue, 18 Jul 2023 10:23:52 +0200 Subject: [PATCH 2/7] Move the fix to `parenthesizerRules` --- src/compiler/checker.ts | 12 ---------- src/compiler/emitter.ts | 18 ++------------- src/compiler/factory/parenthesizerRules.ts | 23 ++++++++++++++++--- src/compiler/transformers/ts.ts | 13 ++++------- src/compiler/types.ts | 1 - src/compiler/utilities.ts | 8 +++++++ .../reference/castExpressionParentheses.js | 6 ++--- .../reference/constEnumPropertyAccess3.js | 18 +++++++-------- .../reference/constEnumToStringNoComments.js | 8 +++---- .../constEnumToStringWithComments.js | 20 ++++++++-------- .../reference/destructuringControlFlow.js | 4 ++-- .../destructuringInVariableDeclarations1.js | 2 +- .../destructuringInVariableDeclarations3.js | 2 +- .../destructuringInVariableDeclarations5.js | 2 +- .../destructuringInVariableDeclarations7.js | 2 +- .../destructuringInVariableDeclarations8.js | 2 +- .../destructuringTypeAssertionsES5_5.js | 2 +- .../destructuringWithNumberLiteral.js | 2 +- .../propertyAccessNumericLiterals.js | 4 ++-- 19 files changed, 72 insertions(+), 77 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 0d110bd6e616e..73a15fd578777 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -46483,14 +46483,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { return undefined; } - function shouldParenthesizeConstantValue(node: AccessExpression) { - if (isAccessExpression(node.parent) && node.parent.expression === node) { - const value = getConstantValue(node); - return typeof value === "number" && value < 0; - } - return false; - } - function isFunctionType(type: Type): boolean { return !!(type.flags & TypeFlags.Object) && getSignaturesOfType(type, SignatureKind.Call).length > 0; } @@ -46813,10 +46805,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const node = getParseTreeNode(nodeIn, canHaveConstantValue); return node ? getConstantValue(node) : undefined; }, - shouldParenthesizeConstantValue: nodeIn => { - const node = getParseTreeNode(nodeIn, isAccessExpression); - return !!node && shouldParenthesizeConstantValue(node); - }, collectLinkedAliases, getReferencedValueDeclaration, getReferencedValueDeclarations, diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 7199968109a54..0e0c198ae1a84 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -137,7 +137,6 @@ import { getBaseFileName, GetCanonicalFileName, getCommentRange, - getConstantValue, getContainingNodeArray, getDeclarationEmitExtensionForPath, getDeclarationEmitOutputFilePath, @@ -203,7 +202,6 @@ import { InterfaceDeclaration, InternalEmitFlags, IntersectionTypeNode, - isAccessExpression, isArray, isArrowFunction, isBinaryExpression, @@ -239,6 +237,7 @@ import { isModuleDeclaration, isNodeDescendantOf, isNumericLiteral, + isNumericLiteralTextSafeForPropertyAccess, isParenthesizedExpression, isPartiallyEmittedExpression, isPinnedComment, @@ -407,7 +406,6 @@ import { SpreadElement, stableSort, Statement, - stringContains, StringLiteral, supportedJSExtensionsFlat, SwitchStatement, @@ -424,7 +422,6 @@ import { TemplateSpan, TextRange, ThrowStatement, - TokenFlags, tokenToString, tracing, TransformationResult, @@ -1158,7 +1155,6 @@ export const notImplementedResolver: EmitResolver = { isEntityNameVisible: notImplemented, // Returns the constant value this property access resolves to: notImplemented, or 'undefined' for a non-constant getConstantValue: notImplemented, - shouldParenthesizeConstantValue: notImplemented, getReferencedValueDeclaration: notImplemented, getReferencedValueDeclarations: notImplemented, getTypeReferenceSerializationKind: notImplemented, @@ -3056,17 +3052,7 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri const text = getLiteralTextOfNode(expression as LiteralExpression, /*neverAsciiEscape*/ true, /*jsxAttributeEscape*/ false); // If the number will be printed verbatim and it doesn't already contain a dot or an exponent indicator, add one // if the expression doesn't have any comments that will be emitted. - return !(expression.numericLiteralFlags & TokenFlags.WithSpecifier) - && !stringContains(text, tokenToString(SyntaxKind.DotToken)!) - && !stringContains(text, String.fromCharCode(CharacterCodes.E)) - && !stringContains(text, String.fromCharCode(CharacterCodes.e)); - } - else if (isAccessExpression(expression)) { - // check if constant enum value is integer - const constantValue = getConstantValue(expression); - // isFinite handles cases when constantValue is undefined - return typeof constantValue === "number" && isFinite(constantValue) - && Math.floor(constantValue) === constantValue; + return !isNumericLiteralTextSafeForPropertyAccess(expression, text); } } diff --git a/src/compiler/factory/parenthesizerRules.ts b/src/compiler/factory/parenthesizerRules.ts index 5a1386d567b3f..cc139c82aa1fe 100644 --- a/src/compiler/factory/parenthesizerRules.ts +++ b/src/compiler/factory/parenthesizerRules.ts @@ -3,6 +3,7 @@ import { BinaryExpression, BinaryOperator, cast, + CharacterCodes, compareValues, Comparison, ConciseBody, @@ -28,6 +29,7 @@ import { isLiteralKind, isNamedTupleMember, isNodeArray, + isNumericLiteralTextSafeForPropertyAccess, isOptionalChain, isTypeOperatorNode, isUnaryExpression, @@ -38,6 +40,7 @@ import { NewExpression, NodeArray, NodeFactory, + NumericLiteral, OperatorPrecedence, OuterExpressionKinds, ParenthesizerRules, @@ -380,9 +383,23 @@ export function createParenthesizerRules(factory: NodeFactory): ParenthesizerRul // new C.x -> not the same as (new C).x // const emittedExpression = skipPartiallyEmittedExpressions(expression); - if (isLeftHandSideExpression(emittedExpression) - && (emittedExpression.kind !== SyntaxKind.NewExpression || (emittedExpression as NewExpression).arguments) - && (optionalChain || !isOptionalChain(emittedExpression))) { + if (isLeftHandSideExpression(emittedExpression) && (optionalChain || !isOptionalChain(emittedExpression))) { + switch (emittedExpression.kind) { + case SyntaxKind.NewExpression: + if ((emittedExpression as NewExpression).arguments) { + break; + } + // TODO(rbuckton): Verifiy whether `setTextRange` is needed. + return setTextRange(factory.createParenthesizedExpression(expression), expression); + case SyntaxKind.NumericLiteral: { + const literal = emittedExpression as NumericLiteral; + if (literal.text.charCodeAt(0) !== CharacterCodes.minus && isNumericLiteralTextSafeForPropertyAccess(literal)) { + break; + } + // TODO(rbuckton): Verifiy whether `setTextRange` is needed. + return setTextRange(factory.createParenthesizedExpression(expression), expression); + } + } // TODO(rbuckton): Verify whether this assertion holds. return expression as LeftHandSideExpression; } diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 444334cce042c..8b97637c07722 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -110,7 +110,6 @@ import { isNamedExportBindings, isNamedImportBindings, isNamespaceExport, - isNumericLiteral, isObjectLiteralElementLike, isParameterPropertyDeclaration, isPrivateIdentifier, @@ -120,7 +119,6 @@ import { isSimpleInlineableExpression, isSourceFile, isStatement, - isStringLiteral, isTemplateLiteral, isTryStatement, JsxOpeningElement, @@ -2671,19 +2669,18 @@ export function transformTypeScript(context: TransformationContext) { function substituteConstantValue(node: PropertyAccessExpression | ElementAccessExpression): LeftHandSideExpression { const constantValue = tryGetConstEnumValue(node); if (constantValue !== undefined) { - const substitute = typeof constantValue === "string" ? factory.createStringLiteral(constantValue) : - resolver.shouldParenthesizeConstantValue(node) ? factory.createParenthesizedExpression(factory.createNumericLiteral(constantValue)) : factory.createNumericLiteral(constantValue); - // track the constant value on the node for the printer in needsDotDotForPropertyAccess - if (isStringLiteral(substitute) || isNumericLiteral(substitute)) { - setConstantValue(node, constantValue); - } + setConstantValue(node, constantValue); + + const substitute = typeof constantValue === "string" ? factory.createStringLiteral(constantValue) : factory.createNumericLiteral(constantValue); if (!compilerOptions.removeComments) { const originalNode = getOriginalNode(node, isAccessExpression); + addSyntheticTrailingComment(substitute, SyntaxKind.MultiLineCommentTrivia, ` ${safeMultiLineComment(getTextOfNode(originalNode))} `); } return substitute; } + return node; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index dc94361199d9a..63ca3980ca64a 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5700,7 +5700,6 @@ export interface EmitResolver { isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node): SymbolVisibilityResult; // Returns the constant value this property access resolves to, or 'undefined' for a non-constant getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): string | number | undefined; - shouldParenthesizeConstantValue(node: PropertyAccessExpression | ElementAccessExpression): boolean; getReferencedValueDeclaration(reference: Identifier): Declaration | undefined; getReferencedValueDeclarations(reference: Identifier): Declaration[] | undefined; getTypeReferenceSerializationKind(typeName: EntityName, location?: Node): TypeReferenceSerializationKind; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index b850483038351..f495c5b5be31b 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10311,3 +10311,11 @@ export function getTextOfJsxNamespacedName(node: JsxNamespacedName) { export function intrinsicTagNameToString(node: Identifier | JsxNamespacedName) { return isIdentifier(node) ? idText(node) : getTextOfJsxNamespacedName(node); } + +/** @internal */ +export function isNumericLiteralTextSafeForPropertyAccess(numericLiteral: NumericLiteral, text = numericLiteral.text) { + return numericLiteral.numericLiteralFlags & TokenFlags.WithSpecifier + || stringContains(text, tokenToString(SyntaxKind.DotToken)!) + || stringContains(text, String.fromCharCode(CharacterCodes.E)) + || stringContains(text, String.fromCharCode(CharacterCodes.e)); +} diff --git a/tests/baselines/reference/castExpressionParentheses.js b/tests/baselines/reference/castExpressionParentheses.js index 8d2f0a15ddf98..cc71b70b89cba 100644 --- a/tests/baselines/reference/castExpressionParentheses.js +++ b/tests/baselines/reference/castExpressionParentheses.js @@ -76,9 +76,9 @@ a; a[0]; a.b["0"]; a().x; -1..foo; -1..foo; -1.0.foo; +(1).foo; +(1.).foo; +(1.0).foo; 12e+34.foo; 0xff.foo; // should keep the parentheses in emit diff --git a/tests/baselines/reference/constEnumPropertyAccess3.js b/tests/baselines/reference/constEnumPropertyAccess3.js index ab6237c380cd3..b7f184d281e5f 100644 --- a/tests/baselines/reference/constEnumPropertyAccess3.js +++ b/tests/baselines/reference/constEnumPropertyAccess3.js @@ -22,12 +22,12 @@ E["E"].toString(); //// [constEnumPropertyAccess3.js] -(-2) /* E.A */.toString(); -(-1) /* E.B */.toString(); -(-3) /* E.C */.toString(); -(-3) /* E.D */.toString(); -(-2) /* E["A"] */.toString(); -(-1) /* E["B"] */.toString(); -(-3) /* E["C"] */.toString(); -(-3) /* E["D"] */.toString(); -(-9) /* E["E"] */.toString(); +(-2 /* E.A */).toString(); +(-1 /* E.B */).toString(); +(-3 /* E.C */).toString(); +(-3 /* E.D */).toString(); +(-2 /* E["A"] */).toString(); +(-1 /* E["B"] */).toString(); +(-3 /* E["C"] */).toString(); +(-3 /* E["D"] */).toString(); +(-9 /* E["E"] */).toString(); diff --git a/tests/baselines/reference/constEnumToStringNoComments.js b/tests/baselines/reference/constEnumToStringNoComments.js index 4ab485ee16635..692fddca792e6 100644 --- a/tests/baselines/reference/constEnumToStringNoComments.js +++ b/tests/baselines/reference/constEnumToStringNoComments.js @@ -25,12 +25,12 @@ let c1 = Foo["C"].toString(); //// [constEnumToStringNoComments.js] -var x0 = 100..toString(); -var x1 = 100..toString(); +var x0 = (100).toString(); +var x1 = (100).toString(); var y0 = 0.5.toString(); var y1 = 0.5.toString(); -var z0 = 2..toString(); -var z1 = 2..toString(); +var z0 = (2).toString(); +var z1 = (2).toString(); var a0 = (-1).toString(); var a1 = (-1).toString(); var b0 = (-1.5).toString(); diff --git a/tests/baselines/reference/constEnumToStringWithComments.js b/tests/baselines/reference/constEnumToStringWithComments.js index ad5b3305cea7a..f3509f0b69b90 100644 --- a/tests/baselines/reference/constEnumToStringWithComments.js +++ b/tests/baselines/reference/constEnumToStringWithComments.js @@ -25,15 +25,15 @@ let c1 = Foo["C"].toString(); //// [constEnumToStringWithComments.js] -var x0 = 100 /* Foo.X */.toString(); -var x1 = 100 /* Foo["X"] */.toString(); +var x0 = (100 /* Foo.X */).toString(); +var x1 = (100 /* Foo["X"] */).toString(); var y0 = 0.5 /* Foo.Y */.toString(); var y1 = 0.5 /* Foo["Y"] */.toString(); -var z0 = 2 /* Foo.Z */.toString(); -var z1 = 2 /* Foo["Z"] */.toString(); -var a0 = (-1) /* Foo.A */.toString(); -var a1 = (-1) /* Foo["A"] */.toString(); -var b0 = (-1.5) /* Foo.B */.toString(); -var b1 = (-1.5) /* Foo["B"] */.toString(); -var c0 = (-1) /* Foo.C */.toString(); -var c1 = (-1) /* Foo["C"] */.toString(); +var z0 = (2 /* Foo.Z */).toString(); +var z1 = (2 /* Foo["Z"] */).toString(); +var a0 = (-1 /* Foo.A */).toString(); +var a1 = (-1 /* Foo["A"] */).toString(); +var b0 = (-1.5 /* Foo.B */).toString(); +var b1 = (-1.5 /* Foo["B"] */).toString(); +var c0 = (-1 /* Foo.C */).toString(); +var c1 = (-1 /* Foo["C"] */).toString(); diff --git a/tests/baselines/reference/destructuringControlFlow.js b/tests/baselines/reference/destructuringControlFlow.js index 1f444d4d56df1..f17b8214773c3 100644 --- a/tests/baselines/reference/destructuringControlFlow.js +++ b/tests/baselines/reference/destructuringControlFlow.js @@ -73,8 +73,8 @@ function f3(obj) { function f4() { var _a, _b; var x; - (x = 0..x); // Error - (x = 0["x"]); // Error + (x = (0).x); // Error + (x = (0)["x"]); // Error (_a = 0, _b = "x" + "", x = _a[_b]); // Errpr } var _a = ["foo"], key = _a[0], value = _a[1]; diff --git a/tests/baselines/reference/destructuringInVariableDeclarations1.js b/tests/baselines/reference/destructuringInVariableDeclarations1.js index 365697ec80f9e..c9b8457aa0676 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations1.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations1.js @@ -11,7 +11,7 @@ export let { toString } = 1; "use strict"; Object.defineProperty(exports, "__esModule", { value: true }); exports.toString = void 0; -exports.toString = 1..toString; +exports.toString = (1).toString; { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringInVariableDeclarations3.js b/tests/baselines/reference/destructuringInVariableDeclarations3.js index 01bfa38e040dd..e56b2c0b42280 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations3.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations3.js @@ -12,7 +12,7 @@ define(["require", "exports"], function (require, exports) { "use strict"; Object.defineProperty(exports, "__esModule", { value: true }); exports.toString = void 0; - exports.toString = 1..toString; + exports.toString = (1).toString; { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringInVariableDeclarations5.js b/tests/baselines/reference/destructuringInVariableDeclarations5.js index 91720175a5f6f..3f9fb686afb94 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations5.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations5.js @@ -20,7 +20,7 @@ export let { toString } = 1; "use strict"; Object.defineProperty(exports, "__esModule", { value: true }); exports.toString = void 0; - exports.toString = 1..toString; + exports.toString = (1).toString; { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringInVariableDeclarations7.js b/tests/baselines/reference/destructuringInVariableDeclarations7.js index 2dde4439ba681..e8de6a6556eff 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations7.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations7.js @@ -15,7 +15,7 @@ System.register([], function (exports_1, context_1) { return { setters: [], execute: function () { - exports_1("toString", toString = 1..toString); + exports_1("toString", toString = (1).toString); { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringInVariableDeclarations8.js b/tests/baselines/reference/destructuringInVariableDeclarations8.js index 585601e445d44..91066e2a56657 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations8.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations8.js @@ -16,7 +16,7 @@ System.register([], function (exports_1, context_1) { return { setters: [], execute: function () { - toString = 1..toString; + toString = (1).toString; { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringTypeAssertionsES5_5.js b/tests/baselines/reference/destructuringTypeAssertionsES5_5.js index 9a4f306b69a63..4d4d5a1d2bd25 100644 --- a/tests/baselines/reference/destructuringTypeAssertionsES5_5.js +++ b/tests/baselines/reference/destructuringTypeAssertionsES5_5.js @@ -4,4 +4,4 @@ var { x } = 0; //// [destructuringTypeAssertionsES5_5.js] -var x = 0..x; +var x = (0).x; diff --git a/tests/baselines/reference/destructuringWithNumberLiteral.js b/tests/baselines/reference/destructuringWithNumberLiteral.js index 613ab30c13563..9062f68fa660e 100644 --- a/tests/baselines/reference/destructuringWithNumberLiteral.js +++ b/tests/baselines/reference/destructuringWithNumberLiteral.js @@ -4,4 +4,4 @@ var { toExponential } = 0; //// [destructuringWithNumberLiteral.js] -var toExponential = 0..toExponential; +var toExponential = (0).toExponential; diff --git a/tests/baselines/reference/propertyAccessNumericLiterals.js b/tests/baselines/reference/propertyAccessNumericLiterals.js index 450cffd379c3a..fb9db800e5977 100644 --- a/tests/baselines/reference/propertyAccessNumericLiterals.js +++ b/tests/baselines/reference/propertyAccessNumericLiterals.js @@ -17,8 +17,8 @@ //// [propertyAccessNumericLiterals.js] 0xffffffff.toString(); -668..toString(); -109..toString(); +(668).toString(); +(109).toString(); 1234..toString(); 1e0.toString(); 0..toString(); From 6c60cb6c2457d2c8b9c28a2f30539b591b0725ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 20 Jul 2023 23:17:46 +0200 Subject: [PATCH 3/7] Revert "Move the fix to `parenthesizerRules`" This reverts commit 2ef9980dc8301410523fdd7683c8a28c7835d3dc. --- src/compiler/checker.ts | 12 ++++++++++ src/compiler/emitter.ts | 18 +++++++++++++-- src/compiler/factory/parenthesizerRules.ts | 23 +++---------------- src/compiler/transformers/ts.ts | 13 +++++++---- src/compiler/types.ts | 1 + src/compiler/utilities.ts | 8 ------- .../reference/castExpressionParentheses.js | 6 ++--- .../reference/constEnumPropertyAccess3.js | 18 +++++++-------- .../reference/constEnumToStringNoComments.js | 8 +++---- .../constEnumToStringWithComments.js | 20 ++++++++-------- .../reference/destructuringControlFlow.js | 4 ++-- .../destructuringInVariableDeclarations1.js | 2 +- .../destructuringInVariableDeclarations3.js | 2 +- .../destructuringInVariableDeclarations5.js | 2 +- .../destructuringInVariableDeclarations7.js | 2 +- .../destructuringInVariableDeclarations8.js | 2 +- .../destructuringTypeAssertionsES5_5.js | 2 +- .../destructuringWithNumberLiteral.js | 2 +- .../propertyAccessNumericLiterals.js | 4 ++-- 19 files changed, 77 insertions(+), 72 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 73a15fd578777..0d110bd6e616e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -46483,6 +46483,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { return undefined; } + function shouldParenthesizeConstantValue(node: AccessExpression) { + if (isAccessExpression(node.parent) && node.parent.expression === node) { + const value = getConstantValue(node); + return typeof value === "number" && value < 0; + } + return false; + } + function isFunctionType(type: Type): boolean { return !!(type.flags & TypeFlags.Object) && getSignaturesOfType(type, SignatureKind.Call).length > 0; } @@ -46805,6 +46813,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const node = getParseTreeNode(nodeIn, canHaveConstantValue); return node ? getConstantValue(node) : undefined; }, + shouldParenthesizeConstantValue: nodeIn => { + const node = getParseTreeNode(nodeIn, isAccessExpression); + return !!node && shouldParenthesizeConstantValue(node); + }, collectLinkedAliases, getReferencedValueDeclaration, getReferencedValueDeclarations, diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 0e0c198ae1a84..7199968109a54 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -137,6 +137,7 @@ import { getBaseFileName, GetCanonicalFileName, getCommentRange, + getConstantValue, getContainingNodeArray, getDeclarationEmitExtensionForPath, getDeclarationEmitOutputFilePath, @@ -202,6 +203,7 @@ import { InterfaceDeclaration, InternalEmitFlags, IntersectionTypeNode, + isAccessExpression, isArray, isArrowFunction, isBinaryExpression, @@ -237,7 +239,6 @@ import { isModuleDeclaration, isNodeDescendantOf, isNumericLiteral, - isNumericLiteralTextSafeForPropertyAccess, isParenthesizedExpression, isPartiallyEmittedExpression, isPinnedComment, @@ -406,6 +407,7 @@ import { SpreadElement, stableSort, Statement, + stringContains, StringLiteral, supportedJSExtensionsFlat, SwitchStatement, @@ -422,6 +424,7 @@ import { TemplateSpan, TextRange, ThrowStatement, + TokenFlags, tokenToString, tracing, TransformationResult, @@ -1155,6 +1158,7 @@ export const notImplementedResolver: EmitResolver = { isEntityNameVisible: notImplemented, // Returns the constant value this property access resolves to: notImplemented, or 'undefined' for a non-constant getConstantValue: notImplemented, + shouldParenthesizeConstantValue: notImplemented, getReferencedValueDeclaration: notImplemented, getReferencedValueDeclarations: notImplemented, getTypeReferenceSerializationKind: notImplemented, @@ -3052,7 +3056,17 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri const text = getLiteralTextOfNode(expression as LiteralExpression, /*neverAsciiEscape*/ true, /*jsxAttributeEscape*/ false); // If the number will be printed verbatim and it doesn't already contain a dot or an exponent indicator, add one // if the expression doesn't have any comments that will be emitted. - return !isNumericLiteralTextSafeForPropertyAccess(expression, text); + return !(expression.numericLiteralFlags & TokenFlags.WithSpecifier) + && !stringContains(text, tokenToString(SyntaxKind.DotToken)!) + && !stringContains(text, String.fromCharCode(CharacterCodes.E)) + && !stringContains(text, String.fromCharCode(CharacterCodes.e)); + } + else if (isAccessExpression(expression)) { + // check if constant enum value is integer + const constantValue = getConstantValue(expression); + // isFinite handles cases when constantValue is undefined + return typeof constantValue === "number" && isFinite(constantValue) + && Math.floor(constantValue) === constantValue; } } diff --git a/src/compiler/factory/parenthesizerRules.ts b/src/compiler/factory/parenthesizerRules.ts index cc139c82aa1fe..5a1386d567b3f 100644 --- a/src/compiler/factory/parenthesizerRules.ts +++ b/src/compiler/factory/parenthesizerRules.ts @@ -3,7 +3,6 @@ import { BinaryExpression, BinaryOperator, cast, - CharacterCodes, compareValues, Comparison, ConciseBody, @@ -29,7 +28,6 @@ import { isLiteralKind, isNamedTupleMember, isNodeArray, - isNumericLiteralTextSafeForPropertyAccess, isOptionalChain, isTypeOperatorNode, isUnaryExpression, @@ -40,7 +38,6 @@ import { NewExpression, NodeArray, NodeFactory, - NumericLiteral, OperatorPrecedence, OuterExpressionKinds, ParenthesizerRules, @@ -383,23 +380,9 @@ export function createParenthesizerRules(factory: NodeFactory): ParenthesizerRul // new C.x -> not the same as (new C).x // const emittedExpression = skipPartiallyEmittedExpressions(expression); - if (isLeftHandSideExpression(emittedExpression) && (optionalChain || !isOptionalChain(emittedExpression))) { - switch (emittedExpression.kind) { - case SyntaxKind.NewExpression: - if ((emittedExpression as NewExpression).arguments) { - break; - } - // TODO(rbuckton): Verifiy whether `setTextRange` is needed. - return setTextRange(factory.createParenthesizedExpression(expression), expression); - case SyntaxKind.NumericLiteral: { - const literal = emittedExpression as NumericLiteral; - if (literal.text.charCodeAt(0) !== CharacterCodes.minus && isNumericLiteralTextSafeForPropertyAccess(literal)) { - break; - } - // TODO(rbuckton): Verifiy whether `setTextRange` is needed. - return setTextRange(factory.createParenthesizedExpression(expression), expression); - } - } + if (isLeftHandSideExpression(emittedExpression) + && (emittedExpression.kind !== SyntaxKind.NewExpression || (emittedExpression as NewExpression).arguments) + && (optionalChain || !isOptionalChain(emittedExpression))) { // TODO(rbuckton): Verify whether this assertion holds. return expression as LeftHandSideExpression; } diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 8b97637c07722..444334cce042c 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -110,6 +110,7 @@ import { isNamedExportBindings, isNamedImportBindings, isNamespaceExport, + isNumericLiteral, isObjectLiteralElementLike, isParameterPropertyDeclaration, isPrivateIdentifier, @@ -119,6 +120,7 @@ import { isSimpleInlineableExpression, isSourceFile, isStatement, + isStringLiteral, isTemplateLiteral, isTryStatement, JsxOpeningElement, @@ -2669,18 +2671,19 @@ export function transformTypeScript(context: TransformationContext) { function substituteConstantValue(node: PropertyAccessExpression | ElementAccessExpression): LeftHandSideExpression { const constantValue = tryGetConstEnumValue(node); if (constantValue !== undefined) { - // track the constant value on the node for the printer in needsDotDotForPropertyAccess - setConstantValue(node, constantValue); + const substitute = typeof constantValue === "string" ? factory.createStringLiteral(constantValue) : + resolver.shouldParenthesizeConstantValue(node) ? factory.createParenthesizedExpression(factory.createNumericLiteral(constantValue)) : factory.createNumericLiteral(constantValue); - const substitute = typeof constantValue === "string" ? factory.createStringLiteral(constantValue) : factory.createNumericLiteral(constantValue); + // track the constant value on the node for the printer in needsDotDotForPropertyAccess + if (isStringLiteral(substitute) || isNumericLiteral(substitute)) { + setConstantValue(node, constantValue); + } if (!compilerOptions.removeComments) { const originalNode = getOriginalNode(node, isAccessExpression); - addSyntheticTrailingComment(substitute, SyntaxKind.MultiLineCommentTrivia, ` ${safeMultiLineComment(getTextOfNode(originalNode))} `); } return substitute; } - return node; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 63ca3980ca64a..dc94361199d9a 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5700,6 +5700,7 @@ export interface EmitResolver { isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node): SymbolVisibilityResult; // Returns the constant value this property access resolves to, or 'undefined' for a non-constant getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): string | number | undefined; + shouldParenthesizeConstantValue(node: PropertyAccessExpression | ElementAccessExpression): boolean; getReferencedValueDeclaration(reference: Identifier): Declaration | undefined; getReferencedValueDeclarations(reference: Identifier): Declaration[] | undefined; getTypeReferenceSerializationKind(typeName: EntityName, location?: Node): TypeReferenceSerializationKind; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index f495c5b5be31b..b850483038351 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10311,11 +10311,3 @@ export function getTextOfJsxNamespacedName(node: JsxNamespacedName) { export function intrinsicTagNameToString(node: Identifier | JsxNamespacedName) { return isIdentifier(node) ? idText(node) : getTextOfJsxNamespacedName(node); } - -/** @internal */ -export function isNumericLiteralTextSafeForPropertyAccess(numericLiteral: NumericLiteral, text = numericLiteral.text) { - return numericLiteral.numericLiteralFlags & TokenFlags.WithSpecifier - || stringContains(text, tokenToString(SyntaxKind.DotToken)!) - || stringContains(text, String.fromCharCode(CharacterCodes.E)) - || stringContains(text, String.fromCharCode(CharacterCodes.e)); -} diff --git a/tests/baselines/reference/castExpressionParentheses.js b/tests/baselines/reference/castExpressionParentheses.js index cc71b70b89cba..8d2f0a15ddf98 100644 --- a/tests/baselines/reference/castExpressionParentheses.js +++ b/tests/baselines/reference/castExpressionParentheses.js @@ -76,9 +76,9 @@ a; a[0]; a.b["0"]; a().x; -(1).foo; -(1.).foo; -(1.0).foo; +1..foo; +1..foo; +1.0.foo; 12e+34.foo; 0xff.foo; // should keep the parentheses in emit diff --git a/tests/baselines/reference/constEnumPropertyAccess3.js b/tests/baselines/reference/constEnumPropertyAccess3.js index b7f184d281e5f..ab6237c380cd3 100644 --- a/tests/baselines/reference/constEnumPropertyAccess3.js +++ b/tests/baselines/reference/constEnumPropertyAccess3.js @@ -22,12 +22,12 @@ E["E"].toString(); //// [constEnumPropertyAccess3.js] -(-2 /* E.A */).toString(); -(-1 /* E.B */).toString(); -(-3 /* E.C */).toString(); -(-3 /* E.D */).toString(); -(-2 /* E["A"] */).toString(); -(-1 /* E["B"] */).toString(); -(-3 /* E["C"] */).toString(); -(-3 /* E["D"] */).toString(); -(-9 /* E["E"] */).toString(); +(-2) /* E.A */.toString(); +(-1) /* E.B */.toString(); +(-3) /* E.C */.toString(); +(-3) /* E.D */.toString(); +(-2) /* E["A"] */.toString(); +(-1) /* E["B"] */.toString(); +(-3) /* E["C"] */.toString(); +(-3) /* E["D"] */.toString(); +(-9) /* E["E"] */.toString(); diff --git a/tests/baselines/reference/constEnumToStringNoComments.js b/tests/baselines/reference/constEnumToStringNoComments.js index 692fddca792e6..4ab485ee16635 100644 --- a/tests/baselines/reference/constEnumToStringNoComments.js +++ b/tests/baselines/reference/constEnumToStringNoComments.js @@ -25,12 +25,12 @@ let c1 = Foo["C"].toString(); //// [constEnumToStringNoComments.js] -var x0 = (100).toString(); -var x1 = (100).toString(); +var x0 = 100..toString(); +var x1 = 100..toString(); var y0 = 0.5.toString(); var y1 = 0.5.toString(); -var z0 = (2).toString(); -var z1 = (2).toString(); +var z0 = 2..toString(); +var z1 = 2..toString(); var a0 = (-1).toString(); var a1 = (-1).toString(); var b0 = (-1.5).toString(); diff --git a/tests/baselines/reference/constEnumToStringWithComments.js b/tests/baselines/reference/constEnumToStringWithComments.js index f3509f0b69b90..ad5b3305cea7a 100644 --- a/tests/baselines/reference/constEnumToStringWithComments.js +++ b/tests/baselines/reference/constEnumToStringWithComments.js @@ -25,15 +25,15 @@ let c1 = Foo["C"].toString(); //// [constEnumToStringWithComments.js] -var x0 = (100 /* Foo.X */).toString(); -var x1 = (100 /* Foo["X"] */).toString(); +var x0 = 100 /* Foo.X */.toString(); +var x1 = 100 /* Foo["X"] */.toString(); var y0 = 0.5 /* Foo.Y */.toString(); var y1 = 0.5 /* Foo["Y"] */.toString(); -var z0 = (2 /* Foo.Z */).toString(); -var z1 = (2 /* Foo["Z"] */).toString(); -var a0 = (-1 /* Foo.A */).toString(); -var a1 = (-1 /* Foo["A"] */).toString(); -var b0 = (-1.5 /* Foo.B */).toString(); -var b1 = (-1.5 /* Foo["B"] */).toString(); -var c0 = (-1 /* Foo.C */).toString(); -var c1 = (-1 /* Foo["C"] */).toString(); +var z0 = 2 /* Foo.Z */.toString(); +var z1 = 2 /* Foo["Z"] */.toString(); +var a0 = (-1) /* Foo.A */.toString(); +var a1 = (-1) /* Foo["A"] */.toString(); +var b0 = (-1.5) /* Foo.B */.toString(); +var b1 = (-1.5) /* Foo["B"] */.toString(); +var c0 = (-1) /* Foo.C */.toString(); +var c1 = (-1) /* Foo["C"] */.toString(); diff --git a/tests/baselines/reference/destructuringControlFlow.js b/tests/baselines/reference/destructuringControlFlow.js index f17b8214773c3..1f444d4d56df1 100644 --- a/tests/baselines/reference/destructuringControlFlow.js +++ b/tests/baselines/reference/destructuringControlFlow.js @@ -73,8 +73,8 @@ function f3(obj) { function f4() { var _a, _b; var x; - (x = (0).x); // Error - (x = (0)["x"]); // Error + (x = 0..x); // Error + (x = 0["x"]); // Error (_a = 0, _b = "x" + "", x = _a[_b]); // Errpr } var _a = ["foo"], key = _a[0], value = _a[1]; diff --git a/tests/baselines/reference/destructuringInVariableDeclarations1.js b/tests/baselines/reference/destructuringInVariableDeclarations1.js index c9b8457aa0676..365697ec80f9e 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations1.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations1.js @@ -11,7 +11,7 @@ export let { toString } = 1; "use strict"; Object.defineProperty(exports, "__esModule", { value: true }); exports.toString = void 0; -exports.toString = (1).toString; +exports.toString = 1..toString; { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringInVariableDeclarations3.js b/tests/baselines/reference/destructuringInVariableDeclarations3.js index e56b2c0b42280..01bfa38e040dd 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations3.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations3.js @@ -12,7 +12,7 @@ define(["require", "exports"], function (require, exports) { "use strict"; Object.defineProperty(exports, "__esModule", { value: true }); exports.toString = void 0; - exports.toString = (1).toString; + exports.toString = 1..toString; { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringInVariableDeclarations5.js b/tests/baselines/reference/destructuringInVariableDeclarations5.js index 3f9fb686afb94..91720175a5f6f 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations5.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations5.js @@ -20,7 +20,7 @@ export let { toString } = 1; "use strict"; Object.defineProperty(exports, "__esModule", { value: true }); exports.toString = void 0; - exports.toString = (1).toString; + exports.toString = 1..toString; { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringInVariableDeclarations7.js b/tests/baselines/reference/destructuringInVariableDeclarations7.js index e8de6a6556eff..2dde4439ba681 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations7.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations7.js @@ -15,7 +15,7 @@ System.register([], function (exports_1, context_1) { return { setters: [], execute: function () { - exports_1("toString", toString = (1).toString); + exports_1("toString", toString = 1..toString); { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringInVariableDeclarations8.js b/tests/baselines/reference/destructuringInVariableDeclarations8.js index 91066e2a56657..585601e445d44 100644 --- a/tests/baselines/reference/destructuringInVariableDeclarations8.js +++ b/tests/baselines/reference/destructuringInVariableDeclarations8.js @@ -16,7 +16,7 @@ System.register([], function (exports_1, context_1) { return { setters: [], execute: function () { - toString = (1).toString; + toString = 1..toString; { let { toFixed } = 1; } diff --git a/tests/baselines/reference/destructuringTypeAssertionsES5_5.js b/tests/baselines/reference/destructuringTypeAssertionsES5_5.js index 4d4d5a1d2bd25..9a4f306b69a63 100644 --- a/tests/baselines/reference/destructuringTypeAssertionsES5_5.js +++ b/tests/baselines/reference/destructuringTypeAssertionsES5_5.js @@ -4,4 +4,4 @@ var { x } = 0; //// [destructuringTypeAssertionsES5_5.js] -var x = (0).x; +var x = 0..x; diff --git a/tests/baselines/reference/destructuringWithNumberLiteral.js b/tests/baselines/reference/destructuringWithNumberLiteral.js index 9062f68fa660e..613ab30c13563 100644 --- a/tests/baselines/reference/destructuringWithNumberLiteral.js +++ b/tests/baselines/reference/destructuringWithNumberLiteral.js @@ -4,4 +4,4 @@ var { toExponential } = 0; //// [destructuringWithNumberLiteral.js] -var toExponential = (0).toExponential; +var toExponential = 0..toExponential; diff --git a/tests/baselines/reference/propertyAccessNumericLiterals.js b/tests/baselines/reference/propertyAccessNumericLiterals.js index fb9db800e5977..450cffd379c3a 100644 --- a/tests/baselines/reference/propertyAccessNumericLiterals.js +++ b/tests/baselines/reference/propertyAccessNumericLiterals.js @@ -17,8 +17,8 @@ //// [propertyAccessNumericLiterals.js] 0xffffffff.toString(); -(668).toString(); -(109).toString(); +668..toString(); +109..toString(); 1234..toString(); 1e0.toString(); 0..toString(); From 577d9fcbe517eef4d0e1d4876a0357ba7b3a8b1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 20 Jul 2023 23:33:41 +0200 Subject: [PATCH 4/7] Properly create a prefix unary expression for inlined negative numbers --- src/compiler/checker.ts | 12 ------------ src/compiler/emitter.ts | 1 - src/compiler/transformers/ts.ts | 6 +++--- src/compiler/types.ts | 1 - .../reference/constEnumPropertyAccess3.js | 18 +++++++++--------- .../reference/constEnumToStringWithComments.js | 12 ++++++------ 6 files changed, 18 insertions(+), 32 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 0d110bd6e616e..73a15fd578777 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -46483,14 +46483,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { return undefined; } - function shouldParenthesizeConstantValue(node: AccessExpression) { - if (isAccessExpression(node.parent) && node.parent.expression === node) { - const value = getConstantValue(node); - return typeof value === "number" && value < 0; - } - return false; - } - function isFunctionType(type: Type): boolean { return !!(type.flags & TypeFlags.Object) && getSignaturesOfType(type, SignatureKind.Call).length > 0; } @@ -46813,10 +46805,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const node = getParseTreeNode(nodeIn, canHaveConstantValue); return node ? getConstantValue(node) : undefined; }, - shouldParenthesizeConstantValue: nodeIn => { - const node = getParseTreeNode(nodeIn, isAccessExpression); - return !!node && shouldParenthesizeConstantValue(node); - }, collectLinkedAliases, getReferencedValueDeclaration, getReferencedValueDeclarations, diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 7199968109a54..a58223868f8b0 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -1158,7 +1158,6 @@ export const notImplementedResolver: EmitResolver = { isEntityNameVisible: notImplemented, // Returns the constant value this property access resolves to: notImplemented, or 'undefined' for a non-constant getConstantValue: notImplemented, - shouldParenthesizeConstantValue: notImplemented, getReferencedValueDeclaration: notImplemented, getReferencedValueDeclarations: notImplemented, getTypeReferenceSerializationKind: notImplemented, diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 444334cce042c..c61a399fba241 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -125,7 +125,6 @@ import { isTryStatement, JsxOpeningElement, JsxSelfClosingElement, - LeftHandSideExpression, map, mapDefined, MethodDeclaration, @@ -2668,11 +2667,12 @@ export function transformTypeScript(context: TransformationContext) { return value.replace(/\*\//g, "*_/"); } - function substituteConstantValue(node: PropertyAccessExpression | ElementAccessExpression): LeftHandSideExpression { + function substituteConstantValue(node: PropertyAccessExpression | ElementAccessExpression) { const constantValue = tryGetConstEnumValue(node); if (constantValue !== undefined) { const substitute = typeof constantValue === "string" ? factory.createStringLiteral(constantValue) : - resolver.shouldParenthesizeConstantValue(node) ? factory.createParenthesizedExpression(factory.createNumericLiteral(constantValue)) : factory.createNumericLiteral(constantValue); + constantValue < 0 ? factory.createPrefixUnaryExpression(SyntaxKind.MinusToken, factory.createNumericLiteral(Math.abs(constantValue))) : + factory.createNumericLiteral(constantValue); // track the constant value on the node for the printer in needsDotDotForPropertyAccess if (isStringLiteral(substitute) || isNumericLiteral(substitute)) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index dc94361199d9a..63ca3980ca64a 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5700,7 +5700,6 @@ export interface EmitResolver { isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node): SymbolVisibilityResult; // Returns the constant value this property access resolves to, or 'undefined' for a non-constant getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): string | number | undefined; - shouldParenthesizeConstantValue(node: PropertyAccessExpression | ElementAccessExpression): boolean; getReferencedValueDeclaration(reference: Identifier): Declaration | undefined; getReferencedValueDeclarations(reference: Identifier): Declaration[] | undefined; getTypeReferenceSerializationKind(typeName: EntityName, location?: Node): TypeReferenceSerializationKind; diff --git a/tests/baselines/reference/constEnumPropertyAccess3.js b/tests/baselines/reference/constEnumPropertyAccess3.js index ab6237c380cd3..b7f184d281e5f 100644 --- a/tests/baselines/reference/constEnumPropertyAccess3.js +++ b/tests/baselines/reference/constEnumPropertyAccess3.js @@ -22,12 +22,12 @@ E["E"].toString(); //// [constEnumPropertyAccess3.js] -(-2) /* E.A */.toString(); -(-1) /* E.B */.toString(); -(-3) /* E.C */.toString(); -(-3) /* E.D */.toString(); -(-2) /* E["A"] */.toString(); -(-1) /* E["B"] */.toString(); -(-3) /* E["C"] */.toString(); -(-3) /* E["D"] */.toString(); -(-9) /* E["E"] */.toString(); +(-2 /* E.A */).toString(); +(-1 /* E.B */).toString(); +(-3 /* E.C */).toString(); +(-3 /* E.D */).toString(); +(-2 /* E["A"] */).toString(); +(-1 /* E["B"] */).toString(); +(-3 /* E["C"] */).toString(); +(-3 /* E["D"] */).toString(); +(-9 /* E["E"] */).toString(); diff --git a/tests/baselines/reference/constEnumToStringWithComments.js b/tests/baselines/reference/constEnumToStringWithComments.js index ad5b3305cea7a..22d160b033af2 100644 --- a/tests/baselines/reference/constEnumToStringWithComments.js +++ b/tests/baselines/reference/constEnumToStringWithComments.js @@ -31,9 +31,9 @@ var y0 = 0.5 /* Foo.Y */.toString(); var y1 = 0.5 /* Foo["Y"] */.toString(); var z0 = 2 /* Foo.Z */.toString(); var z1 = 2 /* Foo["Z"] */.toString(); -var a0 = (-1) /* Foo.A */.toString(); -var a1 = (-1) /* Foo["A"] */.toString(); -var b0 = (-1.5) /* Foo.B */.toString(); -var b1 = (-1.5) /* Foo["B"] */.toString(); -var c0 = (-1) /* Foo.C */.toString(); -var c1 = (-1) /* Foo["C"] */.toString(); +var a0 = (-1 /* Foo.A */).toString(); +var a1 = (-1 /* Foo["A"] */).toString(); +var b0 = (-1.5 /* Foo.B */).toString(); +var b1 = (-1.5 /* Foo["B"] */).toString(); +var c0 = (-1 /* Foo.C */).toString(); +var c1 = (-1 /* Foo["C"] */).toString(); From 28da083c89e7f499887a1e0ef8f16fd9f9053887 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 20 Jul 2023 23:43:46 +0200 Subject: [PATCH 5/7] Add evaluation-based test --- src/testRunner/tests.ts | 1 + .../unittests/evaluation/constEnum.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 src/testRunner/unittests/evaluation/constEnum.ts diff --git a/src/testRunner/tests.ts b/src/testRunner/tests.ts index 19474a4f4bea0..1a3da111540f4 100644 --- a/src/testRunner/tests.ts +++ b/src/testRunner/tests.ts @@ -34,6 +34,7 @@ import "./unittests/evaluation/asyncArrow"; import "./unittests/evaluation/asyncGenerator"; import "./unittests/evaluation/autoAccessors"; import "./unittests/evaluation/awaiter"; +import "./unittests/evaluation/constEnum"; import "./unittests/evaluation/destructuring"; import "./unittests/evaluation/externalModules"; import "./unittests/evaluation/esDecorators"; diff --git a/src/testRunner/unittests/evaluation/constEnum.ts b/src/testRunner/unittests/evaluation/constEnum.ts new file mode 100644 index 0000000000000..8074c00627ff6 --- /dev/null +++ b/src/testRunner/unittests/evaluation/constEnum.ts @@ -0,0 +1,17 @@ +import * as evaluator from "../../_namespaces/evaluator"; + +describe("unittests:: evaluation:: constEnum", () => { + it("correct order of operations for inlined negative numbers", async () => { + const result = evaluator.evaluateTypeScript(` + const enum TestEnum { + A = 1, + B = -1 + } + + export const a = typeof TestEnum.A.toString(); + export const b = typeof TestEnum.B.toString(); + `); + assert.equal(result.a, "string"); + assert.equal(result.b, "string"); + }); +}); From 88d83050ab2f339efd4cc255f84683cb68e4a9a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 20 Jul 2023 23:59:56 +0200 Subject: [PATCH 6/7] Always track constant values on nodes --- src/compiler/emitter.ts | 4 ++-- src/compiler/transformers/ts.ts | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index a58223868f8b0..36ba6d65a22b0 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -3061,11 +3061,11 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri && !stringContains(text, String.fromCharCode(CharacterCodes.e)); } else if (isAccessExpression(expression)) { - // check if constant enum value is integer + // check if constant enum value is a non-negative integer const constantValue = getConstantValue(expression); // isFinite handles cases when constantValue is undefined return typeof constantValue === "number" && isFinite(constantValue) - && Math.floor(constantValue) === constantValue; + && constantValue >= 0 && Math.floor(constantValue) === constantValue; } } diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index c61a399fba241..569ab7985bdac 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -110,7 +110,6 @@ import { isNamedExportBindings, isNamedImportBindings, isNamespaceExport, - isNumericLiteral, isObjectLiteralElementLike, isParameterPropertyDeclaration, isPrivateIdentifier, @@ -120,7 +119,6 @@ import { isSimpleInlineableExpression, isSourceFile, isStatement, - isStringLiteral, isTemplateLiteral, isTryStatement, JsxOpeningElement, @@ -2670,15 +2668,13 @@ export function transformTypeScript(context: TransformationContext) { function substituteConstantValue(node: PropertyAccessExpression | ElementAccessExpression) { const constantValue = tryGetConstEnumValue(node); if (constantValue !== undefined) { + // track the constant value on the node for the printer in mayNeedDotDotForPropertyAccess + setConstantValue(node, constantValue); const substitute = typeof constantValue === "string" ? factory.createStringLiteral(constantValue) : constantValue < 0 ? factory.createPrefixUnaryExpression(SyntaxKind.MinusToken, factory.createNumericLiteral(Math.abs(constantValue))) : factory.createNumericLiteral(constantValue); - // track the constant value on the node for the printer in needsDotDotForPropertyAccess - if (isStringLiteral(substitute) || isNumericLiteral(substitute)) { - setConstantValue(node, constantValue); - } - if (!compilerOptions.removeComments) { + if (!compilerOptions.removeComments) { const originalNode = getOriginalNode(node, isAccessExpression); addSyntheticTrailingComment(substitute, SyntaxKind.MultiLineCommentTrivia, ` ${safeMultiLineComment(getTextOfNode(originalNode))} `); } From 7e14a9eb4b6f4bb44982d553c7e983189b691b83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Fri, 21 Jul 2023 00:33:19 +0200 Subject: [PATCH 7/7] fix broken indent --- src/compiler/transformers/ts.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index 569ab7985bdac..d94554c128b81 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -2674,7 +2674,7 @@ export function transformTypeScript(context: TransformationContext) { constantValue < 0 ? factory.createPrefixUnaryExpression(SyntaxKind.MinusToken, factory.createNumericLiteral(Math.abs(constantValue))) : factory.createNumericLiteral(constantValue); - if (!compilerOptions.removeComments) { + if (!compilerOptions.removeComments) { const originalNode = getOriginalNode(node, isAccessExpression); addSyntheticTrailingComment(substitute, SyntaxKind.MultiLineCommentTrivia, ` ${safeMultiLineComment(getTextOfNode(originalNode))} `); }