Skip to content
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

initial revision of external module augmentations #6213

Merged
merged 10 commits into from
Jan 16, 2016
Merged

initial revision of external module augmentations #6213

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4a963a2
initial revision of external module augmentations
vladima Dec 22, 2015
3e1bc01
address PR feedback
vladima Dec 23, 2015
7f2ebf9
use 'declare global' to define augmentations for the global scope
vladima Dec 28, 2015
7cef1ef
merge with origin/master
vladima Jan 12, 2016
0e1c6e3
fix linter issues
vladima Jan 12, 2016
1e11a55
address PR feedback
vladima Jan 13, 2016
3aa92f5
addressed PR feedback
vladima Jan 14, 2016
a69dcd1
Merge remote-tracking branch 'origin/master' into moduleAugmentations
vladima Jan 14, 2016
e90db46
merge with origin/master
vladima Jan 14, 2016
36c489c
address PR feedback
vladima Jan 15, 2016
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
14 changes: 12 additions & 2 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ namespace ts {
let blockScopeContainer: Node;
let lastContainer: Node;
let seenThisKeyword: boolean;
let isSourceFileExternalModule: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

I don't see this getting used anywhere apart from the later assignment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, forgot to remove that. Thanks


// state used by reachability checks
let hasExplicitReturn: boolean;
Expand All @@ -129,8 +130,9 @@ namespace ts {
function bindSourceFile(f: SourceFile, opts: CompilerOptions) {
file = f;
options = opts;
inStrictMode = !!file.externalModuleIndicator;
isSourceFileExternalModule = inStrictMode = !!file.externalModuleIndicator;
classifiableNames = {};

Symbol = objectAllocator.getSymbolConstructor();

if (!file.locals) {
Expand Down Expand Up @@ -348,7 +350,12 @@ namespace ts {
// 2. When we checkIdentifier in the checker, we set its resolved symbol to the local symbol,
// but return the export symbol (by calling getExportSymbolOfValueSymbolIfExported). That way
// when the emitter comes back to it, it knows not to qualify the name if it was found in a containing scope.
if (hasExportModifier || container.flags & NodeFlags.ExportContext) {

// NOTE: Nested ambient modules always should go to to 'locals' table to prevent their automatic merge
// during global merging in the checker. Why? The only case when ambient module is permitted inside another module is module augmentation
// and this case is specially handled. Module augmentations should only be merged with original module definition
// and should never be merged directly with other augmentation and the latter case would be possible is automatic merge is allowed.
Copy link
Member

Choose a reason for hiding this comment

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

add a comma after "augmentation" so this reads more easily, and add I think you meant "if" instead of "is" on this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if (!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) {
const exportKind =
(symbolFlags & SymbolFlags.Value ? SymbolFlags.ExportValue : 0) |
(symbolFlags & SymbolFlags.Type ? SymbolFlags.ExportType : 0) |
Expand Down Expand Up @@ -844,6 +851,9 @@ namespace ts {
function bindModuleDeclaration(node: ModuleDeclaration) {
setExportContextFlag(node);
if (node.name.kind === SyntaxKind.StringLiteral) {
if (node.flags & NodeFlags.Export) {
errorOnFirstToken(node, Diagnostics.export_modifier_cannot_be_applied_to_ambient_modules_and_module_augmentations_since_they_are_always_visible);
}
declareSymbolAndAddToSymbolTable(node, SymbolFlags.ValueModule, SymbolFlags.ValueModuleExcludes);
}
else {
Expand Down
206 changes: 175 additions & 31 deletions src/compiler/checker.ts
View file
Open in desktop

Large diffs are not rendered by default.

28 changes: 26 additions & 2 deletions src/compiler/declarationEmitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ namespace ts {
let writer = createAndSetNewTextWriterWithSymbolWriter();

let enclosingDeclaration: Node;
let resultHasExternalModuleIndicator: boolean;
let currentText: string;
let currentLineMap: number[];
let currentIdentifiers: Map<string>;
Expand Down Expand Up @@ -101,6 +102,7 @@ namespace ts {
});
}

resultHasExternalModuleIndicator = false;
if (!isBundledEmit || !isExternalModule(sourceFile)) {
noDeclare = false;
emitSourceFile(sourceFile);
Expand Down Expand Up @@ -139,6 +141,14 @@ namespace ts {
allSourcesModuleElementDeclarationEmitInfo = allSourcesModuleElementDeclarationEmitInfo.concat(moduleElementDeclarationEmitInfo);
moduleElementDeclarationEmitInfo = [];
}

if (!isBundledEmit && isExternalModule(sourceFile) && sourceFile.moduleAugmentations.length && !resultHasExternalModuleIndicator) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about external modules with no exports or no module augmentation? soemthing like:

import {a} from "./mod";
// do something with a

Copy link
Contributor

Choose a reason for hiding this comment

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

i thought we will do this for any external module, that we never emitted any import, export, or module augmentation for.

// if file was external module with augmentations - this fact should be preserved in .d.ts as well.
// in case if we didn't write any external module specifiers in .d.ts we need to emit something
// that will force compiler to think that this file is an external module - 'export {}' is a reasonable choice here.
write("export {};");
writeLine();
}
});

return {
Expand Down Expand Up @@ -720,16 +730,25 @@ namespace ts {
writer.writeLine();
}

function emitExternalModuleSpecifier(parent: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration) {
function emitExternalModuleSpecifier(parent: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration | ModuleDeclaration) {
// emitExternalModuleSpecifier is usualyl called when we emit something in the.d.ts file that will make it an external module (i.e. import/export declarations).
// the only case when it is not true is when we call it to emit correct name for module augmentation - d.ts files with just module augmentations are not considered
// external modules since they are indistingushable from script files with ambient modules. To fix this in such d.ts files we'll emit top level 'export {}'
// so compiler will treat them as external modules.
resultHasExternalModuleIndicator = resultHasExternalModuleIndicator || parent.kind !== SyntaxKind.ModuleDeclaration;
let moduleSpecifier: Node;
if (parent.kind === SyntaxKind.ImportEqualsDeclaration) {
const node = parent as ImportEqualsDeclaration;
moduleSpecifier = getExternalModuleImportEqualsDeclarationExpression(node);
}
else if (parent.kind === SyntaxKind.ModuleDeclaration) {
moduleSpecifier = (<ModuleDeclaration>parent).name;
}
else {
const node = parent as (ImportDeclaration | ExportDeclaration);
moduleSpecifier = node.moduleSpecifier;
}

if (moduleSpecifier.kind === SyntaxKind.StringLiteral && isBundledEmit && (compilerOptions.out || compilerOptions.outFile)) {
const moduleName = getExternalModuleNameFromDeclaration(host, resolver, parent);
if (moduleName) {
Expand Down Expand Up @@ -789,7 +808,12 @@ namespace ts {
else {
write("module ");
}
writeTextOfNode(currentText, node.name);
if (isExternalModuleAugmentation(node)) {
emitExternalModuleSpecifier(node);
}
else {
writeTextOfNode(currentText, node.name);
}
while (node.body.kind !== SyntaxKind.ModuleBlock) {
node = <ModuleDeclaration>node.body;
write(".");
Expand Down
20 changes: 20 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1771,6 +1771,26 @@
"category": "Error",
"code": 2660
},
"Invalid module name in augmentation, module '{0}' cannot be found.": {
"category": "Error",
"code": 2661
},
"Module augmentation cannot introduce new names in the top level scope.": {
"category": "Error",
"code": 2662
},
"Exports are not permitted in module augmentations.": {
"category": "Error",
"code": 2663
},
"Imports are not permitted in module augmentations. Consider moving them to the enclosing external module.": {
"category": "Error",
"code": 2664
},
"'export' modifier cannot be applied to ambient modules and module augmentations since they are always visible.": {
"category": "Error",
"code": 2665
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
"code": 4000
Expand Down
141 changes: 99 additions & 42 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,24 @@ namespace ts {
const currentDirectory = host.getCurrentDirectory();
const resolveModuleNamesWorker = host.resolveModuleNames
? ((moduleNames: string[], containingFile: string) => host.resolveModuleNames(moduleNames, containingFile))
: ((moduleNames: string[], containingFile: string) => map(moduleNames, moduleName => resolveModuleName(moduleName, containingFile, options, host).resolvedModule));
: ((moduleNames: string[], containingFile: string) => {
const resolvedModuleNames: ResolvedModule[] = [];
// resolveModuleName does not store any results between calls.
// lookup is a local cache to avoid resolving the same module name several times
const lookup: Map<ResolvedModule> = {};
for (const moduleName of moduleNames) {
let resolvedName: ResolvedModule;
if (hasProperty(lookup, moduleName)) {
resolvedName = lookup[moduleName];
}
else {
resolvedName = resolveModuleName(moduleName, containingFile, options, host).resolvedModule;
lookup[moduleName] = resolvedName;
}
resolvedModuleNames.push(resolvedName);
}
return resolvedModuleNames;
});

const filesByName = createFileMap<SourceFile>();
// stores 'filename -> file association' ignoring case
Expand Down Expand Up @@ -484,15 +501,25 @@ namespace ts {
return false;
}

// check imports
// check imports and module augmentations
collectExternalModuleReferences(newSourceFile);
if (!arrayIsEqualTo(oldSourceFile.imports, newSourceFile.imports, moduleNameIsEqualTo)) {
// imports has changed
return false;
}
if (!arrayIsEqualTo(oldSourceFile.moduleAugmentations, newSourceFile.moduleAugmentations, moduleNameIsEqualTo)) {
// moduleAugmentations has changed
return false;
}

if (resolveModuleNamesWorker) {
const moduleNames = map(newSourceFile.imports, name => name.text);
const moduleNames: string[] = [];
for (const moduleName of newSourceFile.imports) {
moduleNames.push(moduleName.text);
}
for (const moduleName of newSourceFile.moduleAugmentations) {
moduleNames.push(moduleName.text);
}
Copy link
Member

Choose a reason for hiding this comment

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

const moduleNames = map(concatenate(newSourceFile.imports, newSourceFile.moduleAugmentations), name => name.text);

Copy link
Member

Choose a reason for hiding this comment

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

Just FYI, the concatenate function treats undefined the same as an empty array and only allocates a new array if both arguments are defined. Otherwise it just returns the one defined argument. Since it'll be rare for both to be defined here, it's fine to use this function instead of the imperative loops.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅

const resolutions = resolveModuleNamesWorker(moduleNames, getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory));
// ensure that module resolution results are still correct
for (let i = 0; i < moduleNames.length; ++i) {
Expand Down Expand Up @@ -887,59 +914,75 @@ namespace ts {
}

const isJavaScriptFile = isSourceFileJavaScript(file);
const isExternalModuleFile = isExternalModule(file);

let imports: LiteralExpression[];
let moduleAugmentations: LiteralExpression[];

for (const node of file.statements) {
collect(node, /*allowRelativeModuleNames*/ true, /*collectOnlyRequireCalls*/ false);
collectModuleReferences(node, /*inAmbientModule*/ false);
if (isJavaScriptFile) {
collectRequireCalls(node);
}
}

file.imports = imports || emptyArray;
file.moduleAugmentations = moduleAugmentations || emptyArray;

return;

function collect(node: Node, allowRelativeModuleNames: boolean, collectOnlyRequireCalls: boolean): void {
if (!collectOnlyRequireCalls) {
switch (node.kind) {
case SyntaxKind.ImportDeclaration:
case SyntaxKind.ImportEqualsDeclaration:
case SyntaxKind.ExportDeclaration:
let moduleNameExpr = getExternalModuleName(node);
if (!moduleNameExpr || moduleNameExpr.kind !== SyntaxKind.StringLiteral) {
break;
}
if (!(<LiteralExpression>moduleNameExpr).text) {
break;
}
function collectModuleReferences(node: Node, inAmbientModule: boolean): void {
switch (node.kind) {
case SyntaxKind.ImportDeclaration:
case SyntaxKind.ImportEqualsDeclaration:
case SyntaxKind.ExportDeclaration:
let moduleNameExpr = getExternalModuleName(node);
if (!moduleNameExpr || moduleNameExpr.kind !== SyntaxKind.StringLiteral) {
break;
}
if (!(<LiteralExpression>moduleNameExpr).text) {
break;
}

if (allowRelativeModuleNames || !isExternalModuleNameRelative((<LiteralExpression>moduleNameExpr).text)) {
(imports || (imports = [])).push(<LiteralExpression>moduleNameExpr);
// TypeScript 1.0 spec (April 2014): 12.1.6
// An ExternalImportDeclaration in an AmbientExternalModuleDeclaration may reference other external modules
// only through top - level external module names. Relative external module names are not permitted.
if (!inAmbientModule || !isExternalModuleNameRelative((<LiteralExpression>moduleNameExpr).text)) {
(imports || (imports = [])).push(<LiteralExpression>moduleNameExpr);
}
break;
case SyntaxKind.ModuleDeclaration:
if ((<ModuleDeclaration>node).name.kind === SyntaxKind.StringLiteral && (inAmbientModule || node.flags & NodeFlags.Ambient || isDeclarationFile(file))) {
const moduleName = <LiteralExpression>(<ModuleDeclaration>node).name;
// 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
Copy link
Member

Choose a reason for hiding this comment

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

I think you mean "if the ambient module declaration is in a module file, then it is a module augmentation"

// - if current file is not external module then module augmentation is an ambient module declaration with non-relative module name
Copy link
Member

Choose a reason for hiding this comment

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

"if the current file is not a module (e.g. an ambient declaration file), then if the module name is non-relative, the declaration is a module augmentation"

// immediately nested in top level ambient module declaration .
if (isExternalModuleFile || (inAmbientModule && !isExternalModuleNameRelative(moduleName.text))) {
(moduleAugmentations || (moduleAugmentations = [])).push(moduleName);
}
break;
case SyntaxKind.ModuleDeclaration:
if ((<ModuleDeclaration>node).name.kind === SyntaxKind.StringLiteral && (node.flags & NodeFlags.Ambient || isDeclarationFile(file))) {
// TypeScript 1.0 spec (April 2014): 12.1.6
else if (!inAmbientModule) {
// An AmbientExternalModuleDeclaration declares an external module.
// This type of declaration is permitted only in the global module.
// The StringLiteral must specify a top - level external module name.
// Relative external module names are not permitted
forEachChild((<ModuleDeclaration>node).body, node => {
// TypeScript 1.0 spec (April 2014): 12.1.6
// An ExternalImportDeclaration in anAmbientExternalModuleDeclaration may reference other external modules
// only through top - level external module names. Relative external module names are not permitted.
collect(node, /*allowRelativeModuleNames*/ false, collectOnlyRequireCalls);
});

// NOTE: body of ambient module is always a module block
for (const statement of (<ModuleBlock>(<ModuleDeclaration>node).body).statements) {
collectModuleReferences(statement, /*inAmbientModule*/ true);
}
}
break;
}
}
}
}

if (isJavaScriptFile) {
if (isRequireCall(node)) {
(imports || (imports = [])).push(<StringLiteral>(<CallExpression>node).arguments[0]);
}
else {
forEachChild(node, node => collect(node, allowRelativeModuleNames, /*collectOnlyRequireCalls*/ true));
}
function collectRequireCalls(node: Node): void {
if (isRequireCall(node)) {
(imports || (imports = [])).push(<StringLiteral>(<CallExpression>node).arguments[0]);
}
else {
forEachChild(node, collectRequireCalls);
}
}
}
Expand Down Expand Up @@ -1069,14 +1112,28 @@ namespace ts {

function processImportedModules(file: SourceFile, basePath: string) {
collectExternalModuleReferences(file);
if (file.imports.length) {
if (file.imports.length || file.moduleAugmentations.length) {
file.resolvedModules = {};
const moduleNames = map(file.imports, name => name.text);
const moduleNames: string[] = [];
for (const name of file.imports) {
moduleNames.push(name.text);
}
for (const name of file.moduleAugmentations) {
moduleNames.push(name.text);
}
Copy link
Member

Choose a reason for hiding this comment

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

const moduleNames = map(concatenate(file.imports, file.moduleAugmentations), name => name.text);

const resolutions = resolveModuleNamesWorker(moduleNames, getNormalizedAbsolutePath(file.fileName, currentDirectory));
for (let i = 0; i < file.imports.length; ++i) {
for (let i = 0; i < moduleNames.length; ++i) {
const resolution = resolutions[i];
setResolvedModule(file, moduleNames[i], resolution);
if (resolution && !options.noResolve) {
// add file to program only if:
// - resolution was successfull
// - noResolve is falsy
// - module name come from the list fo imports
const shouldAddFile = resolution &&
!options.noResolve &&
i < file.imports.length;

if (shouldAddFile) {
const importedFile = findSourceFile(resolution.resolvedFileName, toPath(resolution.resolvedFileName, currentDirectory, getCanonicalFileName), /*isDefaultLib*/ false, file, skipTrivia(file.text, file.imports[i].pos), file.imports[i].end);

if (importedFile && resolution.isExternalLibraryImport) {
Expand Down
Loading