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

isolatedModules errors for non-literal enum initializers #56736

Merged
merged 11 commits into from Mar 20, 2024
28 changes: 22 additions & 6 deletions src/compiler/checker.ts
Expand Up @@ -719,6 +719,8 @@ import {
isStringOrNumericLiteralLike,
isSuperCall,
isSuperProperty,
isSyntacticallyNumeric,
isSyntacticallyString,
isTaggedTemplateExpression,
isTemplateSpan,
isThisContainerOrFunctionBlock,
Expand Down Expand Up @@ -45518,15 +45520,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (!(nodeLinks.flags & NodeCheckFlags.EnumValuesComputed)) {
nodeLinks.flags |= NodeCheckFlags.EnumValuesComputed;
let autoValue: number | undefined = 0;
let previous: EnumMember | undefined;
for (const member of node.members) {
const value = computeMemberValue(member, autoValue);
const value = computeMemberValue(member, autoValue, previous);
getNodeLinks(member).enumMemberValue = value;
autoValue = typeof value === "number" ? value + 1 : undefined;
previous = member;
}
}
}

function computeMemberValue(member: EnumMember, autoValue: number | undefined) {
function computeMemberValue(member: EnumMember, autoValue: number | undefined, previous: EnumMember | undefined) {
if (isComputedNonLiteralName(member.name)) {
error(member.name, Diagnostics.Computed_property_names_are_not_allowed_in_enums);
}
Expand All @@ -45548,11 +45552,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// If the member is the first member in the enum declaration, it is assigned the value zero.
// Otherwise, it is assigned the value of the immediately preceding member plus one, and an error
// occurs if the immediately preceding member is not a constant enum member.
if (autoValue !== undefined) {
return autoValue;
if (autoValue === undefined) {
error(member.name, Diagnostics.Enum_member_must_have_initializer);
return undefined;
}
error(member.name, Diagnostics.Enum_member_must_have_initializer);
return undefined;
if (getIsolatedModules(compilerOptions) && previous?.initializer && !isSyntacticallyNumeric(previous.initializer)) {
error(
member.name,
Diagnostics.Enum_member_following_a_non_literal_numeric_member_must_have_an_initializer_when_isolatedModules_is_enabled,
);
}
return autoValue;
}

function computeConstantValue(member: EnumMember): string | number | undefined {
Expand All @@ -45568,6 +45578,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
Diagnostics.const_enum_member_initializer_was_evaluated_to_a_non_finite_value,
);
}
else if (getIsolatedModules(compilerOptions) && typeof value === "string" && !isSyntacticallyString(initializer)) {
error(
initializer,
Diagnostics.A_string_member_initializer_in_a_enum_declaration_can_only_use_constant_expressions_when_isolatedModules_is_enabled,
);
}
}
else if (isConstEnum) {
error(initializer, Diagnostics.const_enum_member_initializers_must_be_constant_expressions);
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Expand Up @@ -7976,5 +7976,13 @@
"'await using' statements cannot be used inside a class static block.": {
"category": "Error",
"code": 18054
},
"A string member initializer in a enum declaration can only use constant expressions when 'isolatedModules' is enabled.": {
Copy link
Member

Choose a reason for hiding this comment

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

Paraphrasing a suggestion from @DanielRosenwasser:

Suggested change
"A string member initializer in a enum declaration can only use constant expressions when 'isolatedModules' is enabled.": {
"'{0}' has a string type, but must have syntactically recognizable string syntax when 'isolatedModules' is enabled.": {

We could totally do a quick fix to wrap it in a template literal but I think this error is going to be so niche that I’m not sure it’s worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the suggested error message. But what do I substitute {0} with? The initializer can be a relatively complex expression. Is there a utility function to stringify that? Or should I use the enum member name or even the computed value?

Copy link
Member

Choose a reason for hiding this comment

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

I forgot to include that; Daniel’s suggestion was 'EnumName.MemberName'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Do we want to handle member names that are not valid identifier? E.g.:

import {bar} from './bar';
enum Foo { ['not an identifier'] = bar }
// 'Foo.not an identifier' has a string type, ...
// 'Foo["not an identifier"]' has a string type, ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the error message and ignored the non-identifier case for now, because it seems like an extremely rare case. That means it would print 'Foo.not an identifier' in the error message.

"category": "Error",
"code": 18055
},
"Enum member following a non-literal numeric member must have an initializer when 'isolatedModules' is enabled.": {
"category": "Error",
"code": 18056
}
}
14 changes: 14 additions & 0 deletions src/compiler/utilities.ts
Expand Up @@ -10657,3 +10657,17 @@ export function isSyntacticallyString(expr: Expression): boolean {
}
return false;
}

/** @internal */
export function isSyntacticallyNumeric(expr: Expression): boolean {
frigus02 marked this conversation as resolved.
Show resolved Hide resolved
expr = skipOuterExpressions(expr);
switch (expr.kind) {
case SyntaxKind.PrefixUnaryExpression:
return isSyntacticallyNumeric((expr as PrefixUnaryExpression).operand);
case SyntaxKind.BinaryExpression:
return isSyntacticallyNumeric((expr as BinaryExpression).left) && isSyntacticallyNumeric((expr as BinaryExpression).right);
case SyntaxKind.NumericLiteral:
return true;
}
return false;
}
@@ -0,0 +1,34 @@
bad.ts(4,5): error TS18056: Enum member following a non-literal numeric member must have an initializer when 'isolatedModules' is enabled.


==== ./helpers.ts (0 errors) ====
export const foo = 2;

==== ./bad.ts (1 errors) ====
import { foo } from "./helpers";
enum A {
a = foo,
b,
~
!!! error TS18056: Enum member following a non-literal numeric member must have an initializer when 'isolatedModules' is enabled.
}

==== ./good.ts (0 errors) ====
import { foo } from "./helpers";
enum A {
a = foo,
b = 3,
}
enum B {
a = 1 + 1,
b,
}
enum C {
a = +2,
b,
}
enum D {
a = (2),
b,
}

@@ -0,0 +1,70 @@
//// [tests/cases/compiler/enumNoInitializerFollowsNonLiteralInitializer.ts] ////

//// [helpers.ts]
export const foo = 2;

//// [bad.ts]
import { foo } from "./helpers";
enum A {
a = foo,
b,
}

//// [good.ts]
import { foo } from "./helpers";
enum A {
a = foo,
b = 3,
}
enum B {
a = 1 + 1,
b,
}
enum C {
a = +2,
b,
}
enum D {
a = (2),
b,
}


//// [helpers.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = void 0;
exports.foo = 2;
//// [bad.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var helpers_1 = require("./helpers");
var A;
(function (A) {
A[A["a"] = 2] = "a";
A[A["b"] = 3] = "b";
})(A || (A = {}));
//// [good.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var helpers_1 = require("./helpers");
var A;
(function (A) {
A[A["a"] = 2] = "a";
A[A["b"] = 3] = "b";
})(A || (A = {}));
var B;
(function (B) {
B[B["a"] = 2] = "a";
B[B["b"] = 3] = "b";
})(B || (B = {}));
var C;
(function (C) {
C[C["a"] = 2] = "a";
C[C["b"] = 3] = "b";
})(C || (C = {}));
var D;
(function (D) {
D[D["a"] = 2] = "a";
D[D["b"] = 3] = "b";
})(D || (D = {}));
@@ -0,0 +1,24 @@
bad.ts(3,8): error TS18055: A string member initializer in a enum declaration can only use constant expressions when 'isolatedModules' is enabled.


==== ./helpers.ts (0 errors) ====
export const foo = 2;
export const bar = "bar";

==== ./bad.ts (1 errors) ====
import { bar } from "./helpers";
enum A {
a = bar,
~~~
!!! error TS18055: A string member initializer in a enum declaration can only use constant expressions when 'isolatedModules' is enabled.
}

==== ./good.ts (0 errors) ====
import { foo } from "./helpers";
enum A {
a = `${foo}`,
b = "" + 2,
c = 2 + "",
d = ("foo"),
}

47 changes: 47 additions & 0 deletions tests/baselines/reference/enumWithNonLiteralStringInitializer.js
@@ -0,0 +1,47 @@
//// [tests/cases/compiler/enumWithNonLiteralStringInitializer.ts] ////

//// [helpers.ts]
export const foo = 2;
export const bar = "bar";

//// [bad.ts]
import { bar } from "./helpers";
enum A {
a = bar,
}

//// [good.ts]
import { foo } from "./helpers";
enum A {
a = `${foo}`,
b = "" + 2,
c = 2 + "",
d = ("foo"),
}


//// [helpers.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.bar = exports.foo = void 0;
exports.foo = 2;
exports.bar = "bar";
//// [bad.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var helpers_1 = require("./helpers");
var A;
(function (A) {
A["a"] = "bar";
})(A || (A = {}));
//// [good.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var helpers_1 = require("./helpers");
var A;
(function (A) {
A["a"] = "2";
A["b"] = "2";
A["c"] = "2";
A["d"] = "foo";
})(A || (A = {}));
@@ -1,3 +1,4 @@
enum2.ts(2,9): error TS18055: A string member initializer in a enum declaration can only use constant expressions when 'isolatedModules' is enabled.
enum2.ts(3,9): error TS1281: Cannot access 'A' from another file without qualification when 'isolatedModules' is enabled. Use 'Enum.A' instead.
enum2.ts(4,9): error TS1281: Cannot access 'X' from another file without qualification when 'isolatedModules' is enabled. Use 'Enum.X' instead.
script-namespaces.ts(1,11): error TS1280: Namespaces are not allowed in global script files when 'isolatedModules' is enabled. If this file is not intended to be a global script, set 'moduleDetection' to 'force' or add an empty 'export {}' statement.
Expand Down Expand Up @@ -26,9 +27,11 @@ script-namespaces.ts(1,11): error TS1280: Namespaces are not allowed in global s
declare enum Enum { X = 1_000_000 }
const d = 'd';

==== enum2.ts (2 errors) ====
==== enum2.ts (3 errors) ====
enum Enum {
D = d,
~
!!! error TS18055: A string member initializer in a enum declaration can only use constant expressions when 'isolatedModules' is enabled.
E = A, // error
~
!!! error TS1281: Cannot access 'A' from another file without qualification when 'isolatedModules' is enabled. Use 'Enum.A' instead.
Expand Down
@@ -0,0 +1,31 @@
// @isolatedModules: true
// @noTypesAndSymbols: true

// @filename: ./helpers.ts
export const foo = 2;

// @filename: ./bad.ts
import { foo } from "./helpers";
enum A {
a = foo,
b,
}

// @filename: ./good.ts
import { foo } from "./helpers";
enum A {
a = foo,
b = 3,
}
enum B {
a = 1 + 1,
b,
}
enum C {
a = +2,
b,
}
enum D {
a = (2),
b,
}
21 changes: 21 additions & 0 deletions tests/cases/compiler/enumWithNonLiteralStringInitializer.ts
@@ -0,0 +1,21 @@
// @isolatedModules: true
// @noTypesAndSymbols: true

// @filename: ./helpers.ts
export const foo = 2;
export const bar = "bar";

// @filename: ./bad.ts
import { bar } from "./helpers";
enum A {
a = bar,
}

// @filename: ./good.ts
import { foo } from "./helpers";
enum A {
a = `${foo}`,
b = "" + 2,
c = 2 + "",
d = ("foo"),
}