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
64 changes: 58 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45010,15 +45010,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 @@ -45040,11 +45042,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)) {
frigus02 marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -45060,6 +45068,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 All @@ -45073,6 +45087,44 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return value;
}

function isSyntacticallyNumeric(expr: Expression): boolean {
frigus02 marked this conversation as resolved.
Show resolved Hide resolved
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.ParenthesizedExpression:
return isSyntacticallyNumeric((expr as ParenthesizedExpression).expression);
case SyntaxKind.NumericLiteral:
return true;
}
return false;
}

function isSyntacticallyString(expr: Expression): boolean {
switch (expr.kind) {
case SyntaxKind.BinaryExpression:
const left = (expr as BinaryExpression).left;
const right = (expr as BinaryExpression).right;
const leftIsNumeric = isSyntacticallyNumeric(left);
const rightIsNumeric = isSyntacticallyNumeric(right);
return (
!(leftIsNumeric && rightIsNumeric) &&
(isSyntacticallyString(left) || leftIsNumeric) &&
(isSyntacticallyString(right) || rightIsNumeric) &&
(expr as BinaryExpression).operatorToken.kind === SyntaxKind.PlusToken
);
case SyntaxKind.TemplateExpression:
return (expr as TemplateExpression).templateSpans.every(span => isSyntacticallyString(span.expression));
andrewbranch marked this conversation as resolved.
Show resolved Hide resolved
case SyntaxKind.ParenthesizedExpression:
return isSyntacticallyString((expr as ParenthesizedExpression).expression);
case SyntaxKind.StringLiteral:
case SyntaxKind.NoSubstitutionTemplateLiteral:
return true;
}
return false;
}

function evaluate(expr: Expression, location?: Declaration): string | number | undefined {
switch (expr.kind) {
case SyntaxKind.PrefixUnaryExpression:
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -7936,5 +7936,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
}
}
Original file line number Diff line number Diff line change
@@ -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,
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
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;

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

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

Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// @isolatedModules: true
// @noEmit: true
frigus02 marked this conversation as resolved.
Show resolved Hide resolved
// @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,
}
20 changes: 20 additions & 0 deletions tests/cases/compiler/enumWithNonLiteralStringInitializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// @isolatedModules: true
// @noEmit: true
frigus02 marked this conversation as resolved.
Show resolved Hide resolved
// @noTypesAndSymbols: true

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

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

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