From 64f17c5a9870fc452be98256402177a68105c30e Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 18 Nov 2016 07:48:12 -0800 Subject: [PATCH 1/3] Use a symbol for untyped modules to distinguish from unknownSymbol --- src/compiler/checker.ts | 16 ++++++----- src/compiler/utilities.ts | 4 +-- .../reference/extendsUntypedModule.errors.txt | 14 ++++++++++ .../reference/extendsUntypedModule.js | 27 +++++++++++++++++++ tests/cases/compiler/extendsUntypedModule.ts | 9 +++++++ tests/cases/fourslash/untypedModuleImport.ts | 2 +- 6 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 tests/baselines/reference/extendsUntypedModule.errors.txt create mode 100644 tests/baselines/reference/extendsUntypedModule.js create mode 100644 tests/cases/compiler/extendsUntypedModule.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3d25fc8bbbb0d..cf87755034ec5 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1099,7 +1099,7 @@ namespace ts { const moduleSymbol = resolveExternalModuleName(node, (node.parent).moduleSpecifier); if (moduleSymbol) { - const exportDefaultSymbol = isShorthandAmbientModuleSymbol(moduleSymbol) ? + const exportDefaultSymbol = isUntypedOrShorthandAmbientModuleSymbol(moduleSymbol) ? moduleSymbol : moduleSymbol.exports["export="] ? getPropertyOfType(getTypeOfSymbol(moduleSymbol.exports["export="]), "default") : @@ -1175,7 +1175,7 @@ namespace ts { if (targetSymbol) { const name = specifier.propertyName || specifier.name; if (name.text) { - if (isShorthandAmbientModuleSymbol(moduleSymbol)) { + if (isUntypedOrShorthandAmbientModuleSymbol(moduleSymbol)) { return moduleSymbol; } @@ -1427,15 +1427,19 @@ namespace ts { Debug.assert(!!moduleNotFoundError); const diag = Diagnostics.Invalid_module_name_in_augmentation_Module_0_resolves_to_an_untyped_module_at_1_which_cannot_be_augmented; error(errorNode, diag, moduleName, resolvedModule.resolvedFileName); + return undefined; } else if (compilerOptions.noImplicitAny && moduleNotFoundError) { error(errorNode, Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type, moduleReference, resolvedModule.resolvedFileName); + return undefined; } - // Failed imports and untyped modules are both treated in an untyped manner; only difference is whether we give a diagnostic first. - return undefined; + // Unlike a failed import, an untyped module produces a dummy symbol. This is checked for by `isUntypedOrShorthandAmbientModuleSymbol`. + const untypedSymbol = createSymbol(SymbolFlags.ValueModule, `"${moduleName}"`); + untypedSymbol.exports = createMap(); + return untypedSymbol; } if (moduleNotFoundError) { @@ -3575,7 +3579,7 @@ namespace ts { function getTypeOfFuncClassEnumModule(symbol: Symbol): Type { const links = getSymbolLinks(symbol); if (!links.type) { - if (symbol.flags & SymbolFlags.Module && isShorthandAmbientModuleSymbol(symbol)) { + if (symbol.flags & SymbolFlags.Module && isUntypedOrShorthandAmbientModuleSymbol(symbol)) { links.type = anyType; } else { @@ -19610,7 +19614,7 @@ namespace ts { function moduleExportsSomeValue(moduleReferenceExpression: Expression): boolean { let moduleSymbol = resolveExternalModuleName(moduleReferenceExpression.parent, moduleReferenceExpression); - if (!moduleSymbol || isShorthandAmbientModuleSymbol(moduleSymbol)) { + if (!moduleSymbol || isUntypedOrShorthandAmbientModuleSymbol(moduleSymbol)) { // If the module is not found or is shorthand, assume that it may export a value. return true; } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 546416884f1dc..c6dfcebdc54b7 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -388,8 +388,8 @@ namespace ts { } /** Given a symbol for a module, checks that it is either an untyped import or a shorthand ambient module. */ - export function isShorthandAmbientModuleSymbol(moduleSymbol: Symbol): boolean { - return isShorthandAmbientModule(moduleSymbol.valueDeclaration); + export function isUntypedOrShorthandAmbientModuleSymbol(moduleSymbol: Symbol): boolean { + return !moduleSymbol.valueDeclaration || isShorthandAmbientModule(moduleSymbol.valueDeclaration); } function isShorthandAmbientModule(node: Node): boolean { diff --git a/tests/baselines/reference/extendsUntypedModule.errors.txt b/tests/baselines/reference/extendsUntypedModule.errors.txt new file mode 100644 index 0000000000000..9a1c423535261 --- /dev/null +++ b/tests/baselines/reference/extendsUntypedModule.errors.txt @@ -0,0 +1,14 @@ +/a.ts(2,17): error TS2507: Type 'any' is not a constructor function type. + + +==== /a.ts (1 errors) ==== + import Foo from "foo"; + class A extends Foo { } + ~~~ +!!! error TS2507: Type 'any' is not a constructor function type. + +==== /node_modules/foo/index.js (0 errors) ==== + // Test that extending an untyped module is an error, unlike extending unknownSymbol. + + This file is not read. + \ No newline at end of file diff --git a/tests/baselines/reference/extendsUntypedModule.js b/tests/baselines/reference/extendsUntypedModule.js new file mode 100644 index 0000000000000..0804595c08e8d --- /dev/null +++ b/tests/baselines/reference/extendsUntypedModule.js @@ -0,0 +1,27 @@ +//// [tests/cases/compiler/extendsUntypedModule.ts] //// + +//// [index.js] +// Test that extending an untyped module is an error, unlike extending unknownSymbol. + +This file is not read. + +//// [a.ts] +import Foo from "foo"; +class A extends Foo { } + + +//// [a.js] +"use strict"; +var __extends = (this && this.__extends) || function (d, b) { + for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; + function __() { this.constructor = d; } + d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); +}; +var foo_1 = require("foo"); +var A = (function (_super) { + __extends(A, _super); + function A() { + return _super.apply(this, arguments) || this; + } + return A; +}(foo_1["default"])); diff --git a/tests/cases/compiler/extendsUntypedModule.ts b/tests/cases/compiler/extendsUntypedModule.ts new file mode 100644 index 0000000000000..8eaf6b3833a83 --- /dev/null +++ b/tests/cases/compiler/extendsUntypedModule.ts @@ -0,0 +1,9 @@ +// Test that extending an untyped module is an error, unlike extending unknownSymbol. +// @noImplicitReferences: true + +// @Filename: /node_modules/foo/index.js +This file is not read. + +// @Filename: /a.ts +import Foo from "foo"; +class A extends Foo { } diff --git a/tests/cases/fourslash/untypedModuleImport.ts b/tests/cases/fourslash/untypedModuleImport.ts index d128ad24ea3da..3fd209d480d31 100644 --- a/tests/cases/fourslash/untypedModuleImport.ts +++ b/tests/cases/fourslash/untypedModuleImport.ts @@ -13,7 +13,7 @@ verify.numberOfErrorsInCurrentFile(0); goTo.marker("fooModule"); verify.goToDefinitionIs([]); -verify.quickInfoIs(""); +verify.quickInfoIs('module "foo"'); verify.referencesAre([]) goTo.marker("foo"); From 4df4a2bbc7a1a71a440636f3bb18e5d9495569c8 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 25 Jan 2017 06:53:07 -0800 Subject: [PATCH 2/3] Use a single 'untypedModuleSymbol' for all untyped modules instead of creating a new one for each module. --- src/compiler/checker.ts | 10 ++++++---- .../baselines/reference/extendsUntypedModule.js | 17 +++++++++++------ tests/cases/fourslash/untypedModuleImport.ts | 2 +- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 332031903dba3..224da5e69c289 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -132,6 +132,8 @@ namespace ts { const evolvingArrayTypes: EvolvingArrayType[] = []; const unknownSymbol = createSymbol(SymbolFlags.Property | SymbolFlags.Transient, "unknown"); + const untypedModuleSymbol = createSymbol(SymbolFlags.ValueModule, ""); + untypedModuleSymbol.exports = createMap(); const resolvingSymbol = createSymbol(SymbolFlags.Transient, "__resolving__"); const anyType = createIntrinsicType(TypeFlags.Any, "any"); @@ -1490,10 +1492,10 @@ namespace ts { resolvedModule.resolvedFileName); return undefined; } - // Unlike a failed import, an untyped module produces a dummy symbol. This is checked for by `isUntypedOrShorthandAmbientModuleSymbol`. - const untypedSymbol = createSymbol(SymbolFlags.ValueModule, `"${moduleName}"`); - untypedSymbol.exports = createMap(); - return untypedSymbol; + // Unlike a failed import, an untyped module produces a dummy symbol. + // This is checked for by `isUntypedOrShorthandAmbientModuleSymbol`. + // This must be different than `unknownSymbol` because `getBaseConstructorTypeOfClass` won't fail for `unknownSymbol`. + return untypedModuleSymbol; } if (moduleNotFoundError) { diff --git a/tests/baselines/reference/extendsUntypedModule.js b/tests/baselines/reference/extendsUntypedModule.js index 0804595c08e8d..ad35e4abaac14 100644 --- a/tests/baselines/reference/extendsUntypedModule.js +++ b/tests/baselines/reference/extendsUntypedModule.js @@ -12,16 +12,21 @@ class A extends Foo { } //// [a.js] "use strict"; -var __extends = (this && this.__extends) || function (d, b) { - for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; - function __() { this.constructor = d; } - d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); -}; +var __extends = (this && this.__extends) || (function () { + var extendStatics = Object.setPrototypeOf || + ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) || + function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; }; + return function (d, b) { + extendStatics(d, b); + function __() { this.constructor = d; } + d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); + }; +})(); var foo_1 = require("foo"); var A = (function (_super) { __extends(A, _super); function A() { - return _super.apply(this, arguments) || this; + return _super !== null && _super.apply(this, arguments) || this; } return A; }(foo_1["default"])); diff --git a/tests/cases/fourslash/untypedModuleImport.ts b/tests/cases/fourslash/untypedModuleImport.ts index c09f710177b8a..276277102b0ca 100644 --- a/tests/cases/fourslash/untypedModuleImport.ts +++ b/tests/cases/fourslash/untypedModuleImport.ts @@ -12,7 +12,7 @@ verify.numberOfErrorsInCurrentFile(0); goTo.marker("fooModule"); verify.goToDefinitionIs([]); -verify.quickInfoIs('module "foo"'); +verify.quickInfoIs('module '); verify.referencesAre([]) goTo.marker("foo"); From de5091081a4768ff7684170d2ad8137cc02efaa0 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 14 Feb 2017 06:53:44 -0800 Subject: [PATCH 3/3] Fix failing tests --- src/compiler/utilities.ts | 2 +- tests/baselines/reference/extendsUntypedModule.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 6a338a0f9d2ed..f4c505b945031 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -426,7 +426,7 @@ namespace ts { /** Given a symbol for a module, checks that it is either an untyped import or a shorthand ambient module. */ export function isUntypedOrShorthandAmbientModuleSymbol(moduleSymbol: Symbol): boolean { - return !moduleSymbol.valueDeclaration || isShorthandAmbientModule(moduleSymbol.valueDeclaration); + return !moduleSymbol.declarations || isShorthandAmbientModule(moduleSymbol.valueDeclaration); } function isShorthandAmbientModule(node: Node): boolean { diff --git a/tests/baselines/reference/extendsUntypedModule.js b/tests/baselines/reference/extendsUntypedModule.js index ad35e4abaac14..f86ded7e6cb9f 100644 --- a/tests/baselines/reference/extendsUntypedModule.js +++ b/tests/baselines/reference/extendsUntypedModule.js @@ -22,6 +22,7 @@ var __extends = (this && this.__extends) || (function () { d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); }; })(); +exports.__esModule = true; var foo_1 = require("foo"); var A = (function (_super) { __extends(A, _super);