Navigation Menu

Skip to content

Commit

Permalink
Merge pull request #31078 from andrewbranch/bug/30752
Browse files Browse the repository at this point in the history
Fix symbol merging of augmentations to pattern ambient modules
  • Loading branch information
andrewbranch committed Apr 30, 2019
2 parents 9005449 + 973c3ca commit 90d3acf
Show file tree
Hide file tree
Showing 16 changed files with 386 additions and 7 deletions.
41 changes: 34 additions & 7 deletions src/compiler/checker.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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`,
Expand Down Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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);
}
}
Expand Down
@@ -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'.

25 changes: 25 additions & 0 deletions 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;
@@ -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))

@@ -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

@@ -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;
}
27 changes: 27 additions & 0 deletions 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;
@@ -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))
}
@@ -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
}
@@ -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'.

21 changes: 21 additions & 0 deletions 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
@@ -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))

0 comments on commit 90d3acf

Please sign in to comment.