Skip to content

Commit

Permalink
Explicitly merge module augmentation members into the exports added b…
Browse files Browse the repository at this point in the history
…y export * declarations (microsoft#37691)

* Explicitly merge module augmentation members into the exports added by export * declarations

* Use ?., add namespace merge test

* Add missing merged symbol call to handle enum/ns merges during entity name lookup

* Add test with error case

* Handle missing value declarations in more places, unify duplicate declaration error handling to improve assignment declaration duplicate errors
  • Loading branch information
weswigham authored Apr 1, 2020
1 parent c546988 commit 15aff05
Show file tree
Hide file tree
Showing 58 changed files with 793 additions and 74 deletions.
51 changes: 32 additions & 19 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -813,8 +813,8 @@ namespace ts {
};

interface DuplicateInfoForSymbol {
readonly firstFileLocations: Node[];
readonly secondFileLocations: Node[];
readonly firstFileLocations: Declaration[];
readonly secondFileLocations: Declaration[];
readonly isBlockScoped: boolean;
}
interface DuplicateInfoForFiles {
Expand Down Expand Up @@ -1172,26 +1172,30 @@ namespace ts {
}
return target;

function addDuplicateLocations(locs: Node[], symbol: Symbol): void {
function addDuplicateLocations(locs: Declaration[], symbol: Symbol): void {
for (const decl of symbol.declarations) {
pushIfUnique(locs, (getExpandoInitializer(decl, /*isPrototypeAssignment*/ false) ? getNameOfExpando(decl) : getNameOfDeclaration(decl)) || decl);
pushIfUnique(locs, decl);
}
}
}

function addDuplicateDeclarationErrorsForSymbols(target: Symbol, message: DiagnosticMessage, symbolName: string, source: Symbol) {
forEach(target.declarations, node => {
const errorNode = (getExpandoInitializer(node, /*isPrototypeAssignment*/ false) ? getNameOfExpando(node) : getNameOfDeclaration(node)) || node;
addDuplicateDeclarationError(errorNode, message, symbolName, source.declarations);
addDuplicateDeclarationError(node, message, symbolName, source.declarations);
});
}

function addDuplicateDeclarationError(errorNode: Node, message: DiagnosticMessage, symbolName: string, relatedNodes: readonly Node[] | undefined) {
function addDuplicateDeclarationError(node: Declaration, message: DiagnosticMessage, symbolName: string, relatedNodes: readonly Declaration[] | undefined) {
const errorNode = (getExpandoInitializer(node, /*isPrototypeAssignment*/ false) ? getNameOfExpando(node) : getNameOfDeclaration(node)) || node;
const err = lookupOrIssueError(errorNode, message, symbolName);
for (const relatedNode of relatedNodes || emptyArray) {
const adjustedNode = (getExpandoInitializer(relatedNode, /*isPrototypeAssignment*/ false) ? getNameOfExpando(relatedNode) : getNameOfDeclaration(relatedNode)) || relatedNode;
if (adjustedNode === errorNode) continue;
err.relatedInformation = err.relatedInformation || [];
if (length(err.relatedInformation) >= 5) continue;
addRelatedInfo(err, !length(err.relatedInformation) ? createDiagnosticForNode(relatedNode, Diagnostics._0_was_also_declared_here, symbolName) : createDiagnosticForNode(relatedNode, Diagnostics.and_here));
const leadingMessage = createDiagnosticForNode(adjustedNode, Diagnostics._0_was_also_declared_here, symbolName);
const followOnMessage = createDiagnosticForNode(adjustedNode, Diagnostics.and_here);
if (length(err.relatedInformation) >= 5 || some(err.relatedInformation, r => compareDiagnostics(r, followOnMessage) === Comparison.EqualTo || compareDiagnostics(r, leadingMessage) === Comparison.EqualTo)) continue;
addRelatedInfo(err, !length(err.relatedInformation) ? leadingMessage : followOnMessage);
}
}

Expand Down Expand Up @@ -1251,6 +1255,15 @@ namespace ts {
patternAmbientModuleAugmentations.set((moduleName as StringLiteral).text, merged);
}
else {
if (mainModule.exports?.get(InternalSymbolName.ExportStar) && moduleAugmentation.symbol.exports?.size) {
// We may need to merge the module augmentation's exports into the target symbols of the resolved exports
const resolvedExports = getResolvedMembersOrExportsOfSymbol(mainModule, MembersOrExportsResolutionKind.resolvedExports);
for (const [key, value] of arrayFrom(moduleAugmentation.symbol.exports.entries())) {
if (resolvedExports.has(key) && !mainModule.exports.has(key)) {
mergeSymbol(resolvedExports.get(key)!, value);
}
}
}
mergeSymbol(mainModule, moduleAugmentation.symbol);
}
}
Expand Down Expand Up @@ -1295,7 +1308,7 @@ namespace ts {

function getSymbol(symbols: SymbolTable, name: __String, meaning: SymbolFlags): Symbol | undefined {
if (meaning) {
const symbol = symbols.get(name);
const symbol = getMergedSymbol(symbols.get(name));
if (symbol) {
Debug.assert((getCheckFlags(symbol) & CheckFlags.Instantiated) === 0, "Should never get an instantiated symbol here.");
if (symbol.flags & meaning) {
Expand Down Expand Up @@ -2878,9 +2891,9 @@ namespace ts {
if (name.kind === SyntaxKind.Identifier) {
const message = meaning === namespaceMeaning || nodeIsSynthesized(name) ? Diagnostics.Cannot_find_namespace_0 : getCannotFindNameDiagnosticForName(getFirstIdentifier(name));
const symbolFromJSPrototype = isInJSFile(name) && !nodeIsSynthesized(name) ? resolveEntityNameFromAssignmentDeclaration(name, meaning) : undefined;
symbol = resolveName(location || name, name.escapedText, meaning, ignoreErrors || symbolFromJSPrototype ? undefined : message, name, /*isUse*/ true);
symbol = getMergedSymbol(resolveName(location || name, name.escapedText, meaning, ignoreErrors || symbolFromJSPrototype ? undefined : message, name, /*isUse*/ true));
if (!symbol) {
return symbolFromJSPrototype;
return getMergedSymbol(symbolFromJSPrototype);
}
}
else if (name.kind === SyntaxKind.QualifiedName || name.kind === SyntaxKind.PropertyAccessExpression) {
Expand Down Expand Up @@ -2908,7 +2921,7 @@ namespace ts {
}
}
}
symbol = getSymbol(getExportsOfSymbol(namespace), right.escapedText, meaning);
symbol = getMergedSymbol(getSymbol(getExportsOfSymbol(namespace), right.escapedText, meaning));
if (!symbol) {
if (!ignoreErrors) {
error(right, Diagnostics.Namespace_0_has_no_exported_member_1, getFullyQualifiedName(namespace), declarationNameToString(right));
Expand Down Expand Up @@ -7974,7 +7987,7 @@ namespace ts {
let links = getSymbolLinks(symbol);
const originalLinks = links;
if (!links.type) {
const jsDeclaration = getDeclarationOfExpando(symbol.valueDeclaration);
const jsDeclaration = symbol.valueDeclaration && getDeclarationOfExpando(symbol.valueDeclaration);
if (jsDeclaration) {
const merged = mergeJSSymbols(symbol, getSymbolOfNode(jsDeclaration));
if (merged) {
Expand All @@ -7992,9 +8005,9 @@ namespace ts {
if (symbol.flags & SymbolFlags.Module && isShorthandAmbientModuleSymbol(symbol)) {
return anyType;
}
else if (declaration.kind === SyntaxKind.BinaryExpression ||
else if (declaration && (declaration.kind === SyntaxKind.BinaryExpression ||
isAccessExpression(declaration) &&
declaration.parent.kind === SyntaxKind.BinaryExpression) {
declaration.parent.kind === SyntaxKind.BinaryExpression)) {
return getWidenedTypeForAssignmentDeclaration(symbol);
}
else if (symbol.flags & SymbolFlags.ValueModule && declaration && isSourceFile(declaration) && declaration.commonJsModuleIndicator) {
Expand Down Expand Up @@ -28188,8 +28201,8 @@ namespace ts {
const name = prop.escapedName;
const symbol = resolveName(prop.valueDeclaration, name, SymbolFlags.Type, undefined, name, /*isUse*/ false);
if (symbol && symbol.declarations.some(isJSDocTypedefTag)) {
grammarErrorOnNode(symbol.declarations[0], Diagnostics.Duplicate_identifier_0, unescapeLeadingUnderscores(name));
return grammarErrorOnNode(prop.valueDeclaration, Diagnostics.Duplicate_identifier_0, unescapeLeadingUnderscores(name));
addDuplicateDeclarationErrorsForSymbols(symbol, Diagnostics.Duplicate_identifier_0, unescapeLeadingUnderscores(name), prop);
addDuplicateDeclarationErrorsForSymbols(prop, Diagnostics.Duplicate_identifier_0, unescapeLeadingUnderscores(name), symbol);
}
}
}
Expand Down Expand Up @@ -29995,7 +30008,7 @@ namespace ts {
// entirely unable to merge - a more helpful message like "Class declaration cannot implement overload list"
// might be warranted. :shrug:
forEach(declarations, declaration => {
addDuplicateDeclarationError(getNameOfDeclaration(declaration) || declaration, Diagnostics.Duplicate_identifier_0, symbolName(symbol), filter(declarations, d => d !== declaration));
addDuplicateDeclarationError(declaration, Diagnostics.Duplicate_identifier_0, symbolName(symbol), declarations);
});
}

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals3.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace foo {
>v : Symbol(v, Decl(file1.ts, 2, 14))
}
export = foo;
>foo : Symbol(foo, Decl(file1.ts, 0, 0), Decl(file1.ts, 0, 17))
>foo : Symbol(foo, Decl(file1.ts, 0, 0), Decl(file1.ts, 0, 17), Decl(file2.ts, 1, 8))

=== tests/cases/compiler/file2.ts ===
import x = require("./file1");
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals3.types
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace foo {
>1 : 1
}
export = foo;
>foo : typeof import("tests/cases/compiler/file1")
>foo : typeof import("tests/cases/compiler/file1.ts")

=== tests/cases/compiler/file2.ts ===
import x = require("./file1");
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals3_1.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ declare module "file1" {
>v : Symbol(v, Decl(file1.d.ts, 3, 18))
}
export = foo;
>foo : Symbol(foo, Decl(file1.d.ts, 0, 24), Decl(file1.d.ts, 1, 25))
>foo : Symbol(foo, Decl(file1.d.ts, 0, 24), Decl(file1.d.ts, 1, 25), Decl(file2.ts, 2, 8))
}


Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals3_1.types
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ declare module "file1" {
>v : number
}
export = foo;
>foo : typeof foo
>foo : typeof import("tests/cases/compiler/file1.d.ts")
}


Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals4.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace foo {
>v : Symbol(v, Decl(file1.ts, 2, 14))
}
export = foo;
>foo : Symbol(foo, Decl(file1.ts, 0, 0), Decl(file1.ts, 0, 12))
>foo : Symbol(foo, Decl(file1.ts, 0, 0), Decl(file1.ts, 0, 12), Decl(file2.ts, 1, 8))

=== tests/cases/compiler/file2.ts ===
import x = require("./file1");
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals4.types
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace foo {
>1 : 1
}
export = foo;
>foo : import("tests/cases/compiler/file1")
>foo : import("tests/cases/compiler/file1.ts")

=== tests/cases/compiler/file2.ts ===
import x = require("./file1");
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals4_1.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ declare module "file1" {
>v : Symbol(v, Decl(file1.d.ts, 3, 18))
}
export = foo;
>foo : Symbol(foo, Decl(file1.d.ts, 0, 24), Decl(file1.d.ts, 1, 16))
>foo : Symbol(foo, Decl(file1.d.ts, 0, 24), Decl(file1.d.ts, 1, 16), Decl(file2.ts, 2, 8))
}


Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals4_1.types
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ declare module "file1" {
>v : number
}
export = foo;
>foo : foo
>foo : import("tests/cases/compiler/file1.d.ts")
}


Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/augmentExportEquals5.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ declare module "express" {

function e(): e.Express;
>e : Symbol(e, Decl(express.d.ts, 6, 26), Decl(express.d.ts, 7, 28), Decl(augmentation.ts, 1, 29))
>e : Symbol(e, Decl(express.d.ts, 6, 26), Decl(express.d.ts, 7, 28))
>e : Symbol(e, Decl(express.d.ts, 6, 26), Decl(express.d.ts, 7, 28), Decl(augmentation.ts, 1, 29))
>Express : Symbol(e.Express, Decl(express.d.ts, 52, 9))

namespace e {
Expand Down Expand Up @@ -160,7 +160,7 @@ declare module "express" {
}

export = e;
>e : Symbol(e, Decl(express.d.ts, 6, 26), Decl(express.d.ts, 7, 28))
>e : Symbol(e, Decl(express.d.ts, 6, 26), Decl(express.d.ts, 7, 28), Decl(augmentation.ts, 1, 29))
}

=== tests/cases/compiler/augmentation.ts ===
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals5.types
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ declare module "express" {
}

export = e;
>e : typeof e
>e : typeof import("tests/cases/compiler/express.d.ts")
}

=== tests/cases/compiler/augmentation.ts ===
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals6.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace foo {
>a : Symbol(a, Decl(file1.ts, 3, 35))
}
export = foo;
>foo : Symbol(foo, Decl(file1.ts, 0, 0), Decl(file1.ts, 0, 12))
>foo : Symbol(foo, Decl(file1.ts, 0, 0), Decl(file1.ts, 0, 12), Decl(file2.ts, 1, 10))

=== tests/cases/compiler/file2.ts ===
import x = require("./file1");
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals6.types
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace foo {
>a : any
}
export = foo;
>foo : import("tests/cases/compiler/file1")
>foo : import("tests/cases/compiler/file1.ts")

=== tests/cases/compiler/file2.ts ===
import x = require("./file1");
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals6_1.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ declare module "file1" {
>A : Symbol(A, Decl(file1.d.ts, 2, 19), Decl(file2.ts, 4, 24))
}
export = foo;
>foo : Symbol(foo, Decl(file1.d.ts, 0, 24), Decl(file1.d.ts, 1, 16))
>foo : Symbol(foo, Decl(file1.d.ts, 0, 24), Decl(file1.d.ts, 1, 16), Decl(file2.ts, 1, 28))
}


Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/augmentExportEquals6_1.types
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ declare module "file1" {
>A : A
}
export = foo;
>foo : foo
>foo : import("tests/cases/compiler/file1.d.ts")
}


Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
=== tests/cases/compiler/input.ts ===
export = exports;
>exports : import("tests/cases/compiler/input")
>exports : exports

declare class exports {
>exports : exports
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ module P { }

import p = P;
>p : Symbol(p, Decl(duplicateVarsAcrossFileBoundaries_4.ts, 0, 12))
>P : Symbol(P, Decl(duplicateVarsAcrossFileBoundaries_4.ts, 0, 0))
>P : Symbol(P, Decl(duplicateVarsAcrossFileBoundaries_4.ts, 0, 0), Decl(duplicateVarsAcrossFileBoundaries_5.ts, 2, 3))

var q;
>q : Symbol(q, Decl(duplicateVarsAcrossFileBoundaries_4.ts, 2, 3), Decl(duplicateVarsAcrossFileBoundaries_5.ts, 0, 12))
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/es5ExportEquals.types
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ export function f() { }
>f : typeof f

export = f;
>f : { (): void; f: typeof import("tests/cases/compiler/es5ExportEquals"); }
>f : typeof f

2 changes: 1 addition & 1 deletion tests/baselines/reference/es6ExportEquals.types
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ export function f() { }
>f : typeof f

export = f;
>f : { (): void; f: typeof import("tests/cases/compiler/es6ExportEquals"); }
>f : typeof f

Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ class C1 {

// Invalid, as there is already an exported member.
export = C1;
>C1 : import("tests/cases/conformance/externalModules/foo_0")
>C1 : C1

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
=== /node_modules/foo/index.d.ts ===
export = foo;
>foo : Symbol(foo, Decl(index.d.ts, 0, 13))
>foo : Symbol(foo, Decl(index.d.ts, 0, 13), Decl(a.ts, 0, 27), Decl(b.ts, 0, 27))

declare namespace foo {
>foo : Symbol(foo, Decl(index.d.ts, 0, 13), Decl(a.ts, 0, 27), Decl(b.ts, 0, 27))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
=== /node_modules/foo/index.d.ts ===
export = foo;
>foo : any
>foo : typeof import("/node_modules/foo/index.d.ts")

declare namespace foo {
export type T = number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ class D { }
>D : D

export = D;
>D : import("tests/cases/compiler/exportAssignmentWithExports")
>D : D

Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ module x {
}
export import a = x.c;
>a : any
>x : any
>x : typeof x
>c : any

export = x;
>x : any
>x : typeof x

Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ declare module "m" {
}
export import a = x.c;
>a : any
>x : any
>x : typeof x
>c : a

export = x;
>x : any
>x : typeof x
}
4 changes: 2 additions & 2 deletions tests/baselines/reference/intersectionsOfLargeUnions2.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ declare global {

[index: number]: HTMLElement
>index : Symbol(index, Decl(intersectionsOfLargeUnions2.ts, 4, 9))
>HTMLElement : Symbol(HTMLElement, Decl(intersectionsOfLargeUnions2.ts, 5, 5))
>HTMLElement : Symbol(HTMLElement, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --), Decl(intersectionsOfLargeUnions2.ts, 5, 5))
}

interface HTMLElement {
>HTMLElement : Symbol(HTMLElement, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --), Decl(intersectionsOfLargeUnions2.ts, 5, 5))

[index: number]: HTMLElement;
>index : Symbol(index, Decl(intersectionsOfLargeUnions2.ts, 8, 9))
>HTMLElement : Symbol(HTMLElement, Decl(intersectionsOfLargeUnions2.ts, 5, 5))
>HTMLElement : Symbol(HTMLElement, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --), Decl(intersectionsOfLargeUnions2.ts, 5, 5))
}
}

Expand Down
Loading

0 comments on commit 15aff05

Please sign in to comment.