Improved handling of module names (dart-archive#61)

Added support for names that contain dots
Refactored tests to use the namespace keyword as that is preferred for internal modules

lib/base.ts

@@ -307,6 +307,31 @@ export function isInsideConstExpr(node: ts.Node): boolean {
   return isConstCall(<ts.CallExpression>getAncestor(node, ts.SyntaxKind.CallExpression));
 }
 
+export function getModuleBlock(moduleDecl: ts.ModuleDeclaration): ts.ModuleBlock {
+  while (ts.isModuleDeclaration(moduleDecl.body)) {
+    moduleDecl = moduleDecl.body;
+  }
+  if (ts.isModuleBlock(moduleDecl.body)) {
+    return moduleDecl.body;
+  } else {
+    throw new Error('Module body must be a module block.');
+  }
+}
+
+/**
+ * Determine the full module name including dots.
+ *
+ * e.g. returns 'foo.bar' for a declaration of namespace or module foo.bar
+ */
+export function getModuleName(moduleDecl: ts.ModuleDeclaration): string {
+  let name = moduleDecl.name.text;
+  while (ts.isModuleDeclaration(moduleDecl.body)) {
+    moduleDecl = moduleDecl.body;
+    name += '.' + moduleDecl.name.text;
+  }
+  return name;
+}
+
 export function formatType(s: string, comment: string, options: TypeDisplayOptions): string {
   if (!comment || options.hideComment) {
     return s;

lib/declaration.ts

@@ -49,7 +49,7 @@ export default class DeclarationTranspiler extends base.TranspilerBase {
   }
 
   getJsPath(node: ts.Node, suppressUnneededPaths: boolean): string {
-    let path: Array<String> = [];
+    const path: Array<String> = [];
     let moduleDecl =
         base.getAncestor(node, ts.SyntaxKind.ModuleDeclaration) as ts.ModuleDeclaration;
     while (moduleDecl != null) {
@@ -75,7 +75,7 @@ export default class DeclarationTranspiler extends base.TranspilerBase {
     }
 
     if (ts.isModuleDeclaration(node)) {
-      path.push(node.name.text);
+      path.push(base.getModuleName(node));
     } else if (ts.isClassDeclaration(node) || ts.isInterfaceDeclaration(node)) {
       // Already handled by call to getEnclosingClass.
     } else if (ts.isEnumDeclaration(node)) {
@@ -439,16 +439,18 @@ export default class DeclarationTranspiler extends base.TranspilerBase {
   visitNode(node: ts.Node): boolean {
     switch (node.kind) {
       case ts.SyntaxKind.ModuleDeclaration:
-        let moduleDecl = <ts.ModuleDeclaration>node;
-        if (moduleDecl.name.text.slice(0, 2) === '..') {
+        const moduleDecl = <ts.ModuleDeclaration>node;
+        const moduleName = base.getModuleName(moduleDecl);
+        const moduleBlock = base.getModuleBlock(moduleDecl);
+        if (moduleName.slice(0, 2) === '..') {
           this.emit(
               '\n// Library augmentation not allowed by Dart. Ignoring augmentation of ' +
               moduleDecl.name.text + '\n');
           break;
         }
-        this.emit('\n// Module ' + moduleDecl.name.text + '\n');
-        this.visit(moduleDecl.body);
-        this.emit('\n// End module ' + moduleDecl.name.text + '\n');
+        this.emit('\n// Module ' + moduleName + '\n');
+        this.visit(moduleBlock);
+        this.emit('\n// End module ' + moduleName + '\n');
         break;
       case ts.SyntaxKind.ExportKeyword:
         // TODO(jacobr): perhaps add a specific Dart annotation to indicate

lib/facade_converter.ts

@@ -49,13 +49,11 @@ function hasVarArgs(parameters: ts.ParameterDeclaration[]): boolean {
  * Path: m1.m2.foo
  */
 function fullJsPath(node: base.NamedDeclaration): string {
-  let parts: Array<string> = [base.ident(node.name)];
+  const parts: Array<string> = [base.ident(node.name)];
   let p: ts.Node = node.parent;
   while (p != null) {
-    let kind = p.kind;
-    if (kind === ts.SyntaxKind.ModuleDeclaration || kind === ts.SyntaxKind.InterfaceDeclaration ||
-        kind === ts.SyntaxKind.ClassDeclaration) {
-      parts.unshift(base.ident((<base.NamedDeclaration>p).name));
+    if (ts.isModuleDeclaration(p) || ts.isInterfaceDeclaration(p) || ts.isClassDeclaration(p)) {
+      parts.unshift(base.ident(p.name));
     }
     p = p.parent;
   }
@@ -97,20 +95,20 @@ export class NameRewriter {
   constructor(private fc: FacadeConverter) {}
 
   private computeName(node: base.NamedDeclaration): DartNameRecord {
-    let fullPath = fullJsPath(node);
+    const fullPath = fullJsPath(node);
     if (this.dartTypes.has(fullPath)) {
       return this.dartTypes.get(fullPath);
     }
-    let sourceFile = <ts.SourceFile>base.getAncestor(node, ts.SyntaxKind.SourceFile);
-    let fileName = sourceFile.fileName;
+    const sourceFile = <ts.SourceFile>base.getAncestor(node, ts.SyntaxKind.SourceFile);
+    const fileName = sourceFile.fileName;
     let library: DartLibrary;
     if (this.libraries.has(fileName)) {
       library = this.libraries.get(fileName);
     } else {
       library = new DartLibrary(fileName);
       this.libraries.set(fileName, library);
     }
-    let parts = fullPath.split('.');
+    const parts = fullPath.split('.');
     for (let i = parts.length - 1; i >= 0; i--) {
       // Find a unique name by including more of the module hierarchy in the
       // name. This is an arbitrary but hopefully unsurprising scheme to

lib/main.ts

@@ -189,10 +189,10 @@ export class Transpiler {
     });
 
     sourceFiles.forEach((f: ts.SourceFile) => {
-      let dartCode = this.translate(f);
+      const dartCode = this.translate(f);
 
       if (destination) {
-        let outputFile = this.getOutputPath(path.resolve(f.fileName), destination);
+        const outputFile = this.getOutputPath(path.resolve(f.fileName), destination);
         console.log('Output file:', outputFile);
         mkdirP(path.dirname(outputFile));
         fs.writeFileSync(outputFile, dartCode);

lib/merge.ts

@@ -248,31 +248,17 @@ export function normalizeSourceFile(f: ts.SourceFile, fc: FacadeConverter, expli
 
   // Merge top level modules.
   for (let i = 0; i < f.statements.length; ++i) {
-    let statement = f.statements[i];
-    if (statement.kind !== ts.SyntaxKind.ModuleDeclaration) continue;
-    let moduleDecl = <ts.ModuleDeclaration>statement;
-    let name = moduleDecl.name.text;
+    const statement = f.statements[i];
+    if (!ts.isModuleDeclaration(statement)) {
+      continue;
+    }
+    const moduleDecl: ts.ModuleDeclaration = statement;
+    const name = base.getModuleName(moduleDecl);
+    const moduleBlock = base.getModuleBlock(moduleDecl);
     if (modules.has(name)) {
-      let srcBody = modules.get(name).body;
-      let srcBodyBlock: ts.ModuleBlock;
-
-      if (srcBody.kind !== ts.SyntaxKind.ModuleBlock) {
-        throw 'Module body must be a module block.';
-      }
-      srcBodyBlock = <ts.ModuleBlock>srcBody;
-
-      let body = moduleDecl.body;
-      if (body.kind === ts.SyntaxKind.ModuleBlock) {
-        let bodyBlock = <ts.ModuleBlock>body;
-        Array.prototype.push.apply(srcBodyBlock.statements, bodyBlock.statements);
-      } else {
-        // moduleDecl.body is a ModuleDeclaration.
-        // Small hack to get around NodeArrays being readonly
-        Array.prototype.push.call(srcBodyBlock.statements, moduleDecl.body);
-      }
+      const srcBodyBlock = base.getModuleBlock(modules.get(name));
 
-      Array.prototype.splice.call(f.statements, i, 1);
-      i--;
+      Array.prototype.push.apply(srcBodyBlock.statements, moduleBlock.statements);
     } else {
       modules.set(name, moduleDecl);
     }

test/declaration_test.ts

@@ -715,7 +715,7 @@ interface XStatic {
   foo();
 }
 
-declare module Foo {
+declare namespace Foo {
   declare var X: XStatic;
 }
 `).to.equal(`@JS("Foo.X")

test/js_interop_test.ts

@@ -677,10 +677,10 @@ class Color {
 describe('renames', () => {
   it('should support class renames', () => {
     expectTranslate(`
-declare module m1 {
+declare namespace m1 {
   interface A { x(); }
 }
-declare module m2 {
+declare namespace m2 {
   interface A { y(); }
 }
 `).to.equal(`// Module m1
@@ -701,10 +701,33 @@ abstract class m2_A {
 
 // End module m2`);
     expectTranslate(`
-declare module m1 {
+    declare namespace foo.m1 {
+      function x(): number;
+    }
+    declare namespace foo.m2 {
+      function x(): string;
+    }
+    declare namespace m2 {
+      function x(): string[];
+    }`).to.equal(`// Module foo.m1
+@JS("foo.m1.x")
+external num x();
+// End module foo.m1
+
+// Module foo.m2
+@JS("foo.m2.x")
+external String m2_x();
+// End module foo.m2
+
+// Module m2
+@JS("m2.x")
+external List<String> x2();
+// End module m2`);
+    expectTranslate(`
+declare namespace m1 {
   class A { constructor(x); }
 }
-declare module m2 {
+declare namespace m2 {
   class A { constructor(y); }
 }`).to.equal(`// Module m1
 @JS("m1.A")
@@ -726,10 +749,10 @@ class m2_A {
 
 // End module m2`);
     expectTranslate(`
-declare module m1 {
+declare namespace m1 {
   class A { constructor(x:m2.A); }
 }
-declare module m2 {
+declare namespace m2 {
   class A { constructor(y:m1.A); }
 }
 `).to.equal(`// Module m1
@@ -754,10 +777,10 @@ class m2_A {
   });
   it('should support member renames', () => {
     expectTranslate(`
-declare module m1 {
+declare namespace m1 {
   interface A { x(); }
 }
-declare module m2 {
+declare namespace m2 {
   export function A(x:m1.A);
 }`).to.equal(`// Module m1
 @anonymous
@@ -776,10 +799,10 @@ external m2_A(A x);
 
   it('handle class renames in type declarations', () => {
     expectTranslate(`
-declare module m1 {
+declare namespace m1 {
   interface A { x(); }
 }
-declare module m2 {
+declare namespace m2 {
   interface A { y(); }
 }
 export function register(x:m2.A);
@@ -803,18 +826,18 @@ abstract class m2_A {
 @JS()
 external register(m2_A x);`);
     expectTranslate(`
-declare module m1 {
-  module foo {
+declare namespace m1 {
+  namespace foo {
     interface A { x(); }
   }
 }
-declare module m2 {
-  module foo {
+declare namespace m2 {
+  namespace foo {
     interface A { y(); }
   }
 }
-declare module m3 {
-  module foo {
+declare namespace m3 {
+  namespace foo {
     interface A { z(); }
   }
 }
@@ -861,10 +884,10 @@ abstract class m3_foo_A {
 external register(foo_A y, m3_foo_A z);`);
 
     expectTranslate(`
-declare module m1 {
+declare namespace m1 {
   interface A { x(); }
 }
-declare module m2 {
+declare namespace m2 {
   interface A { y(); }
 }
 export function register(x:m1.A);