Skip to content

Commit

Permalink
Forbid augmentation of untyped module
Browse files Browse the repository at this point in the history
  • Loading branch information
Andy Hanson committed Oct 31, 2016
1 parent 44ce59d commit b1c0282
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 15 deletions.
35 changes: 20 additions & 15 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 = <LiteralExpression>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);
Expand All @@ -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) {
Expand All @@ -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,
Expand All @@ -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<Symbol>();
// 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<Symbol>();
// Cache it so subsequent accesses will return the same module.
globals[quotedName] = newSymbol;
return newSymbol;
}
}

if (moduleNotFoundError) {
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

19 changes: 19 additions & 0 deletions tests/baselines/reference/untypedModuleImport_withAugmentation.js
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;

0 comments on commit b1c0282

Please sign in to comment.