feedback from CR

weswigham · weswigham · commit 265fb518a8c3 · 2015-11-02T16:54:12.000-08:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -14908,10 +14908,23 @@ namespace ts {
                 getTypeReferenceSerializationKind,
                 isOptionalParameter,
                 isArgumentsLocalBinding,
-                getSymbolAtLocation
+                getExternalModuleFileFromDeclaration
             };
         }
 
+        function getExternalModuleFileFromDeclaration(declaration: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration): SourceFile {
+                const specifier = getExternalModuleName(declaration);
+                const moduleSymbol = getSymbolAtLocation(specifier);
+                if (!moduleSymbol) {
+                    return undefined;
+                }
+                const moduleDeclaration = getDeclarationOfKind(moduleSymbol, SyntaxKind.SourceFile) as SourceFile;
+                if (!moduleDeclaration) {
+                    return undefined;
+                }
+                return moduleDeclaration;
+        }
+
         function initializeTypeChecker() {
             // Bind all source files and propagate errors
             forEach(host.getSourceFiles(), file => {
diff --git a/src/compiler/declarationEmitter.ts b/src/compiler/declarationEmitter.ts
@@ -130,7 +130,7 @@ namespace ts {
                 }
                 else if (isExternalModule(sourceFile)) {
                     noDeclare = true;
-                    write(`declare module "${getModuleName(host, sourceFile)}" {`);
+                    write(`declare module "${getResolvedExternalModuleName(host, sourceFile)}" {`);
                     writeLine();
                     increaseIndent();
                     emitSourceFile(sourceFile);
@@ -738,16 +738,12 @@ namespace ts {
 
         function emitExternalModuleSpecifier(moduleSpecifier: Expression) {
             if (moduleSpecifier.kind === SyntaxKind.StringLiteral && (!root) && (compilerOptions.out || compilerOptions.outFile)) {
-                let moduleSymbol = resolver.getSymbolAtLocation(moduleSpecifier);
-                if (moduleSymbol) {
-                    let moduleDeclaration = getDeclarationOfKind(moduleSymbol, SyntaxKind.SourceFile) as SourceFile;
-                    if (moduleDeclaration && !isDeclarationFile(moduleDeclaration)) {
-                        let nonRelativeModuleName = getExternalModuleNameFromPath(host, moduleDeclaration.fileName);
-                        write("\"");
-                        write(nonRelativeModuleName);
-                        write("\"");
-                        return;
-                    }
+                let moduleName = getExternalModuleNameFromDeclaration(host, resolver, moduleSpecifier.parent as (ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration));
+                if (moduleName) {
+                    write("\"");
+                    write(moduleName);
+                    write("\"");
+                    return;
                 }
             }
 
diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -7,10 +7,18 @@ namespace ts {
         return isExternalModule(sourceFile) || isDeclarationFile(sourceFile);
     }
 
-    export function getModuleName(host: EmitHost, file: SourceFile): string {
+    export function getResolvedExternalModuleName(host: EmitHost, file: SourceFile): string {
         return file.moduleName || getExternalModuleNameFromPath(host, file.fileName);
     }
 
+    export function getExternalModuleNameFromDeclaration(host: EmitHost, resolver: EmitResolver, declaration: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration): string {
+        let file = resolver.getExternalModuleFileFromDeclaration(declaration);
+        if (!file || isDeclarationFile(file)) {
+            return undefined;
+        }
+        return getResolvedExternalModuleName(host, file);
+    }
+
     type DependencyGroup = Array<ImportDeclaration | ImportEqualsDeclaration | ExportDeclaration>;
 
     let entities: Map<number> = {
@@ -553,15 +561,15 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
             /** If removeComments is true, no leading-comments needed to be emitted **/
             let emitLeadingCommentsOfPosition = compilerOptions.removeComments ? function (pos: number) { } : emitLeadingCommentsOfPositionWorker;
 
-            let moduleEmitDelegates: Map<(node: SourceFile, resolveModuleNames?: boolean) => void> = {
+            let moduleEmitDelegates: Map<(node: SourceFile, emitRelativePathAsModuleName?: boolean) => void> = {
                 [ModuleKind.ES6]: emitES6Module,
                 [ModuleKind.AMD]: emitAMDModule,
                 [ModuleKind.System]: emitSystemModule,
                 [ModuleKind.UMD]: emitUMDModule,
                 [ModuleKind.CommonJS]: emitCommonJSModule,
             };
 
-            let bundleEmitDelegates: Map<(node: SourceFile, resolveModuleNames?: boolean) => void> = {
+            let bundleEmitDelegates: Map<(node: SourceFile, emitRelativePathAsModuleName?: boolean) => void> = {
                 [ModuleKind.ES6]() {},
                 [ModuleKind.AMD]: emitAMDModule,
                 [ModuleKind.System]: emitSystemModule,
@@ -7275,14 +7283,14 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                 write("}"); // execute
             }
 
-            function writeModuleName(node: SourceFile, resolveModuleNames?: boolean): void {
+            function writeModuleName(node: SourceFile, emitRelativePathAsModuleName?: boolean): void {
                 let moduleName = node.moduleName;
-                if (moduleName || (resolveModuleNames && (moduleName = getModuleName(host, node)))) {
+                if (moduleName || (emitRelativePathAsModuleName && (moduleName = getResolvedExternalModuleName(host, node)))) {
                     write(`"${moduleName}", `);
                 }
             }
 
-            function emitSystemModule(node: SourceFile,  resolveModuleNames?: boolean): void {
+            function emitSystemModule(node: SourceFile,  emitRelativePathAsModuleName?: boolean): void {
                 collectExternalModuleInfo(node);
                 // System modules has the following shape
                 // System.register(['dep-1', ... 'dep-n'], function(exports) {/* module body function */})
@@ -7297,7 +7305,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                 exportFunctionForFile = makeUniqueName("exports");
                 writeLine();
                 write("System.register(");
-                writeModuleName(node, resolveModuleNames);
+                writeModuleName(node, emitRelativePathAsModuleName);
                 write("[");
 
                 let groupIndices: Map<number> = {};
@@ -7320,8 +7328,8 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                         write(", ");
                     }
 
-                    if (resolveModuleNames) {
-                        let name = lookupSpecifierName(externalImports[i]);
+                    if (emitRelativePathAsModuleName) {
+                        let name = getExternalModuleNameFromDeclaration(host, resolver, externalImports[i]);
                         if (name) {
                             text = `"${name}"`;
                         }
@@ -7340,32 +7348,13 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                 write("});");
             }
 
-            function lookupSpecifierName(declaration: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration): string {
-                let specifier: Node;
-                if (declaration.kind === SyntaxKind.ImportEqualsDeclaration) {
-                    specifier = (declaration as ImportEqualsDeclaration).moduleReference;
-                }
-                else {
-                    specifier = (declaration as ImportDeclaration | ExportDeclaration).moduleSpecifier;
-                }
-                let moduleSymbol = resolver.getSymbolAtLocation(specifier);
-                if (!moduleSymbol) {
-                    return undefined;
-                }
-                let moduleDeclaration = getDeclarationOfKind(moduleSymbol, SyntaxKind.SourceFile) as SourceFile;
-                if (!moduleDeclaration || isDeclarationFile(moduleDeclaration)) {
-                    return undefined;
-                }
-                return getExternalModuleNameFromPath(host, moduleDeclaration.fileName);
-            }
-
             interface AMDDependencyNames {
                 aliasedModuleNames: string[];
                 unaliasedModuleNames: string[];
                 importAliasNames: string[];
             }
 
-            function getAMDDependencyNames(node: SourceFile, includeNonAmdDependencies: boolean, resolveModuleNames?: boolean): AMDDependencyNames {
+            function getAMDDependencyNames(node: SourceFile, includeNonAmdDependencies: boolean, emitRelativePathAsModuleName?: boolean): AMDDependencyNames {
                 // names of modules with corresponding parameter in the factory function
                 let aliasedModuleNames: string[] = [];
                 // names of modules with no corresponding parameters in factory function
@@ -7389,8 +7378,8 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                     // Find the name of the external module
                     let externalModuleName = getExternalModuleNameText(importNode);
 
-                    if (resolveModuleNames) {
-                        let name = lookupSpecifierName(importNode);
+                    if (emitRelativePathAsModuleName) {
+                        let name = getExternalModuleNameFromDeclaration(host, resolver, importNode);
                         if (name) {
                             externalModuleName = `"${name}"`;
                         }
@@ -7410,7 +7399,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                 return { aliasedModuleNames, unaliasedModuleNames, importAliasNames };
             }
 
-            function emitAMDDependencies(node: SourceFile, includeNonAmdDependencies: boolean, resolveModuleNames?: boolean) {
+            function emitAMDDependencies(node: SourceFile, includeNonAmdDependencies: boolean, emitRelativePathAsModuleName?: boolean) {
                 // An AMD define function has the following shape:
                 //     define(id?, dependencies?, factory);
                 //
@@ -7423,7 +7412,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                 // `import "module"` or `<amd-dependency path= "a.css" />`
                 // we need to add modules without alias names to the end of the dependencies list
 
-                let dependencyNames = getAMDDependencyNames(node, includeNonAmdDependencies, resolveModuleNames);
+                let dependencyNames = getAMDDependencyNames(node, includeNonAmdDependencies, emitRelativePathAsModuleName);
                 emitAMDDependencyList(dependencyNames);
                 write(", ");
                 emitAMDFactoryHeader(dependencyNames);
@@ -7451,14 +7440,14 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                 write(") {");
             }
 
-            function emitAMDModule(node: SourceFile, resolveModuleNames?: boolean) {
+            function emitAMDModule(node: SourceFile, emitRelativePathAsModuleName?: boolean) {
                 emitEmitHelpers(node);
                 collectExternalModuleInfo(node);
 
                 writeLine();
                 write("define(");
-                writeModuleName(node, resolveModuleNames);
-                emitAMDDependencies(node, /*includeNonAmdDependencies*/ true, resolveModuleNames);
+                writeModuleName(node, emitRelativePathAsModuleName);
+                emitAMDDependencies(node, /*includeNonAmdDependencies*/ true, emitRelativePathAsModuleName);
                 increaseIndent();
                 let startIndex = emitDirectivePrologues(node.statements, /*startWithNewLine*/ true);
                 emitExportStarHelper();
@@ -7712,7 +7701,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                         emitModule(node);
                     }
                     else {
-                        bundleEmitDelegates[modulekind](node, /*resolveModuleNames*/true);
+                        bundleEmitDelegates[modulekind](node, /*emitRelativePathAsModuleName*/true);
                     }
                 }
                 else {
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -1619,7 +1619,7 @@ namespace ts {
         getTypeReferenceSerializationKind(typeName: EntityName): TypeReferenceSerializationKind;
         isOptionalParameter(node: ParameterDeclaration): boolean;
         isArgumentsLocalBinding(node: Identifier): boolean;
-        getSymbolAtLocation(node: Node): Symbol;
+        getExternalModuleFileFromDeclaration(declaration: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration): SourceFile;
     }
 
     export const enum SymbolFlags {

Original file line number	Diff line number	Diff line change
`@@ -1619,7 +1619,7 @@ namespace ts {`
`1619`	`1619`	`getTypeReferenceSerializationKind(typeName: EntityName): TypeReferenceSerializationKind;`
`1620`	`1620`	`isOptionalParameter(node: ParameterDeclaration): boolean;`
`1621`	`1621`	`isArgumentsLocalBinding(node: Identifier): boolean;`
`1622`		`- getSymbolAtLocation(node: Node): Symbol;`
	`1622`	`+ getExternalModuleFileFromDeclaration(declaration: ImportEqualsDeclaration \| ImportDeclaration \| ExportDeclaration): SourceFile;`
`1623`	`1623`	`}`
`1624`	`1624`
`1625`	`1625`	`export const enum SymbolFlags {`