Skip to content

fix: const enums + isolatedModules emit invalid code #41933

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

Merged
merged 9 commits into from
Jan 13, 2021
2 changes: 1 addition & 1 deletion src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3435,7 +3435,7 @@ namespace ts {

function shouldReportErrorOnModuleDeclaration(node: ModuleDeclaration): boolean {
const instanceState = getModuleInstanceState(node);
return instanceState === ModuleInstanceState.Instantiated || (instanceState === ModuleInstanceState.ConstEnumOnly && !!options.preserveConstEnums);
return instanceState === ModuleInstanceState.Instantiated || (instanceState === ModuleInstanceState.ConstEnumOnly && shouldPreserveConstEnums(options));
}

function checkUnreachable(node: Node): boolean {
Expand Down
25 changes: 18 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2391,7 +2391,7 @@ namespace ts {
}
else {
Debug.assert(!!(result.flags & SymbolFlags.ConstEnum));
if (compilerOptions.preserveConstEnums) {
if (shouldPreserveConstEnums(compilerOptions)) {
diagnosticMessage = error(errorLocation, Diagnostics.Enum_0_used_before_its_declaration, declarationName);
}
}
Expand Down Expand Up @@ -22950,7 +22950,13 @@ namespace ts {
if (isNonLocalAlias(symbol, /*excludes*/ SymbolFlags.Value) && !isInTypeQuery(location) && !getTypeOnlyAliasDeclaration(symbol)) {
const target = resolveAlias(symbol);
if (target.flags & SymbolFlags.Value) {
if (compilerOptions.preserveConstEnums && isExportOrExportExpression(location) || !isConstEnumOrConstEnumOnlyModule(target)) {
// An alias resolving to a const enum cannot be elided if (1) 'isolatedModules' is enabled
// (because the const enum value will not be inlined), or if (2) the alias is an export
// of a const enum declaration that will be preserved.
if (compilerOptions.isolatedModules ||
shouldPreserveConstEnums(compilerOptions) && isExportOrExportExpression(location) ||
!isConstEnumOrConstEnumOnlyModule(target)
) {
markAliasSymbolAsReferenced(symbol);
}
else {
Expand Down Expand Up @@ -26009,7 +26015,11 @@ namespace ts {
}
prop = getPropertyOfType(apparentType, right.escapedText);
}
if (isIdentifier(left) && parentSymbol && !(prop && isConstEnumOrConstEnumOnlyModule(prop))) {
// In `Foo.Bar.Baz`, 'Foo' is not referenced if 'Bar' is a const enum or a module containing only const enums.
// The exceptions are:
// 1. if 'isolatedModules' is enabled, because the const enum value will not be inlined, and
// 2. if 'preserveConstEnums' is enabled and the expression is itself an export, e.g. `export = Foo.Bar.Baz`.
if (isIdentifier(left) && parentSymbol && (compilerOptions.isolatedModules || !(prop && isConstEnumOrConstEnumOnlyModule(prop)) || shouldPreserveConstEnums(compilerOptions) && isExportOrExportExpression(node))) {
markAliasReferenced(parentSymbol, node);
}

Expand Down Expand Up @@ -36330,7 +36340,7 @@ namespace ts {
if (symbol.flags & SymbolFlags.ValueModule
&& !inAmbientContext
&& symbol.declarations.length > 1
&& isInstantiatedModule(node, !!compilerOptions.preserveConstEnums || !!compilerOptions.isolatedModules)) {
&& isInstantiatedModule(node, shouldPreserveConstEnums(compilerOptions))) {
const firstNonAmbientClassOrFunc = getFirstNonAmbientClassOrFunctionDeclaration(symbol);
if (firstNonAmbientClassOrFunc) {
if (getSourceFileOfNode(node) !== getSourceFileOfNode(firstNonAmbientClassOrFunc)) {
Expand Down Expand Up @@ -38253,7 +38263,7 @@ namespace ts {
// const enums and modules that contain only const enums are not considered values from the emit perspective
// unless 'preserveConstEnums' option is set to true
return !!(target.flags & SymbolFlags.Value) &&
(compilerOptions.preserveConstEnums || !isConstEnumOrConstEnumOnlyModule(target));
(shouldPreserveConstEnums(compilerOptions) || !isConstEnumOrConstEnumOnlyModule(target));
}

function isConstEnumOrConstEnumOnlyModule(s: Symbol): boolean {
Expand All @@ -38263,13 +38273,14 @@ namespace ts {
function isReferencedAliasDeclaration(node: Node, checkChildren?: boolean): boolean {
if (isAliasSymbolDeclaration(node)) {
const symbol = getSymbolOfNode(node);
if (symbol && getSymbolLinks(symbol).referenced) {
const links = symbol && getSymbolLinks(symbol);
if (links?.referenced) {
return true;
}
const target = getSymbolLinks(symbol!).target; // TODO: GH#18217
if (target && getEffectiveModifierFlags(node) & ModifierFlags.Export &&
target.flags & SymbolFlags.Value &&
(compilerOptions.preserveConstEnums || !isConstEnumOrConstEnumOnlyModule(target))) {
(shouldPreserveConstEnums(compilerOptions) || !isConstEnumOrConstEnumOnlyModule(target))) {
// An `export import ... =` of a value symbol is always considered referenced
return true;
}
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3789,6 +3789,10 @@
"category": "Error",
"code": 5090
},
"Option 'preserveConstEnums' cannot be disabled when 'isolatedModules' is enabled.": {
"category": "Error",
"code": 5091
},

"Generates a sourcemap for each corresponding '.d.ts' file.": {
"category": "Message",
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3156,6 +3156,10 @@ namespace ts {
createDiagnosticForOptionName(Diagnostics.Option_isolatedModules_can_only_be_used_when_either_option_module_is_provided_or_option_target_is_ES2015_or_higher, "isolatedModules", "target");
}

if (options.preserveConstEnums === false) {
createDiagnosticForOptionName(Diagnostics.Option_preserveConstEnums_cannot_be_disabled_when_isolatedModules_is_enabled, "preserveConstEnums", "isolatedModules");
}

const firstNonExternalModuleSourceFile = find(files, f => !isExternalModule(f) && !isSourceFileJS(f) && !f.isDeclarationFile && f.scriptKind !== ScriptKind.JSON);
if (firstNonExternalModuleSourceFile) {
const span = getErrorSpanForNode(firstNonExternalModuleSourceFile, firstNonExternalModuleSourceFile);
Expand Down
5 changes: 2 additions & 3 deletions src/compiler/transformers/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2316,8 +2316,7 @@ namespace ts {
*/
function shouldEmitEnumDeclaration(node: EnumDeclaration) {
return !isEnumConst(node)
|| compilerOptions.preserveConstEnums
|| compilerOptions.isolatedModules;
|| shouldPreserveConstEnums(compilerOptions);
}

/**
Expand Down Expand Up @@ -2507,7 +2506,7 @@ namespace ts {
// If we can't find a parse tree node, assume the node is instantiated.
return true;
}
return isInstantiatedModule(node, !!compilerOptions.preserveConstEnums || !!compilerOptions.isolatedModules);
return isInstantiatedModule(node, shouldPreserveConstEnums(compilerOptions));
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6000,6 +6000,10 @@ namespace ts {
return !!(compilerOptions.declaration || compilerOptions.composite);
}

export function shouldPreserveConstEnums(compilerOptions: CompilerOptions): boolean {
return !!(compilerOptions.preserveConstEnums || compilerOptions.isolatedModules);
}

export function isIncrementalCompilation(options: CompilerOptions) {
return !!(options.incremental || options.composite);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//// [tests/cases/compiler/constEnumNamespaceReferenceCausesNoImport.ts] ////

//// [foo.ts]
export const enum ConstFooEnum {
Some,
Values,
Here
};
export function fooFunc(): void { /* removed */ }
//// [index.ts]
import * as Foo from "./foo";

function check(x: Foo.ConstFooEnum): void {
switch (x) {
case Foo.ConstFooEnum.Some:
break;
}
}

//// [foo.js]
"use strict";
exports.__esModule = true;
exports.fooFunc = void 0;
;
function fooFunc() { }
exports.fooFunc = fooFunc;
//// [index.js]
"use strict";
exports.__esModule = true;
function check(x) {
switch (x) {
case 0 /* Some */:
break;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
=== tests/cases/compiler/foo.ts ===
export const enum ConstFooEnum {
>ConstFooEnum : Symbol(ConstFooEnum, Decl(foo.ts, 0, 0))

Some,
>Some : Symbol(ConstFooEnum.Some, Decl(foo.ts, 0, 32))

Values,
>Values : Symbol(ConstFooEnum.Values, Decl(foo.ts, 1, 9))

Here
>Here : Symbol(ConstFooEnum.Here, Decl(foo.ts, 2, 11))

};
export function fooFunc(): void { /* removed */ }
>fooFunc : Symbol(fooFunc, Decl(foo.ts, 4, 2))

=== tests/cases/compiler/index.ts ===
import * as Foo from "./foo";
>Foo : Symbol(Foo, Decl(index.ts, 0, 6))

function check(x: Foo.ConstFooEnum): void {
>check : Symbol(check, Decl(index.ts, 0, 29))
>x : Symbol(x, Decl(index.ts, 2, 15))
>Foo : Symbol(Foo, Decl(index.ts, 0, 6))
>ConstFooEnum : Symbol(Foo.ConstFooEnum, Decl(foo.ts, 0, 0))

switch (x) {
>x : Symbol(x, Decl(index.ts, 2, 15))

case Foo.ConstFooEnum.Some:
>Foo.ConstFooEnum.Some : Symbol(Foo.ConstFooEnum.Some, Decl(foo.ts, 0, 32))
>Foo.ConstFooEnum : Symbol(Foo.ConstFooEnum, Decl(foo.ts, 0, 0))
>Foo : Symbol(Foo, Decl(index.ts, 0, 6))
>ConstFooEnum : Symbol(Foo.ConstFooEnum, Decl(foo.ts, 0, 0))
>Some : Symbol(Foo.ConstFooEnum.Some, Decl(foo.ts, 0, 32))

break;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
=== tests/cases/compiler/foo.ts ===
export const enum ConstFooEnum {
>ConstFooEnum : ConstFooEnum

Some,
>Some : ConstFooEnum.Some

Values,
>Values : ConstFooEnum.Values

Here
>Here : ConstFooEnum.Here

};
export function fooFunc(): void { /* removed */ }
>fooFunc : () => void

=== tests/cases/compiler/index.ts ===
import * as Foo from "./foo";
>Foo : typeof Foo

function check(x: Foo.ConstFooEnum): void {
>check : (x: Foo.ConstFooEnum) => void
>x : Foo.ConstFooEnum
>Foo : any

switch (x) {
>x : Foo.ConstFooEnum

case Foo.ConstFooEnum.Some:
>Foo.ConstFooEnum.Some : Foo.ConstFooEnum.Some
>Foo.ConstFooEnum : typeof Foo.ConstFooEnum
>Foo : typeof Foo
>ConstFooEnum : typeof Foo.ConstFooEnum
>Some : Foo.ConstFooEnum.Some

break;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//// [tests/cases/compiler/constEnumNamespaceReferenceCausesNoImport.ts] ////

//// [foo.ts]
export const enum ConstFooEnum {
Some,
Values,
Here
};
export function fooFunc(): void { /* removed */ }
//// [index.ts]
import * as Foo from "./foo";

function check(x: Foo.ConstFooEnum): void {
switch (x) {
case Foo.ConstFooEnum.Some:
break;
}
}

//// [foo.js]
"use strict";
exports.__esModule = true;
exports.fooFunc = exports.ConstFooEnum = void 0;
var ConstFooEnum;
(function (ConstFooEnum) {
ConstFooEnum[ConstFooEnum["Some"] = 0] = "Some";
ConstFooEnum[ConstFooEnum["Values"] = 1] = "Values";
ConstFooEnum[ConstFooEnum["Here"] = 2] = "Here";
})(ConstFooEnum = exports.ConstFooEnum || (exports.ConstFooEnum = {}));
;
function fooFunc() { }
exports.fooFunc = fooFunc;
//// [index.js]
"use strict";
exports.__esModule = true;
var Foo = require("./foo");
function check(x) {
switch (x) {
case Foo.ConstFooEnum.Some:
break;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
=== tests/cases/compiler/foo.ts ===
export const enum ConstFooEnum {
>ConstFooEnum : Symbol(ConstFooEnum, Decl(foo.ts, 0, 0))

Some,
>Some : Symbol(ConstFooEnum.Some, Decl(foo.ts, 0, 32))

Values,
>Values : Symbol(ConstFooEnum.Values, Decl(foo.ts, 1, 9))

Here
>Here : Symbol(ConstFooEnum.Here, Decl(foo.ts, 2, 11))

};
export function fooFunc(): void { /* removed */ }
>fooFunc : Symbol(fooFunc, Decl(foo.ts, 4, 2))

=== tests/cases/compiler/index.ts ===
import * as Foo from "./foo";
>Foo : Symbol(Foo, Decl(index.ts, 0, 6))

function check(x: Foo.ConstFooEnum): void {
>check : Symbol(check, Decl(index.ts, 0, 29))
>x : Symbol(x, Decl(index.ts, 2, 15))
>Foo : Symbol(Foo, Decl(index.ts, 0, 6))
>ConstFooEnum : Symbol(Foo.ConstFooEnum, Decl(foo.ts, 0, 0))

switch (x) {
>x : Symbol(x, Decl(index.ts, 2, 15))

case Foo.ConstFooEnum.Some:
>Foo.ConstFooEnum.Some : Symbol(Foo.ConstFooEnum.Some, Decl(foo.ts, 0, 32))
>Foo.ConstFooEnum : Symbol(Foo.ConstFooEnum, Decl(foo.ts, 0, 0))
>Foo : Symbol(Foo, Decl(index.ts, 0, 6))
>ConstFooEnum : Symbol(Foo.ConstFooEnum, Decl(foo.ts, 0, 0))
>Some : Symbol(Foo.ConstFooEnum.Some, Decl(foo.ts, 0, 32))

break;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
=== tests/cases/compiler/foo.ts ===
export const enum ConstFooEnum {
>ConstFooEnum : ConstFooEnum

Some,
>Some : ConstFooEnum.Some

Values,
>Values : ConstFooEnum.Values

Here
>Here : ConstFooEnum.Here

};
export function fooFunc(): void { /* removed */ }
>fooFunc : () => void

=== tests/cases/compiler/index.ts ===
import * as Foo from "./foo";
>Foo : typeof Foo

function check(x: Foo.ConstFooEnum): void {
>check : (x: Foo.ConstFooEnum) => void
>x : Foo.ConstFooEnum
>Foo : any

switch (x) {
>x : Foo.ConstFooEnum

case Foo.ConstFooEnum.Some:
>Foo.ConstFooEnum.Some : Foo.ConstFooEnum.Some
>Foo.ConstFooEnum : typeof Foo.ConstFooEnum
>Foo : typeof Foo
>ConstFooEnum : typeof Foo.ConstFooEnum
>Some : Foo.ConstFooEnum.Some

break;
}
}
Loading