Forbid augmentation of untyped module (#11962)

* Forbid augmentation of untyped module

* Just use `undefined` for untyped modules

Author: Andy

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;
                 }
@@ -1074,7 +1074,7 @@ namespace ts {
             const moduleSymbol = resolveExternalModuleName(node, (<ImportDeclaration>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;
                     }
 
@@ -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);
@@ -1403,24 +1403,19 @@ namespace ts {
 
              // May be an untyped module. If so, ignore resolutionDiagnostic.
             if (!isRelative && resolvedModule && !extensionIsTypeScript(resolvedModule.extension)) {
-                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;
-                }
-
-                // 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;
+                if (isForAugmentation) {
+                    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 && 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) {
@@ -3490,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 {
@@ -19063,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;
             }

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

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 {

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.
+    

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;

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;

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");