From b1c02824c35f7081fefa9db1bbf14c406083a6d7 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 31 Oct 2016 08:45:50 -0700 Subject: [PATCH 1/2] Forbid augmentation of untyped module --- src/compiler/checker.ts | 35 +++++++++++-------- src/compiler/diagnosticMessages.json | 4 +++ ...edModuleImport_withAugmentation.errors.txt | 17 +++++++++ .../untypedModuleImport_withAugmentation.js | 19 ++++++++++ .../untypedModuleImport_withAugmentation.ts | 13 +++++++ 5 files changed, 73 insertions(+), 15 deletions(-) create mode 100644 tests/baselines/reference/untypedModuleImport_withAugmentation.errors.txt create mode 100644 tests/baselines/reference/untypedModuleImport_withAugmentation.js create mode 100644 tests/cases/conformance/moduleResolution/untypedModuleImport_withAugmentation.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 704018627b24e..96efd4c0b9ce0 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -498,7 +498,7 @@ namespace ts { const moduleNotFoundError = !isInAmbientContext(moduleName.parent.parent) ? Diagnostics.Invalid_module_name_in_augmentation_module_0_cannot_be_found : undefined; - let mainModule = resolveExternalModuleNameWorker(moduleName, moduleName, moduleNotFoundError); + let mainModule = resolveExternalModuleNameWorker(moduleName, moduleName, moduleNotFoundError, /*isForAugmentation*/ true); if (!mainModule) { return; } @@ -1351,16 +1351,16 @@ namespace ts { return resolveExternalModuleNameWorker(location, moduleReferenceExpression, Diagnostics.Cannot_find_module_0); } - function resolveExternalModuleNameWorker(location: Node, moduleReferenceExpression: Expression, moduleNotFoundError: DiagnosticMessage): Symbol { + function resolveExternalModuleNameWorker(location: Node, moduleReferenceExpression: Expression, moduleNotFoundError: DiagnosticMessage, isForAugmentation = false): Symbol { if (moduleReferenceExpression.kind !== SyntaxKind.StringLiteral) { return; } const moduleReferenceLiteral = moduleReferenceExpression; - return resolveExternalModule(location, moduleReferenceLiteral.text, moduleNotFoundError, moduleReferenceLiteral); + return resolveExternalModule(location, moduleReferenceLiteral.text, moduleNotFoundError, moduleReferenceLiteral, isForAugmentation); } - function resolveExternalModule(location: Node, moduleReference: string, moduleNotFoundError: DiagnosticMessage, errorNode: Node): Symbol { + function resolveExternalModule(location: Node, moduleReference: string, moduleNotFoundError: DiagnosticMessage, errorNode: Node, isForAugmentation = false): Symbol { // Module names are escaped in our symbol table. However, string literal values aren't. // Escape the name in the "require(...)" clause to ensure we find the right symbol. const moduleName = escapeIdentifier(moduleReference); @@ -1380,7 +1380,7 @@ namespace ts { } const resolvedModule = getResolvedModule(getSourceFileOfNode(location), moduleReference); - const resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule); + let resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule); const sourceFile = resolvedModule && !resolutionDiagnostic && host.getSourceFile(resolvedModule.resolvedFileName); if (sourceFile) { if (sourceFile.symbol) { @@ -1403,7 +1403,10 @@ namespace ts { // May be an untyped module. If so, ignore resolutionDiagnostic. if (!isRelative && resolvedModule && !extensionIsTypeScript(resolvedModule.extension)) { - if (compilerOptions.noImplicitAny) { + if (isForAugmentation) { + resolutionDiagnostic = Diagnostics.Invalid_module_name_in_augmentation_module_0_resolves_to_an_untyped_module_at_1_which_cannot_be_augmented; + } + else if (compilerOptions.noImplicitAny) { if (moduleNotFoundError) { error(errorNode, Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type, @@ -1412,15 +1415,17 @@ namespace ts { } return undefined; } - - // Create a new symbol to represent the untyped module and store it in globals. - // This provides a name to the module. See the test tests/cases/fourslash/untypedModuleImport.ts - const newSymbol = createSymbol(SymbolFlags.ValueModule, quotedName); - // Module symbols are expected to have 'exports', although since this is an untyped module it can be empty. - newSymbol.exports = createMap(); - // Cache it so subsequent accesses will return the same module. - globals[quotedName] = newSymbol; - return newSymbol; + else { + // Create a new symbol to represent the untyped module and store it in globals. + // This provides a name to the module. See the test tests/cases/fourslash/untypedModuleImport.ts + const newSymbol = createSymbol(SymbolFlags.ValueModule, quotedName); + //newSymbol.declarations = []; //want this? + // Module symbols are expected to have 'exports', although since this is an untyped module it can be empty. + newSymbol.exports = createMap(); + // Cache it so subsequent accesses will return the same module. + globals[quotedName] = newSymbol; + return newSymbol; + } } if (moduleNotFoundError) { diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index e2a2a4b9a0cf4..ef3de5b353cc7 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -1839,6 +1839,10 @@ "category": "Error", "code": 2664 }, + "Invalid module name in augmentation. Module '{0}' resolves to an untyped module at '{1}', which cannot be augmented.": { + "category": "Error", + "code": 2665 + }, "Exports and export assignments are not permitted in module augmentations.": { "category": "Error", "code": 2666 diff --git a/tests/baselines/reference/untypedModuleImport_withAugmentation.errors.txt b/tests/baselines/reference/untypedModuleImport_withAugmentation.errors.txt new file mode 100644 index 0000000000000..a53c7c757675e --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_withAugmentation.errors.txt @@ -0,0 +1,17 @@ +/a.ts(1,16): error TS2665: Invalid module name in augmentation, module 'foo' resolves to an untyped module at '/node_modules/foo/index.js', which cannot be augmented. + + +==== /a.ts (1 errors) ==== + declare module "foo" { + ~~~~~ +!!! error TS2665: Invalid module name in augmentation, module 'foo' resolves to an untyped module at '/node_modules/foo/index.js', which cannot be augmented. + export const x: number; + } + import { x } from "foo"; + x; + +==== /node_modules/foo/index.js (0 errors) ==== + // This tests that augmenting an untyped module is forbidden. + + This file is not processed. + \ No newline at end of file diff --git a/tests/baselines/reference/untypedModuleImport_withAugmentation.js b/tests/baselines/reference/untypedModuleImport_withAugmentation.js new file mode 100644 index 0000000000000..17eda95aa8af7 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_withAugmentation.js @@ -0,0 +1,19 @@ +//// [tests/cases/conformance/moduleResolution/untypedModuleImport_withAugmentation.ts] //// + +//// [index.js] +// This tests that augmenting an untyped module is forbidden. + +This file is not processed. + +//// [a.ts] +declare module "foo" { + export const x: number; +} +import { x } from "foo"; +x; + + +//// [a.js] +"use strict"; +var foo_1 = require("foo"); +foo_1.x; diff --git a/tests/cases/conformance/moduleResolution/untypedModuleImport_withAugmentation.ts b/tests/cases/conformance/moduleResolution/untypedModuleImport_withAugmentation.ts new file mode 100644 index 0000000000000..e27c1232d2587 --- /dev/null +++ b/tests/cases/conformance/moduleResolution/untypedModuleImport_withAugmentation.ts @@ -0,0 +1,13 @@ +// @noImplicitReferences: true +// @currentDirectory: / +// This tests that augmenting an untyped module is forbidden. + +// @filename: /node_modules/foo/index.js +This file is not processed. + +// @filename: /a.ts +declare module "foo" { + export const x: number; +} +import { x } from "foo"; +x; From f792258eebbcd08cddda03158155605a943435e4 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 31 Oct 2016 13:44:08 -0700 Subject: [PATCH 2/2] Just use `undefined` for untyped modules --- src/compiler/checker.ts | 40 +++++++------------ src/compiler/utilities.ts | 4 +- ...edModuleImport_withAugmentation.errors.txt | 4 +- tests/cases/fourslash/untypedModuleImport.ts | 2 +- 4 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 96efd4c0b9ce0..37ace5741a174 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1074,7 +1074,7 @@ namespace ts { const moduleSymbol = resolveExternalModuleName(node, (node.parent).moduleSpecifier); if (moduleSymbol) { - const exportDefaultSymbol = isUntypedModuleSymbol(moduleSymbol) ? + const exportDefaultSymbol = isShorthandAmbientModuleSymbol(moduleSymbol) ? moduleSymbol : moduleSymbol.exports["export="] ? getPropertyOfType(getTypeOfSymbol(moduleSymbol.exports["export="]), "default") : @@ -1150,7 +1150,7 @@ namespace ts { if (targetSymbol) { const name = specifier.propertyName || specifier.name; if (name.text) { - if (isUntypedModuleSymbol(moduleSymbol)) { + if (isShorthandAmbientModuleSymbol(moduleSymbol)) { return moduleSymbol; } @@ -1380,7 +1380,7 @@ namespace ts { } const resolvedModule = getResolvedModule(getSourceFileOfNode(location), moduleReference); - let resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule); + const resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule); const sourceFile = resolvedModule && !resolutionDiagnostic && host.getSourceFile(resolvedModule.resolvedFileName); if (sourceFile) { if (sourceFile.symbol) { @@ -1404,28 +1404,18 @@ namespace ts { // May be an untyped module. If so, ignore resolutionDiagnostic. if (!isRelative && resolvedModule && !extensionIsTypeScript(resolvedModule.extension)) { if (isForAugmentation) { - resolutionDiagnostic = Diagnostics.Invalid_module_name_in_augmentation_module_0_resolves_to_an_untyped_module_at_1_which_cannot_be_augmented; + 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); } - else if (compilerOptions.noImplicitAny) { - if (moduleNotFoundError) { - error(errorNode, - Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type, - moduleReference, - resolvedModule.resolvedFileName); - } - return undefined; - } - else { - // Create a new symbol to represent the untyped module and store it in globals. - // This provides a name to the module. See the test tests/cases/fourslash/untypedModuleImport.ts - const newSymbol = createSymbol(SymbolFlags.ValueModule, quotedName); - //newSymbol.declarations = []; //want this? - // Module symbols are expected to have 'exports', although since this is an untyped module it can be empty. - newSymbol.exports = createMap(); - // Cache it so subsequent accesses will return the same module. - globals[quotedName] = newSymbol; - return newSymbol; + 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); } + // Failed imports and untyped modules are both treated in an untyped manner; only difference is whether we give a diagnostic first. + return undefined; } if (moduleNotFoundError) { @@ -3495,7 +3485,7 @@ namespace ts { function getTypeOfFuncClassEnumModule(symbol: Symbol): Type { const links = getSymbolLinks(symbol); if (!links.type) { - if (symbol.flags & SymbolFlags.Module && isUntypedModuleSymbol(symbol)) { + if (symbol.flags & SymbolFlags.Module && isShorthandAmbientModuleSymbol(symbol)) { links.type = anyType; } else { @@ -19068,7 +19058,7 @@ namespace ts { function moduleExportsSomeValue(moduleReferenceExpression: Expression): boolean { let moduleSymbol = resolveExternalModuleName(moduleReferenceExpression.parent, moduleReferenceExpression); - if (!moduleSymbol || isUntypedModuleSymbol(moduleSymbol)) { + if (!moduleSymbol || isShorthandAmbientModuleSymbol(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 f86a87e19eef4..9274d20f76e84 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -403,8 +403,8 @@ namespace ts { } /** Given a symbol for a module, checks that it is either an untyped import or a shorthand ambient module. */ - export function isUntypedModuleSymbol(moduleSymbol: Symbol): boolean { - return !moduleSymbol.valueDeclaration || isShorthandAmbientModule(moduleSymbol.valueDeclaration); + export function isShorthandAmbientModuleSymbol(moduleSymbol: Symbol): boolean { + return isShorthandAmbientModule(moduleSymbol.valueDeclaration); } function isShorthandAmbientModule(node: Node): boolean { diff --git a/tests/baselines/reference/untypedModuleImport_withAugmentation.errors.txt b/tests/baselines/reference/untypedModuleImport_withAugmentation.errors.txt index a53c7c757675e..290cfbc6cbd85 100644 --- a/tests/baselines/reference/untypedModuleImport_withAugmentation.errors.txt +++ b/tests/baselines/reference/untypedModuleImport_withAugmentation.errors.txt @@ -1,10 +1,10 @@ -/a.ts(1,16): error TS2665: Invalid module name in augmentation, module 'foo' resolves to an untyped module at '/node_modules/foo/index.js', which cannot be augmented. +/a.ts(1,16): error TS2665: Invalid module name in augmentation. Module 'foo' resolves to an untyped module at '/node_modules/foo/index.js', which cannot be augmented. ==== /a.ts (1 errors) ==== declare module "foo" { ~~~~~ -!!! error TS2665: Invalid module name in augmentation, module 'foo' resolves to an untyped module at '/node_modules/foo/index.js', which cannot be augmented. +!!! error TS2665: Invalid module name in augmentation. Module 'foo' resolves to an untyped module at '/node_modules/foo/index.js', which cannot be augmented. export const x: number; } import { x } from "foo"; diff --git a/tests/cases/fourslash/untypedModuleImport.ts b/tests/cases/fourslash/untypedModuleImport.ts index 3fd209d480d31..d128ad24ea3da 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('module "foo"'); +verify.quickInfoIs(""); verify.referencesAre([]) goTo.marker("foo");