Skip to content

Commit

Permalink
Wrap inline negative const enum with parens (#55065)
Browse files Browse the repository at this point in the history
Co-authored-by: Oleksandr T <[email protected]>
  • Loading branch information
Andarist and a-tarasyuk authored Jul 31, 2023
1 parent cd23992 commit 6c942fa
Show file tree
Hide file tree
Showing 10 changed files with 279 additions and 20 deletions.
4 changes: 2 additions & 2 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
11 changes: 5 additions & 6 deletions src/compiler/transformers/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ import {
isTryStatement,
JsxOpeningElement,
JsxSelfClosingElement,
LeftHandSideExpression,
map,
mapDefined,
MethodDeclaration,
Expand Down Expand Up @@ -2666,21 +2665,21 @@ 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) {
// track the constant value on the node for the printer in needsDotDotForPropertyAccess
// 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);

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;
}

Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
17 changes: 17 additions & 0 deletions src/testRunner/unittests/evaluation/constEnum.ts
Original file line number Diff line number Diff line change
@@ -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");
});
});
33 changes: 33 additions & 0 deletions tests/baselines/reference/constEnumPropertyAccess3.js
Original file line number Diff line number Diff line change
@@ -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();
80 changes: 80 additions & 0 deletions tests/baselines/reference/constEnumPropertyAccess3.symbols
Original file line number Diff line number Diff line change
@@ -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, --, --))

111 changes: 111 additions & 0 deletions tests/baselines/reference/constEnumPropertyAccess3.types
Original file line number Diff line number Diff line change
@@ -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

12 changes: 6 additions & 6 deletions tests/baselines/reference/constEnumToStringNoComments.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
12 changes: 6 additions & 6 deletions tests/baselines/reference/constEnumToStringWithComments.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
18 changes: 18 additions & 0 deletions tests/cases/conformance/constEnums/constEnumPropertyAccess3.ts
Original file line number Diff line number Diff line change
@@ -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();

0 comments on commit 6c942fa

Please sign in to comment.