Merge pull request #31078 from andrewbranch/bug/30752

andrewbranch · web-flow · commit 90d3acf6c775 · 2019-04-30T06:18:23.000-10:00
Fix symbol merging of augmentations to pattern ambient modules
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -498,6 +498,7 @@ namespace ts {
          * This is only used if there is no exact match.
          */
         let patternAmbientModules: PatternAmbientModule[];
+        let patternAmbientModuleAugmentations: Map<Symbol> | undefined;
 
         let globalObjectType: ObjectType;
         let globalFunctionType: ObjectType;
@@ -888,7 +889,7 @@ namespace ts {
          * Note: if target is transient, then it is mutable, and mergeSymbol with both mutate and return it.
          * If target is not transient, mergeSymbol will produce a transient clone, mutate that and return it.
          */
-        function mergeSymbol(target: Symbol, source: Symbol): Symbol {
+        function mergeSymbol(target: Symbol, source: Symbol, unidirectional = false): Symbol {
             if (!(target.flags & getExcludedSymbolFlags(source.flags)) ||
                 (source.flags | target.flags) & SymbolFlags.Assignment) {
                 Debug.assert(source !== target);
@@ -915,13 +916,16 @@ namespace ts {
                 addRange(target.declarations, source.declarations);
                 if (source.members) {
                     if (!target.members) target.members = createSymbolTable();
-                    mergeSymbolTable(target.members, source.members);
+                    mergeSymbolTable(target.members, source.members, unidirectional);
                 }
                 if (source.exports) {
                     if (!target.exports) target.exports = createSymbolTable();
-                    mergeSymbolTable(target.exports, source.exports);
+                    mergeSymbolTable(target.exports, source.exports, unidirectional
+                        );
+                }
+                if (!unidirectional) {
+                    recordMergedSymbol(target, source);
                 }
-                recordMergedSymbol(target, source);
             }
             else if (target.flags & SymbolFlags.NamespaceModule) {
                 // Do not report an error when merging `var globalThis` with the built-in `globalThis`,
@@ -993,10 +997,10 @@ namespace ts {
             return combined;
         }
 
-        function mergeSymbolTable(target: SymbolTable, source: SymbolTable) {
+        function mergeSymbolTable(target: SymbolTable, source: SymbolTable, unidirectional = false) {
             source.forEach((sourceSymbol, id) => {
                 const targetSymbol = target.get(id);
-                target.set(id, targetSymbol ? mergeSymbol(targetSymbol, sourceSymbol) : sourceSymbol);
+                target.set(id, targetSymbol ? mergeSymbol(targetSymbol, sourceSymbol, unidirectional) : sourceSymbol);
             });
         }
 
@@ -1026,7 +1030,22 @@ namespace ts {
                 // obtain item referenced by 'export='
                 mainModule = resolveExternalModuleSymbol(mainModule);
                 if (mainModule.flags & SymbolFlags.Namespace) {
-                    mainModule = mergeSymbol(mainModule, moduleAugmentation.symbol);
+                    // If we’re merging an augmentation to a pattern ambient module, we want to
+                    // perform the merge unidirectionally from the augmentation ('a.foo') to
+                    // the pattern ('*.foo'), so that 'getMergedSymbol()' on a.foo gives you
+                    // all the exports both from the pattern and from the augmentation, but
+                    // 'getMergedSymbol()' on *.foo only gives you exports from *.foo.
+                    if (some(patternAmbientModules, module => mainModule === module.symbol)) {
+                        const merged = mergeSymbol(moduleAugmentation.symbol, mainModule, /*unidirectional*/ true);
+                        if (!patternAmbientModuleAugmentations) {
+                            patternAmbientModuleAugmentations = createMap();
+                        }
+                        // moduleName will be a StringLiteral since this is not `declare global`.
+                        patternAmbientModuleAugmentations.set((moduleName as StringLiteral).text, merged);
+                    }
+                    else {
+                        mergeSymbol(mainModule, moduleAugmentation.symbol);
+                    }
                 }
                 else {
                     // moduleName will be a StringLiteral since this is not `declare global`.
@@ -2455,6 +2474,14 @@ namespace ts {
             if (patternAmbientModules) {
                 const pattern = findBestPatternMatch(patternAmbientModules, _ => _.pattern, moduleReference);
                 if (pattern) {
+                    // If the module reference matched a pattern ambient module ('*.foo') but there’s also a
+                    // module augmentation by the specific name requested ('a.foo'), we store the merged symbol
+                    // by the augmentation name ('a.foo'), because asking for *.foo should not give you exports
+                    // from a.foo.
+                    const augmentation = patternAmbientModuleAugmentations && patternAmbientModuleAugmentations.get(moduleReference);
+                    if (augmentation) {
+                        return getMergedSymbol(augmentation);
+                    }
                     return getMergedSymbol(pattern.symbol);
                 }
             }
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging1.errors.txt b/tests/baselines/reference/ambientDeclarationsPatterns_merging1.errors.txt
@@ -0,0 +1,20 @@
+tests/cases/conformance/ambient/testB.ts(1,22): error TS2305: Module '"*.foo"' has no exported member 'onlyInA'.
+
+
+==== tests/cases/conformance/ambient/types.ts (0 errors) ====
+    declare module "*.foo" {
+      let everywhere: string;
+    }
+    
+    
+==== tests/cases/conformance/ambient/testA.ts (0 errors) ====
+    import { everywhere, onlyInA } from "a.foo";
+    declare module "a.foo" {
+      let onlyInA: number;
+    }
+    
+==== tests/cases/conformance/ambient/testB.ts (1 errors) ====
+    import { everywhere, onlyInA } from "b.foo"; // Error
+                         ~~~~~~~
+!!! error TS2305: Module '"*.foo"' has no exported member 'onlyInA'.
+    
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging1.js b/tests/baselines/reference/ambientDeclarationsPatterns_merging1.js
@@ -0,0 +1,25 @@
+//// [tests/cases/conformance/ambient/ambientDeclarationsPatterns_merging1.ts] ////
+
+//// [types.ts]
+declare module "*.foo" {
+  let everywhere: string;
+}
+
+
+//// [testA.ts]
+import { everywhere, onlyInA } from "a.foo";
+declare module "a.foo" {
+  let onlyInA: number;
+}
+
+//// [testB.ts]
+import { everywhere, onlyInA } from "b.foo"; // Error
+
+
+//// [types.js]
+//// [testA.js]
+"use strict";
+exports.__esModule = true;
+//// [testB.js]
+"use strict";
+exports.__esModule = true;
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging1.symbols b/tests/baselines/reference/ambientDeclarationsPatterns_merging1.symbols
@@ -0,0 +1,26 @@
+=== tests/cases/conformance/ambient/types.ts ===
+declare module "*.foo" {
+>"*.foo" : Symbol("*.foo", Decl(types.ts, 0, 0))
+
+  let everywhere: string;
+>everywhere : Symbol(everywhere, Decl(types.ts, 1, 5))
+}
+
+
+=== tests/cases/conformance/ambient/testA.ts ===
+import { everywhere, onlyInA } from "a.foo";
+>everywhere : Symbol(everywhere, Decl(testA.ts, 0, 8))
+>onlyInA : Symbol(onlyInA, Decl(testA.ts, 0, 20))
+
+declare module "a.foo" {
+>"a.foo" : Symbol("a.foo", Decl(testA.ts, 0, 44), Decl(types.ts, 0, 0))
+
+  let onlyInA: number;
+>onlyInA : Symbol(onlyInA, Decl(testA.ts, 2, 5))
+}
+
+=== tests/cases/conformance/ambient/testB.ts ===
+import { everywhere, onlyInA } from "b.foo"; // Error
+>everywhere : Symbol(everywhere, Decl(testB.ts, 0, 8))
+>onlyInA : Symbol(onlyInA, Decl(testB.ts, 0, 20))
+
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging1.types b/tests/baselines/reference/ambientDeclarationsPatterns_merging1.types
@@ -0,0 +1,26 @@
+=== tests/cases/conformance/ambient/types.ts ===
+declare module "*.foo" {
+>"*.foo" : typeof import("*.foo")
+
+  let everywhere: string;
+>everywhere : string
+}
+
+
+=== tests/cases/conformance/ambient/testA.ts ===
+import { everywhere, onlyInA } from "a.foo";
+>everywhere : string
+>onlyInA : number
+
+declare module "a.foo" {
+>"a.foo" : typeof import("a.foo")
+
+  let onlyInA: number;
+>onlyInA : number
+}
+
+=== tests/cases/conformance/ambient/testB.ts ===
+import { everywhere, onlyInA } from "b.foo"; // Error
+>everywhere : string
+>onlyInA : any
+
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging2.errors.txt b/tests/baselines/reference/ambientDeclarationsPatterns_merging2.errors.txt
@@ -0,0 +1,25 @@
+tests/cases/conformance/ambient/testB.ts(1,22): error TS2305: Module '"*.foo"' has no exported member 'onlyInA'.
+tests/cases/conformance/ambient/testB.ts(1,31): error TS2305: Module '"*.foo"' has no exported member 'alsoOnlyInA'.
+
+
+==== tests/cases/conformance/ambient/types.ts (0 errors) ====
+    declare module "*.foo" {
+      let everywhere: string;
+    }
+    
+    
+==== tests/cases/conformance/ambient/testA.ts (0 errors) ====
+    import { everywhere, onlyInA, alsoOnlyInA } from "a.foo";
+    declare module "a.foo" {
+      let onlyInA: number;
+    }
+    
+==== tests/cases/conformance/ambient/testB.ts (2 errors) ====
+    import { everywhere, onlyInA, alsoOnlyInA } from "b.foo"; // Error
+                         ~~~~~~~
+!!! error TS2305: Module '"*.foo"' has no exported member 'onlyInA'.
+                                  ~~~~~~~~~~~
+!!! error TS2305: Module '"*.foo"' has no exported member 'alsoOnlyInA'.
+    declare module "a.foo" {
+      let alsoOnlyInA: number;
+    }
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging2.js b/tests/baselines/reference/ambientDeclarationsPatterns_merging2.js
@@ -0,0 +1,27 @@
+//// [tests/cases/conformance/ambient/ambientDeclarationsPatterns_merging2.ts] ////
+
+//// [types.ts]
+declare module "*.foo" {
+  let everywhere: string;
+}
+
+
+//// [testA.ts]
+import { everywhere, onlyInA, alsoOnlyInA } from "a.foo";
+declare module "a.foo" {
+  let onlyInA: number;
+}
+
+//// [testB.ts]
+import { everywhere, onlyInA, alsoOnlyInA } from "b.foo"; // Error
+declare module "a.foo" {
+  let alsoOnlyInA: number;
+}
+
+//// [types.js]
+//// [testA.js]
+"use strict";
+exports.__esModule = true;
+//// [testB.js]
+"use strict";
+exports.__esModule = true;
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging2.symbols b/tests/baselines/reference/ambientDeclarationsPatterns_merging2.symbols
@@ -0,0 +1,34 @@
+=== tests/cases/conformance/ambient/types.ts ===
+declare module "*.foo" {
+>"*.foo" : Symbol("*.foo", Decl(types.ts, 0, 0))
+
+  let everywhere: string;
+>everywhere : Symbol(everywhere, Decl(types.ts, 1, 5))
+}
+
+
+=== tests/cases/conformance/ambient/testA.ts ===
+import { everywhere, onlyInA, alsoOnlyInA } from "a.foo";
+>everywhere : Symbol(everywhere, Decl(testA.ts, 0, 8))
+>onlyInA : Symbol(onlyInA, Decl(testA.ts, 0, 20))
+>alsoOnlyInA : Symbol(alsoOnlyInA, Decl(testA.ts, 0, 29))
+
+declare module "a.foo" {
+>"a.foo" : Symbol("a.foo", Decl(testA.ts, 0, 57), Decl(types.ts, 0, 0), Decl(testB.ts, 0, 57))
+
+  let onlyInA: number;
+>onlyInA : Symbol(onlyInA, Decl(testA.ts, 2, 5))
+}
+
+=== tests/cases/conformance/ambient/testB.ts ===
+import { everywhere, onlyInA, alsoOnlyInA } from "b.foo"; // Error
+>everywhere : Symbol(everywhere, Decl(testB.ts, 0, 8))
+>onlyInA : Symbol(onlyInA, Decl(testB.ts, 0, 20))
+>alsoOnlyInA : Symbol(alsoOnlyInA, Decl(testB.ts, 0, 29))
+
+declare module "a.foo" {
+>"a.foo" : Symbol("a.foo", Decl(testA.ts, 0, 57), Decl(types.ts, 0, 0), Decl(testB.ts, 0, 57))
+
+  let alsoOnlyInA: number;
+>alsoOnlyInA : Symbol(alsoOnlyInA, Decl(testB.ts, 2, 5))
+}
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging2.types b/tests/baselines/reference/ambientDeclarationsPatterns_merging2.types
@@ -0,0 +1,34 @@
+=== tests/cases/conformance/ambient/types.ts ===
+declare module "*.foo" {
+>"*.foo" : typeof import("*.foo")
+
+  let everywhere: string;
+>everywhere : string
+}
+
+
+=== tests/cases/conformance/ambient/testA.ts ===
+import { everywhere, onlyInA, alsoOnlyInA } from "a.foo";
+>everywhere : string
+>onlyInA : number
+>alsoOnlyInA : number
+
+declare module "a.foo" {
+>"a.foo" : typeof import("a.foo")
+
+  let onlyInA: number;
+>onlyInA : number
+}
+
+=== tests/cases/conformance/ambient/testB.ts ===
+import { everywhere, onlyInA, alsoOnlyInA } from "b.foo"; // Error
+>everywhere : string
+>onlyInA : any
+>alsoOnlyInA : any
+
+declare module "a.foo" {
+>"a.foo" : typeof import("a.foo")
+
+  let alsoOnlyInA: number;
+>alsoOnlyInA : number
+}
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging3.errors.txt b/tests/baselines/reference/ambientDeclarationsPatterns_merging3.errors.txt
@@ -0,0 +1,18 @@
+tests/cases/conformance/ambient/test.ts(6,6): error TS2339: Property 'a' does not exist on type 'OhNo'.
+
+
+==== tests/cases/conformance/ambient/types.ts (0 errors) ====
+    declare module "*.foo" {
+      export interface OhNo { star: string }
+    }
+    
+==== tests/cases/conformance/ambient/test.ts (1 errors) ====
+    declare module "a.foo" {
+      export interface OhNo { a: string }
+    }
+    import { OhNo } from "b.foo"
+    declare let ohno: OhNo;
+    ohno.a // oh no
+         ~
+!!! error TS2339: Property 'a' does not exist on type 'OhNo'.
+    
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging3.js b/tests/baselines/reference/ambientDeclarationsPatterns_merging3.js
@@ -0,0 +1,21 @@
+//// [tests/cases/conformance/ambient/ambientDeclarationsPatterns_merging3.ts] ////
+
+//// [types.ts]
+declare module "*.foo" {
+  export interface OhNo { star: string }
+}
+
+//// [test.ts]
+declare module "a.foo" {
+  export interface OhNo { a: string }
+}
+import { OhNo } from "b.foo"
+declare let ohno: OhNo;
+ohno.a // oh no
+
+
+//// [types.js]
+//// [test.js]
+"use strict";
+exports.__esModule = true;
+ohno.a; // oh no
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging3.symbols b/tests/baselines/reference/ambientDeclarationsPatterns_merging3.symbols
@@ -0,0 +1,27 @@
+=== tests/cases/conformance/ambient/types.ts ===
+declare module "*.foo" {
+>"*.foo" : Symbol("*.foo", Decl(types.ts, 0, 0))
+
+  export interface OhNo { star: string }
+>OhNo : Symbol(OhNo, Decl(types.ts, 0, 24))
+>star : Symbol(OhNo.star, Decl(types.ts, 1, 25))
+}
+
+=== tests/cases/conformance/ambient/test.ts ===
+declare module "a.foo" {
+>"a.foo" : Symbol("a.foo", Decl(test.ts, 0, 0), Decl(types.ts, 0, 0))
+
+  export interface OhNo { a: string }
+>OhNo : Symbol(OhNo, Decl(test.ts, 0, 24), Decl(types.ts, 0, 24))
+>a : Symbol(OhNo.a, Decl(test.ts, 1, 25))
+}
+import { OhNo } from "b.foo"
+>OhNo : Symbol(OhNo, Decl(test.ts, 3, 8))
+
+declare let ohno: OhNo;
+>ohno : Symbol(ohno, Decl(test.ts, 4, 11))
+>OhNo : Symbol(OhNo, Decl(test.ts, 3, 8))
+
+ohno.a // oh no
+>ohno : Symbol(ohno, Decl(test.ts, 4, 11))
+
diff --git a/tests/baselines/reference/ambientDeclarationsPatterns_merging3.types b/tests/baselines/reference/ambientDeclarationsPatterns_merging3.types
diff --git a/tests/cases/conformance/ambient/ambientDeclarationsPatterns_merging1.ts b/tests/cases/conformance/ambient/ambientDeclarationsPatterns_merging1.ts
diff --git a/tests/cases/conformance/ambient/ambientDeclarationsPatterns_merging2.ts b/tests/cases/conformance/ambient/ambientDeclarationsPatterns_merging2.ts
diff --git a/tests/cases/conformance/ambient/ambientDeclarationsPatterns_merging3.ts b/tests/cases/conformance/ambient/ambientDeclarationsPatterns_merging3.ts
