Skip to content
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

Forbid augmentation of untyped module #11962

Merged
merged 2 commits into from
Oct 31, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not convinced why do we need to create a new symbol here an stick it into globals? AFAIR returning undefined will be should be properly interpreted downstream and converted into unknownSymbol

//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;