Skip to content

moduleAugmentations may contain an Identifier #18009

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.

Sign up for GitHub

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

Merged
3 commits merged into from
Aug 25, 2017
Merged

moduleAugmentations may contain an Identifier #18009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
014ad42
`moduleAugmentations` may contain an `Identifier`
Aug 24, 2017
130127d
Add comment
Aug 25, 2017
0868980
Rename function
Aug 25, 2017
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
7 changes: 4 additions & 3 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ namespace ts {
});
}

function mergeModuleAugmentation(moduleName: LiteralExpression): void {
function mergeModuleAugmentation(moduleName: StringLiteral | Identifier): void {
const moduleAugmentation = <ModuleDeclaration>moduleName.parent;
if (moduleAugmentation.symbol.declarations[0] !== moduleAugmentation) {
// this is a combined symbol for multiple augmentations within the same file.
Expand Down Expand Up @@ -670,7 +670,8 @@ namespace ts {
mergeSymbol(mainModule, moduleAugmentation.symbol);
}
else {
error(moduleName, Diagnostics.Cannot_augment_module_0_because_it_resolves_to_a_non_module_entity, moduleName.text);
// moduleName will be a StringLiteral since this is not `declare global`.
error(moduleName, Diagnostics.Cannot_augment_module_0_because_it_resolves_to_a_non_module_entity, (moduleName as StringLiteral).text);
}
}
}
Expand Down Expand Up @@ -23799,7 +23800,7 @@ namespace ts {
}

// Initialize global symbol table
let augmentations: ReadonlyArray<StringLiteral>[];
let augmentations: ReadonlyArray<StringLiteral | Identifier>[];
for (const file of host.getSourceFiles()) {
if (!isExternalOrCommonJsModule(file)) {
mergeSymbolTable(globals, file.locals);
Expand Down
34 changes: 21 additions & 13 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ namespace ts {
for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) {
const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory);
if (resolveModuleNamesWorker) {
const moduleNames = map(concatenate(newSourceFile.imports, newSourceFile.moduleAugmentations), getTextOfLiteral);
const moduleNames = getModuleNames(newSourceFile);
const oldProgramState = { program: oldProgram, file: oldSourceFile, modifiedFilePaths };
const resolutions = resolveModuleNamesReusingOldState(moduleNames, newSourceFilePath, newSourceFile, oldProgramState);
// ensure that module resolution results are still correct
Expand Down Expand Up @@ -1430,12 +1430,10 @@ namespace ts {
return a.fileName === b.fileName;
}

function moduleNameIsEqualTo(a: LiteralExpression, b: LiteralExpression): boolean {
return a.text === b.text;
}

function getTextOfLiteral(literal: LiteralExpression): string {
return literal.text;
function moduleNameIsEqualTo(a: StringLiteral | Identifier, b: StringLiteral | Identifier): boolean {
return a.kind === SyntaxKind.StringLiteral
Copy link
Copy Markdown
Member

@amcasey amcasey Aug 25, 2017

Choose a reason for hiding this comment

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

Just to confirm, the string literal "global" is not equal to the identifier global?

Copy link
Copy Markdown
Author

@ghost ghost Aug 25, 2017

Choose a reason for hiding this comment

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

export {}; // this is a module

// Augments a module named "global"
declare module "global" {
    export const x: number;
}

// Augments the global scope
declare global {
    const y: number;
}

// Module access: "y" fails
import { x as _x, y as _y } from "global";

// Global access: "x" fails
x; y;

? b.kind === SyntaxKind.StringLiteral && a.text === b.text
: b.kind === SyntaxKind.Identifier && a.escapedText === b.escapedText;
}

function collectExternalModuleReferences(file: SourceFile): void {
Expand All @@ -1448,7 +1446,7 @@ namespace ts {

// file.imports may not be undefined if there exists dynamic import
let imports: StringLiteral[];
let moduleAugmentations: StringLiteral[];
let moduleAugmentations: Array<StringLiteral | Identifier>;
Copy link
Copy Markdown
Member

@amcasey amcasey Aug 25, 2017

Choose a reason for hiding this comment

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

For my own edification, what's the difference between T[] and Array<T>?

Copy link
Copy Markdown
Author

@ghost ghost Aug 25, 2017 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

No semantic difference. I prefer to use Array<> when wrapping unions to avoid needing parentheses. (Array<A | B> instead of (A | B)[]). (See array-type (the "array-simple" option), which maybe we should start using.)

let ambientModules: string[];

// If we are importing helpers, we need to add a synthetic reference to resolve the
Expand Down Expand Up @@ -1477,7 +1475,7 @@ namespace ts {

return;

function collectModuleReferences(node: Node, inAmbientModule: boolean): void {
function collectModuleReferences(node: Statement, inAmbientModule: boolean): void {
switch (node.kind) {
case SyntaxKind.ImportDeclaration:
case SyntaxKind.ImportEqualsDeclaration:
Expand All @@ -1499,8 +1497,8 @@ namespace ts {
break;
case SyntaxKind.ModuleDeclaration:
if (isAmbientModule(<ModuleDeclaration>node) && (inAmbientModule || hasModifier(node, ModifierFlags.Ambient) || file.isDeclarationFile)) {
const moduleName = <StringLiteral>(<ModuleDeclaration>node).name; // TODO: GH#17347
const nameText = ts.getTextOfIdentifierOrLiteral(moduleName);
const moduleName = (<ModuleDeclaration>node).name;
const nameText = getTextOfIdentifierOrLiteral(moduleName);
// Ambient module declarations can be interpreted as augmentations for some existing external modules.
// This will happen in two cases:
// - if current file is external module then module augmentation is a ambient module declaration defined in the top level scope
Expand Down Expand Up @@ -1817,8 +1815,7 @@ namespace ts {
collectExternalModuleReferences(file);
if (file.imports.length || file.moduleAugmentations.length) {
// Because global augmentation doesn't have string literal name, we can check for global augmentation as such.
const nonGlobalAugmentation = filter(file.moduleAugmentations, (moduleAugmentation) => moduleAugmentation.kind === SyntaxKind.StringLiteral);
const moduleNames = map(concatenate(file.imports, nonGlobalAugmentation), getTextOfLiteral);
const moduleNames = getModuleNames(file);
const oldProgramState = { program: oldProgram, file, modifiedFilePaths };
const resolutions = resolveModuleNamesReusingOldState(moduleNames, getNormalizedAbsolutePath(file.fileName, currentDirectory), file, oldProgramState);
Debug.assert(resolutions.length === moduleNames.length);
Expand Down Expand Up @@ -2229,4 +2226,15 @@ namespace ts {
Debug.assert(names.every(name => name !== undefined), "A name is undefined.", () => JSON.stringify(names));
return names;
}

function getModuleNames({ imports, moduleAugmentations }: SourceFile): string[] {
const res = imports.map(i => i.text);
for (const aug of moduleAugmentations) {
if (aug.kind === SyntaxKind.StringLiteral) {
res.push(aug.text);
}
// Do nothing if it's an Identifier; we don't need to do module resolution for `declare global`.
}
return res;
}
}
3 changes: 2 additions & 1 deletion src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2343,7 +2343,8 @@ namespace ts {
/* @internal */ resolvedModules: Map<ResolvedModuleFull>;
/* @internal */ resolvedTypeReferenceDirectiveNames: Map<ResolvedTypeReferenceDirective>;
/* @internal */ imports: ReadonlyArray<StringLiteral>;
/* @internal */ moduleAugmentations: ReadonlyArray<StringLiteral>;
// Identifier only if `declare global`
/* @internal */ moduleAugmentations: ReadonlyArray<StringLiteral | Identifier>;
/* @internal */ patternAmbientModules?: PatternAmbientModule[];
/* @internal */ ambientModuleNames: ReadonlyArray<string>;
/* @internal */ checkJsDirective: CheckJsDirective | undefined;
Expand Down