-
Notifications
You must be signed in to change notification settings - Fork 13k
align behavior of constant expressions in initializers of ambient enu… #4946
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12940,26 +12940,41 @@ namespace ts { | |
if (!(nodeLinks.flags & NodeCheckFlags.EnumValuesComputed)) { | ||
let enumSymbol = getSymbolOfNode(node); | ||
let enumType = getDeclaredTypeOfSymbol(enumSymbol); | ||
let autoValue = 0; | ||
let autoValue = 0; // set to undefined when enum member is non-constant | ||
let ambient = isInAmbientContext(node); | ||
let enumIsConst = isConst(node); | ||
|
||
forEach(node.members, member => { | ||
if (member.name.kind !== SyntaxKind.ComputedPropertyName && isNumericLiteralName((<Identifier>member.name).text)) { | ||
for (const member of node.members) { | ||
if (member.name.kind === SyntaxKind.ComputedPropertyName) { | ||
error(member.name, Diagnostics.Computed_property_names_are_not_allowed_in_enums); | ||
} | ||
else if (isNumericLiteralName((<Identifier>member.name).text)) { | ||
error(member.name, Diagnostics.An_enum_member_cannot_have_a_numeric_name); | ||
} | ||
|
||
const previousEnumMemberIsNonConstant = autoValue === undefined; | ||
|
||
let initializer = member.initializer; | ||
if (initializer) { | ||
autoValue = computeConstantValueForEnumMemberInitializer(initializer, enumType, enumIsConst, ambient); | ||
} | ||
else if (ambient && !enumIsConst) { | ||
// In ambient enum declarations that specify no const modifier, enum member declarations | ||
// that omit a value are considered computed members (as opposed to having auto-incremented values assigned). | ||
autoValue = undefined; | ||
} | ||
else if (previousEnumMemberIsNonConstant) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: since u only use this variable here, just do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to have the constant since it makes it clearer |
||
// If the member declaration specifies no value, the member is considered a constant enum member. | ||
// 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 | ||
error(member.name, Diagnostics.Enum_member_must_have_initializer); | ||
} | ||
|
||
if (autoValue !== undefined) { | ||
getNodeLinks(member).enumMemberValue = autoValue++; | ||
} | ||
}); | ||
} | ||
|
||
nodeLinks.flags |= NodeCheckFlags.EnumValuesComputed; | ||
} | ||
|
@@ -12975,11 +12990,11 @@ namespace ts { | |
if (enumIsConst) { | ||
error(initializer, Diagnostics.In_const_enum_declarations_member_initializer_must_be_constant_expression); | ||
} | ||
else if (!ambient) { | ||
else if (ambient) { | ||
error(initializer, Diagnostics.In_ambient_enum_declarations_member_initializer_must_be_constant_expression); | ||
} | ||
else { | ||
// Only here do we need to check that the initializer is assignable to the enum type. | ||
// If it is a constant value (not undefined), it is syntactically constrained to be a number. | ||
// Also, we do not need to check this for ambients because there is already | ||
// a syntax error if it is not a constant. | ||
checkTypeAssignableTo(checkExpression(initializer), enumType, initializer, /*headMessage*/ undefined); | ||
} | ||
} | ||
|
@@ -13119,7 +13134,7 @@ namespace ts { | |
} | ||
|
||
// Grammar checking | ||
checkGrammarDecorators(node) || checkGrammarModifiers(node) || checkGrammarEnumDeclaration(node); | ||
checkGrammarDecorators(node) || checkGrammarModifiers(node); | ||
|
||
checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0); | ||
checkCollisionWithCapturedThisVariable(node, node.name); | ||
|
@@ -15619,40 +15634,6 @@ namespace ts { | |
return false; | ||
} | ||
|
||
function checkGrammarEnumDeclaration(enumDecl: EnumDeclaration): boolean { | ||
let enumIsConst = (enumDecl.flags & NodeFlags.Const) !== 0; | ||
|
||
let hasError = false; | ||
|
||
// skip checks below for const enums - they allow arbitrary initializers as long as they can be evaluated to constant expressions. | ||
// since all values are known in compile time - it is not necessary to check that constant enum section precedes computed enum members. | ||
if (!enumIsConst) { | ||
let inConstantEnumMemberSection = true; | ||
let inAmbientContext = isInAmbientContext(enumDecl); | ||
for (let node of enumDecl.members) { | ||
// Do not use hasDynamicName here, because that returns false for well known symbols. | ||
// We want to perform checkComputedPropertyName for all computed properties, including | ||
// well known symbols. | ||
if (node.name.kind === SyntaxKind.ComputedPropertyName) { | ||
hasError = grammarErrorOnNode(node.name, Diagnostics.Computed_property_names_are_not_allowed_in_enums); | ||
} | ||
else if (inAmbientContext) { | ||
if (node.initializer && !isIntegerLiteral(node.initializer)) { | ||
hasError = grammarErrorOnNode(node.name, Diagnostics.Ambient_enum_elements_can_only_have_integer_literal_initializers) || hasError; | ||
} | ||
} | ||
else if (node.initializer) { | ||
inConstantEnumMemberSection = isIntegerLiteral(node.initializer); | ||
} | ||
else if (!inConstantEnumMemberSection) { | ||
hasError = grammarErrorOnNode(node.name, Diagnostics.Enum_member_must_have_initializer) || hasError; | ||
} | ||
} | ||
} | ||
|
||
return hasError; | ||
} | ||
|
||
function hasParseDiagnostics(sourceFile: SourceFile): boolean { | ||
return sourceFile.parseDiagnostics.length > 0; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -195,7 +195,7 @@ | |
"category": "Error", | ||
"code": 1063 | ||
}, | ||
"Ambient enum elements can only have integer literal initializers.": { | ||
"In ambient enum declarations member initializer must be constant expression.": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An ambient enum member must be initialized with a constant expression. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Current message is based on the text that we show for const enums: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit. "Enum member initializer must be constant expression in an ambient enum declaration" the emphasis should be on the enum member initializer and not the ambient enum declaration. |
||
"category": "Error", | ||
"code": 1066 | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,14 @@ | ||
tests/cases/compiler/ambientEnum1.ts(2,9): error TS1066: Ambient enum elements can only have integer literal initializers. | ||
tests/cases/compiler/ambientEnum1.ts(7,9): error TS1066: Ambient enum elements can only have integer literal initializers. | ||
tests/cases/compiler/ambientEnum1.ts(7,13): error TS1066: In ambient enum declarations member initializer must be constant expression. | ||
|
||
|
||
==== tests/cases/compiler/ambientEnum1.ts (2 errors) ==== | ||
==== tests/cases/compiler/ambientEnum1.ts (1 errors) ==== | ||
declare enum E1 { | ||
y = 4.23 | ||
~ | ||
!!! error TS1066: Ambient enum elements can only have integer literal initializers. | ||
} | ||
|
||
// Ambient enum with computer member | ||
declare enum E2 { | ||
x = 'foo'.length | ||
~ | ||
!!! error TS1066: Ambient enum elements can only have integer literal initializers. | ||
~~~~~~~~~~~~ | ||
!!! error TS1066: In ambient enum declarations member initializer must be constant expression. | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
=== tests/cases/conformance/ambient/ambientEnumDeclaration1.ts === | ||
// In ambient enum declarations, all values specified in enum member declarations must be classified as constant enum expressions. | ||
|
||
declare enum E { | ||
>E : Symbol(E, Decl(ambientEnumDeclaration1.ts, 0, 0)) | ||
|
||
a = 10, | ||
>a : Symbol(E.a, Decl(ambientEnumDeclaration1.ts, 2, 16)) | ||
|
||
b = 10 + 1, | ||
>b : Symbol(E.b, Decl(ambientEnumDeclaration1.ts, 3, 11)) | ||
|
||
c = b, | ||
>c : Symbol(E.c, Decl(ambientEnumDeclaration1.ts, 4, 15)) | ||
>b : Symbol(E.b, Decl(ambientEnumDeclaration1.ts, 3, 11)) | ||
|
||
d = (c) + 1, | ||
>d : Symbol(E.d, Decl(ambientEnumDeclaration1.ts, 5, 10)) | ||
>c : Symbol(E.c, Decl(ambientEnumDeclaration1.ts, 4, 15)) | ||
|
||
e = 10 << 2 * 8, | ||
>e : Symbol(E.e, Decl(ambientEnumDeclaration1.ts, 6, 16)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
=== tests/cases/conformance/ambient/ambientEnumDeclaration1.ts === | ||
// In ambient enum declarations, all values specified in enum member declarations must be classified as constant enum expressions. | ||
|
||
declare enum E { | ||
>E : E | ||
|
||
a = 10, | ||
>a : E | ||
>10 : number | ||
|
||
b = 10 + 1, | ||
>b : E | ||
>10 + 1 : number | ||
>10 : number | ||
>1 : number | ||
|
||
c = b, | ||
>c : E | ||
>b : E | ||
|
||
d = (c) + 1, | ||
>d : E | ||
>(c) + 1 : number | ||
>(c) : E | ||
>c : E | ||
>1 : number | ||
|
||
e = 10 << 2 * 8, | ||
>e : E | ||
>10 << 2 * 8 : number | ||
>10 : number | ||
>2 * 8 : number | ||
>2 : number | ||
>8 : number | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
=== tests/cases/compiler/ambientEnumElementInitializer3.ts === | ||
declare enum E { | ||
>E : Symbol(E, Decl(ambientEnumElementInitializer3.ts, 0, 0)) | ||
|
||
e = 3.3 // Decimal | ||
>e : Symbol(E.e, Decl(ambientEnumElementInitializer3.ts, 0, 16)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
=== tests/cases/compiler/ambientEnumElementInitializer3.ts === | ||
declare enum E { | ||
>E : E | ||
|
||
e = 3.3 // Decimal | ||
>e : E | ||
>3.3 : number | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
=== tests/cases/conformance/enums/enumConstantMembers.ts === | ||
// Constant members allow negatives, but not decimals. Also hex literals are allowed | ||
enum E1 { | ||
>E1 : Symbol(E1, Decl(enumConstantMembers.ts, 0, 0)) | ||
|
||
a = 1, | ||
>a : Symbol(E1.a, Decl(enumConstantMembers.ts, 1, 9)) | ||
|
||
b | ||
>b : Symbol(E1.b, Decl(enumConstantMembers.ts, 2, 10)) | ||
} | ||
enum E2 { | ||
>E2 : Symbol(E2, Decl(enumConstantMembers.ts, 4, 1)) | ||
|
||
a = - 1, | ||
>a : Symbol(E2.a, Decl(enumConstantMembers.ts, 5, 9)) | ||
|
||
b | ||
>b : Symbol(E2.b, Decl(enumConstantMembers.ts, 6, 12)) | ||
} | ||
enum E3 { | ||
>E3 : Symbol(E3, Decl(enumConstantMembers.ts, 8, 1)) | ||
|
||
a = 0.1, | ||
>a : Symbol(E3.a, Decl(enumConstantMembers.ts, 9, 9)) | ||
|
||
b // Error because 0.1 is not a constant | ||
>b : Symbol(E3.b, Decl(enumConstantMembers.ts, 10, 12)) | ||
} | ||
|
||
declare enum E4 { | ||
>E4 : Symbol(E4, Decl(enumConstantMembers.ts, 12, 1)) | ||
|
||
a = 1, | ||
>a : Symbol(E4.a, Decl(enumConstantMembers.ts, 14, 17)) | ||
|
||
b = -1, | ||
>b : Symbol(E4.b, Decl(enumConstantMembers.ts, 15, 10)) | ||
|
||
c = 0.1 // Not a constant | ||
>c : Symbol(E4.c, Decl(enumConstantMembers.ts, 16, 11)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment 👍