maxmustermann/TypeScript
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Change isolatedModules to allow const enum declaration and disallow access

Browse source 
Fixes #20703 with solution suggested in https://github.com/Microsoft/TypeScript/issues/20703#issuecomment-361434795

Previously, `--isolatedModules` gave an error for any ambient const enum, which
meant that some third-party libraries would always give errors even if the
ambient const enums they declare were never used. Now, we only give an error
when an ambient const enum is referenced, which allows such libraries to still
be used as long as the const enums are never accessed.

Some nuances:
* As before, the error is only surfaced for *ambient* const enums. With
  non-ambient const enums, we know that an `isolatedModules` build will emit the
  enum and produce a plain reference rather than inlining the constant, so
  everything will still work.
* I originally planned to do this check in the code path that inlines the
  constant, but that code is only exercised at emit time, so, for example, the
  TS language service wasn't giving an error in my editor. Instead, I do the
  check at typecheck time next to another const-enum-related check.
* This can be a breaking change when using `skipLibCheck` because the error is
  typically moved from a .d.ts file to a .ts file.

Testing done:
I ran this TS build on a large project of mine that previously had disabled
`isolatedModules` so I could use the `chalk` library. With `isolatedModules`
enabled, there was no longer an error in the chalk typedefs, and a reference to
the `Level` const enum produced an error in my editor.
This commit is contained in:
Alan Pierce 2018-11-10 14:45:26 -08:00
parent a2205ad53d
commit 293eba6203
7 changed files with 48 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
src/compiler/checker.ts
View file

@ -22567,23 +22567,35 @@ namespace ts {
}
if (isConstEnumObjectType(type)) {
// enum object type for const enums are only permitted in:
// - 'left' in property access
// - 'object' in indexed access
// - target in rhs of import statement
const ok =
(node.parent.kind === SyntaxKind.PropertyAccessExpression && (<PropertyAccessExpression>node.parent).expression === node) ||
(node.parent.kind === SyntaxKind.ElementAccessExpression && (<ElementAccessExpression>node.parent).expression === node) ||
((node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.QualifiedName) && isInRightSideOfImportOrExportAssignment(<Identifier>node) ||
(node.parent.kind === SyntaxKind.TypeQuery && (<TypeQueryNode>node.parent).exprName === node));
if (!ok) {
error(node, Diagnostics.const_enums_can_only_be_used_in_property_or_index_access_expressions_or_the_right_hand_side_of_an_import_declaration_or_export_assignment_or_type_query);
}
checkConstEnumAccess(node, type);
}
return type;
}
function checkConstEnumAccess(node: Expression | QualifiedName, type: Type) {
// enum object type for const enums are only permitted in:
// - 'left' in property access
// - 'object' in indexed access
// - target in rhs of import statement
const ok =
(node.parent.kind === SyntaxKind.PropertyAccessExpression && (<PropertyAccessExpression>node.parent).expression === node) ||
(node.parent.kind === SyntaxKind.ElementAccessExpression && (<ElementAccessExpression>node.parent).expression === node) ||
((node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.QualifiedName) && isInRightSideOfImportOrExportAssignment(<Identifier>node) ||
(node.parent.kind === SyntaxKind.TypeQuery && (<TypeQueryNode>node.parent).exprName === node));
if (!ok) {
error(node, Diagnostics.const_enums_can_only_be_used_in_property_or_index_access_expressions_or_the_right_hand_side_of_an_import_declaration_or_export_assignment_or_type_query);
}
if (compilerOptions.isolatedModules) {
Debug.assert(!!(type.symbol.flags & SymbolFlags.ConstEnum));
const constEnumDeclaration = type.symbol.valueDeclaration as EnumDeclaration;
if (constEnumDeclaration.flags & NodeFlags.Ambient) {
error(node, Diagnostics.Cannot_access_ambient_const_enums_when_the_isolatedModules_flag_is_provided);
}
}
}
function checkParenthesizedExpression(node: ParenthesizedExpression, checkMode?: CheckMode): Type {
const tag = isInJSFile(node) ? getJSDocTypeTag(node) : undefined;
if (tag) {
@ -26721,11 +26733,6 @@ namespace ts {
computeEnumMemberValues(node);
const enumIsConst = isEnumConst(node);
if (compilerOptions.isolatedModules && enumIsConst && node.flags & NodeFlags.Ambient) {
error(node.name, Diagnostics.Ambient_const_enums_are_not_allowed_when_the_isolatedModules_flag_is_provided);
}
// Spec 2014 - Section 9.3:
// It isn't possible for one enum declaration to continue the automatic numbering sequence of another,
// and when an enum type has multiple declarations, only one declaration is permitted to omit a value
@ -26736,6 +26743,7 @@ namespace ts {
const firstDeclaration = getDeclarationOfKind(enumSymbol, node.kind);
if (node === firstDeclaration) {
if (enumSymbol.declarations.length > 1) {
const enumIsConst = isEnumConst(node);
// check that const is placed\omitted on all enum declarations
forEach(enumSymbol.declarations, decl => {
if (isEnumDeclaration(decl) && isEnumConst(decl) !== enumIsConst) {

8
src/compiler/diagnosticMessages.json
View file

@ -659,10 +659,6 @@
"category": "Error",
"code": 1208
},
"Ambient const enums are not allowed when the '--isolatedModules' flag is provided.": {
"category": "Error",
"code": 1209
},
"Invalid use of '{0}'. Class definitions are automatically in strict mode.": {
"category": "Error",
"code": 1210
@ -2513,6 +2509,10 @@
"category": "Message",
"code": 2738
},
"Cannot access ambient const enums when the '--isolatedModules' flag is provided.": {
"category": "Error",
"code": 2739
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",

9
tests/baselines/reference/isolatedModulesAmbientConstEnum.errors.txt
View file

@ -1,8 +1,9 @@
tests/cases/compiler/file1.ts(1,20): error TS1209: Ambient const enums are not allowed when the '--isolatedModules' flag is provided.
tests/cases/compiler/file1.ts(2,16): error TS2739: Cannot access ambient const enums when the '--isolatedModules' flag is provided.
==== tests/cases/compiler/file1.ts (1 errors) ====
declare const enum E { X = 1}
~
!!! error TS1209: Ambient const enums are not allowed when the '--isolatedModules' flag is provided.
export var y;
export var y = E.X;
~
!!! error TS2739: Cannot access ambient const enums when the '--isolatedModules' flag is provided.

5
tests/baselines/reference/isolatedModulesAmbientConstEnum.js
View file

@ -1,6 +1,7 @@
//// [file1.ts]
declare const enum E { X = 1}
export var y;
export var y = E.X;
//// [file1.js]
export var y;
export var y = E.X;

5
tests/baselines/reference/isolatedModulesAmbientConstEnum.symbols
View file

@ -3,6 +3,9 @@ declare const enum E { X = 1}
>E : Symbol(E, Decl(file1.ts, 0, 0))
>X : Symbol(E.X, Decl(file1.ts, 0, 22))
export var y;
export var y = E.X;
>y : Symbol(y, Decl(file1.ts, 1, 10))
>E.X : Symbol(E.X, Decl(file1.ts, 0, 22))
>E : Symbol(E, Decl(file1.ts, 0, 0))
>X : Symbol(E.X, Decl(file1.ts, 0, 22))

7
tests/baselines/reference/isolatedModulesAmbientConstEnum.types
View file

@ -4,6 +4,9 @@ declare const enum E { X = 1}
>X : E.X
>1 : 1
export var y;
>y : any
export var y = E.X;
>y : E
>E.X : E
>E : typeof E
>X : E

2
tests/cases/compiler/isolatedModulesAmbientConstEnum.ts
View file

@ -4,4 +4,4 @@
// @filename: file1.ts
declare const enum E { X = 1}
export var y;
export var y = E.X;