JS Interop Facade generation polish.

lib/facade_converter.ts

Index: lib/facade_converter.ts
diff --git a/lib/facade_converter.ts b/lib/facade_converter.ts
index c43dda92e5216def334104a373d7d09e45d13ab0..2ffaf9941a0e00f805d9ec2576fb84eb2b5ae4d9 100644
--- a/lib/facade_converter.ts
+++ b/lib/facade_converter.ts
@@ -1,7 +1,7 @@
 import * as ts from 'typescript';
 
 import * as base from './base';
-import {Set} from './base';
+import {Set, TypeDisplayOptions} from './base';
 import {DART_LIBRARIES_FOR_BROWSER_TYPES, TS_TO_DART_TYPENAMES} from './dart_libraries_for_browser_types';
 import {Transpiler} from './main';
 import {MergedType} from './merge';
@@ -22,23 +22,23 @@ const MAX_DART_FUNC_ACTION_PARAMETERS_OPTIONAL = 1;
  * Prefix to add to a variable name that leaves the JS name referenced
  * unchanged.
  */
-const DART_RESERVED_NAME_PREFIX = 'JS$';
+export const DART_RESERVED_NAME_PREFIX = 'JS$';
 
 export function fixupIdentifierName(text: string): string {
   return (FacadeConverter.DART_RESERVED_WORDS.indexOf(text) !== -1 ||
-          FacadeConverter.DART_OTHER_KEYWORDS.indexOf(text) !== -1 || text[0] === '_') ?
+          FacadeConverter.DART_OTHER_KEYWORDS.indexOf(text) !== -1 || text.match(/^(\d|_)/)) ?
       DART_RESERVED_NAME_PREFIX + text :
       text;
 }
 
-function numOptionalParameters(parameters: ts.NodeArray<ts.ParameterDeclaration>): number {
+function numOptionalParameters(parameters: ts.ParameterDeclaration[]): number {
   for (let i = 0; i < parameters.length; ++i) {
     if (parameters[i].questionToken) return parameters.length - i;
   }
   return 0;
 }
 
-function hasVarArgs(parameters: ts.NodeArray<ts.ParameterDeclaration>): boolean {
+function hasVarArgs(parameters: ts.ParameterDeclaration[]): boolean {
   for (let i = 0; i < parameters.length; ++i) {
     if (parameters[i].dotDotDotToken) return true;
   }
@@ -161,8 +161,65 @@ export class NameRewriter {
   lookupName(node: base.NamedDeclaration, context: ts.Node) { return this.computeName(node).name; }
 }
 
+function cloneOptions(options: TypeDisplayOptions): TypeDisplayOptions {
+  return {
+    insideComment: options.insideComment,
+    insideTypeArgument: options.insideTypeArgument,
+    hideComment: options.hideComment,
+    typeArguments: options.typeArguments,
+    resolvedTypeArguments: options.resolvedTypeArguments
+  };
+}
+
+function addInsideTypeArgument(options: TypeDisplayOptions): TypeDisplayOptions {
+  let ret = cloneOptions(options);
+  ret.insideTypeArgument = true;
+  return ret;
+}
+
+function addInsideComment(options: TypeDisplayOptions): TypeDisplayOptions {
+  let ret = cloneOptions(options);
+  ret.insideComment = true;
+  return ret;
+}
+
+function addHideComment(options: TypeDisplayOptions): TypeDisplayOptions {
+  let ret = cloneOptions(options);
+  ret.hideComment = true;
+  return ret;
+}
+
+function setTypeArguments(
+    options: TypeDisplayOptions, typeArguments: ts.TypeNode[]): TypeDisplayOptions {
+  let ret = cloneOptions(options);
+  ret.typeArguments = typeArguments;
+  return ret;
+}
+
+function resolveTypeArguments(
+    options: TypeDisplayOptions, parameters: ts.TypeParameterDeclaration[]) {
+  let ret = cloneOptions(options);
+  let typeArguments = options.typeArguments ? options.typeArguments : [];
+  ret.resolvedTypeArguments = {};
+  if (parameters) {
+    for (let i = 0; i < parameters.length; ++i) {
+      let param = parameters[i];
+      ret.resolvedTypeArguments[base.ident(param.name)] = typeArguments[i];
+    }
+  }
+  // Type arguments have been resolved forward so we don't need to emit them directly.
+  ret.typeArguments = null;
+  return ret;
+}
+
+function removeResolvedTypeArguments(options: TypeDisplayOptions): TypeDisplayOptions {
+  let ret = cloneOptions(options);
+  ret.resolvedTypeArguments = null;
+  return ret;
+}
+
 export class FacadeConverter extends base.TranspilerBase {
-  private tc: ts.TypeChecker;
+  tc: ts.TypeChecker;
   // For the Dart keyword list see
   // https://www.dartlang.org/docs/dart-up-and-running/ch02.html#keywords
   static DART_RESERVED_WORDS =
@@ -177,21 +234,31 @@ export class FacadeConverter extends base.TranspilerBase {
        'library operator part set static sync typedef yield call')
           .split(/ /);
 
+  // TODO(jacobr): add separate list of members of the core Dart object class and core Dart List
+  // class that need to be escaped with JS$ when being entered as properties.
+  // As methods on Object and List will take priorty over JS subclasses.
+  // Note that toString is fine as the default implementation is compatible with JavaScript.
+  static DART_OBJECT_MEMBERS = ('hashCode, runtimeType noSuchMethod').split(/ /);
+
+  static DART_LIST_MEMBERS =
+      ('first hashCode isEmpty isNotEmpty iterator last length reversed runtimeType single add ' +
+       'addAll any asMap clear contains elementAt every expand fillRange firstWhere fold forEach ' +
+       'getRange indexOf insert insertAll join lastIndexOf lastWhere map noSuchMethod reduce ' +
+       'remove removeAt removeLast removeRange removeWhere replaceRange retainWhere setAll ' +
+       'setRange shuffle singleWhere skip skipWhile sort sublist take takeWhile toList toSet ' +
+       'toString where')
+          .split(/ /);
+
   private candidateTypes: {[typeName: string]: boolean} = {};
   private typingsRootRegex: RegExp;
   private genericMethodDeclDepth = 0;
 
-  constructor(
-      transpiler: Transpiler, typingsRoot = '', private nameRewriter: NameRewriter,
-      private useHtml: boolean) {
+  constructor(transpiler: Transpiler, typingsRoot = '', private nameRewriter: NameRewriter) {
     super(transpiler);
-    if (useHtml) {
-      this.extractPropertyNames(TS_TO_DART_TYPENAMES, this.candidateTypes);
-      Object.keys(DART_LIBRARIES_FOR_BROWSER_TYPES)
-          .forEach((propName) => this.candidateTypes[propName] = true);
-    } else {
-      this.extractPropertyNames(TS_TO_DART_TYPENAMES, this.candidateTypes);
-    }
+    this.extractPropertyNames(TS_TO_DART_TYPENAMES, this.candidateTypes);
+    // Remove this line if decide to support generating code that avoids dart:html.
+    Object.keys(DART_LIBRARIES_FOR_BROWSER_TYPES)
+        .forEach((propName) => this.candidateTypes[propName] = true);
 
     this.typingsRootRegex = new RegExp('^' + typingsRoot.replace('.', '\\.'));
   }
@@ -262,52 +329,49 @@ export class FacadeConverter extends base.TranspilerBase {
     return symbol.declarations.some(d => d.parent.kind === ts.SyntaxKind.FunctionDeclaration);
   }
 
-  generateTypeList(types: ts.TypeNode[], insideComment: boolean, seperator = ','): string {
+  generateTypeList(types: ts.TypeNode[], options: TypeDisplayOptions, seperator = ','): string {
     let that = this;
-    return types.map((type) => { return that.generateDartTypeName(type, insideComment); })
+    return types
+        .map((type) => { return that.generateDartTypeName(type, addInsideTypeArgument(options)); })
         .join(seperator);
   }
 
-  maybeGenerateTypeArguments(
-      n: {typeArguments?: ts.NodeArray<ts.TypeNode>}, insideComment: boolean): string {
-    if (!n.typeArguments) return '';
-    return '<' + this.generateTypeList(n.typeArguments, insideComment) + '>';
-  }
+  generateDartTypeName(node: ts.TypeNode, options?: TypeDisplayOptions): string {
+    if (!options) {
+      options = {
+        insideComment: this.insideCodeComment,
+        insideTypeArgument: this.insideTypeArgument
+      };
+    }
 
-  generateDartTypeName(node: ts.TypeNode, insideComment: boolean): string {
     let name: string;
     let comment: string;
     if (!node) {
       return 'dynamic';
     }
+
     switch (node.kind) {
       case ts.SyntaxKind.TypeQuery:
-        let query = <ts.TypeQueryNode>node;
         name = 'dynamic';
-        name += '/* Dart does not support TypeQuery: typeof ' + base.ident(query.exprName) + ' */';
+        // TODO(jacobr): evaluate supporting this case.
+        // let query = <ts.TypeQueryNode>node;
+        // name += '/* TypeQuery: typeof ' + base.ident(query.exprName) + ' */';
         break;
-      case ts.SyntaxKind.LastTypeNode:
-        let type = (node as ts.ParenthesizedTypeNode).type;
-        if (!type) {
-          // This case occurs for String literal types
-          comment = node.getText();
-          // TODO(jacobr): find a better way to detect string literal types.
-          name = comment[0] === '"' ? 'String' : 'dynamic';
-          break;
-        }
-        return this.generateDartTypeName(type, insideComment);
       case ts.SyntaxKind.TypePredicate:
-        return this.generateDartTypeName((node as ts.TypePredicateNode).type, insideComment);
+        return this.generateDartTypeName((node as ts.TypePredicateNode).type, options);
       case ts.SyntaxKind.TupleType:
         let tuple = <ts.TupleTypeNode>node;
         name = 'List<';
         let mergedType = new MergedType(this);
         tuple.elementTypes.forEach((t) => mergedType.merge(t));
-        name += this.generateDartTypeName(mergedType.toTypeNode(), insideComment);
+        name += this.generateDartTypeName(mergedType.toTypeNode(), addInsideTypeArgument(options));
         name += '>';
-        comment = 'Tuple<' + this.generateTypeList(tuple.elementTypes, insideComment) + '>';
+        // Intentionally ensure this comment is not valid Dart code so we do not use the /*=
+        // syntax and so that it is clear this isn't a true Dart code comment.
+        comment = 'Tuple of <' +
+            this.generateTypeList(tuple.elementTypes, addInsideComment(options)) + '>';
         break;
-      case ts.SyntaxKind.UnionType:
+      case ts.SyntaxKind.UnionType: {
         let union = <ts.UnionTypeNode>node;
         // TODO(jacobr): this isn't fundamentally JS Interop specific but we
         // choose to be more aggressive at finding a useful value for the
@@ -315,19 +379,34 @@ export class FacadeConverter extends base.TranspilerBase {
         // types will not be used extensively.
         let simpleType = this.toSimpleDartType(union.types);
         if (simpleType) {
-          name = this.generateDartTypeName(simpleType, insideComment);
+          name = this.generateDartTypeName(simpleType, addHideComment(options));
         } else {
           name = 'dynamic';
         }
         let types = union.types;
-        comment = this.generateTypeList(types, true, '|');
-        break;
+        comment = this.generateTypeList(types, addInsideComment(options), '|');
+      } break;
+      case ts.SyntaxKind.IntersectionType: {
+        let intersection = <ts.IntersectionTypeNode>node;
+        // TODO(jacobr): this isn't fundamentally JS Interop specific but we
+        // choose to be more aggressive at finding a useful value for the
+        // union when in JS Interop mode while otherwise we expect that union
+        // types will not be used extensively.
+        let simpleType = this.toSimpleDartType(intersection.types);
+        if (simpleType) {
+          name = this.generateDartTypeName(simpleType, addHideComment(options));
+        } else {
+          name = 'dynamic';
+        }
+        let types = intersection.types;
+        comment = this.generateTypeList(types, addInsideComment(options), '&');
+      } break;
       case ts.SyntaxKind.TypePredicate:
-        return this.generateDartTypeName((node as ts.TypePredicateNode).type, insideComment);
+        return this.generateDartTypeName((node as ts.TypePredicateNode).type, options);
       case ts.SyntaxKind.TypeReference:
         let typeRef = <ts.TypeReferenceNode>node;
-        name = this.generateDartName(typeRef.typeName, insideComment) +
-            this.maybeGenerateTypeArguments(typeRef, insideComment);
+        name = this.generateDartName(
+            typeRef.typeName, setTypeArguments(options, typeRef.typeArguments));
         break;
       case ts.SyntaxKind.TypeLiteral:
         let members = (<ts.TypeLiteralNode>node).members;
@@ -341,8 +420,9 @@ export class FacadeConverter extends base.TranspilerBase {
           // if we define a base JSMap type that is Map like but not actually
           // a map.
           name = 'dynamic';
-          comment = 'JSMap of <' + this.generateDartTypeName(indexSig.parameters[0].type, true) +
-              ',' + this.generateDartTypeName(indexSig.type, true) + '>';
+          comment = 'JSMap of <' +
+              this.generateDartTypeName(indexSig.parameters[0].type, addInsideComment(options)) +
+              ',' + this.generateDartTypeName(indexSig.type, addInsideComment(options)) + '>';
         } else {
           name = 'dynamic';
           comment = node.getText();
@@ -350,8 +430,11 @@ export class FacadeConverter extends base.TranspilerBase {
         break;
       case ts.SyntaxKind.FunctionType:
         let callSignature = <ts.FunctionOrConstructorTypeNode>node;
-        let parameters = callSignature.parameters;
-
+        // TODO(jacobr): instead of removing the expected type of the this parameter, we could add
+        // seperate VoidFuncBindThis and FuncBindThis typedefs to package:func/func.dart if we
+        // decide indicating the parameter type of the bound this is useful enough. As JavaScript
+        // is moving away from binding this
+        let parameters = base.filterThisParameter(callSignature.parameters);
         // Use a function signature from package:func where possible.
         let numOptional = numOptionalParameters(parameters);
         let isVoid = callSignature.type && callSignature.type.kind === ts.SyntaxKind.VoidKeyword;
@@ -359,7 +442,7 @@ export class FacadeConverter extends base.TranspilerBase {
             numOptional <= MAX_DART_FUNC_ACTION_PARAMETERS_OPTIONAL && !hasVarArgs(parameters)) {
           this.emitImport('package:func/func.dart');
           let typeDefName = (isVoid) ? 'VoidFunc' : 'Func';
-          typeDefName += parameters.length;
+          typeDefName += parameters.length.toString();
           if (numOptional > 0) {
             typeDefName += 'Opt' + numOptional;
           }
@@ -375,13 +458,13 @@ export class FacadeConverter extends base.TranspilerBase {
             } else {
               name += ', ';
             }
-            name += this.generateDartTypeName(parameters[i].type, insideComment);
+            name += this.generateDartTypeName(parameters[i].type, addInsideTypeArgument(options));
           }
           if (!isVoid) {
             if (!isFirst) {
               name += ', ';
             }
-            name += this.generateDartTypeName(callSignature.type, insideComment);
+            name += this.generateDartTypeName(callSignature.type, addInsideTypeArgument(options));
           }
           if (numArgs > 0) {
             name += '>';
@@ -395,18 +478,33 @@ export class FacadeConverter extends base.TranspilerBase {
         break;
       case ts.SyntaxKind.ArrayType:
         name = 'List' +
-            '<' + this.generateDartTypeName((<ts.ArrayTypeNode>node).elementType, insideComment) +
+            '<' +
+            this.generateDartTypeName(
+                (<ts.ArrayTypeNode>node).elementType, addInsideTypeArgument(options)) +
             '>';
         break;
       case ts.SyntaxKind.NumberKeyword:
         name = 'num';
         break;
+      case ts.SyntaxKind.StringLiteralType:
+        comment = '\'' + (node as ts.StringLiteralTypeNode).text + '\'';
+        name = 'String';
+        break;
       case ts.SyntaxKind.StringLiteral:
       case ts.SyntaxKind.StringKeyword:
         name = 'String';
         break;
+      case ts.SyntaxKind.NullKeyword:
+        name = 'Null';
+        break;
+      case ts.SyntaxKind.UndefinedKeyword:
+        // TODO(jacobr): unclear if this should be Null or dynamic.
+        name = 'dynamic';
+        break;
       case ts.SyntaxKind.VoidKeyword:
-        name = 'void';
+        // void cannot be used as a type argument in Dart so we fall back to Object if void is
+        // unfortunately specified as a type argument.
+        name = options.insideTypeArgument ? 'Null' : 'void';
         break;
       case ts.SyntaxKind.BooleanKeyword:
         name = 'bool';
@@ -414,15 +512,19 @@ export class FacadeConverter extends base.TranspilerBase {
       case ts.SyntaxKind.AnyKeyword:
         name = 'dynamic';
         break;
+      case ts.SyntaxKind.ParenthesizedType:
+        return this.generateDartTypeName((node as ts.ParenthesizedTypeNode).type, options);
+      case ts.SyntaxKind.ThisType:
+        return this.generateDartName(base.getEnclosingClass(node).name, options);
       default:
-        this.reportError(node, 'Unexpected TypeNode kind');
+        this.reportError(node, 'Unexpected TypeNode kind: ' + node.kind);
     }
     if (name == null) {
-      name = 'XXX NULLNAME';
+      name = 'ERROR_NULL_NAME';
     }
 
     name = name.trim();
-    return base.formatType(name, comment, insideComment);
+    return base.formatType(name, comment, options);
   }
 
   visitTypeName(typeName: ts.EntityName) {
@@ -438,7 +540,9 @@ export class FacadeConverter extends base.TranspilerBase {
       return;
     }
 
-    let custom = this.lookupCustomDartTypeName(<ts.Identifier>typeName, this.insideCodeComment);
+    let custom = this.lookupCustomDartTypeName(
+        <ts.Identifier>typeName,
+        {insideComment: this.insideCodeComment, insideTypeArgument: this.insideTypeArgument});
     if (custom) {
       if (custom.comment) {
         this.emitType(custom.name, custom.comment);
@@ -465,12 +569,15 @@ export class FacadeConverter extends base.TranspilerBase {
     return decl;
   }
 
-  generateDartName(identifier: ts.EntityName, insideComment: boolean): string {
-    let ret = this.lookupCustomDartTypeName(identifier, insideComment);
-    if (ret) return base.formatType(ret.name, ret.comment, insideComment);
-    // TODO(jacobr): handle library import prefixes better. This generally works
-    // but is somewhat fragile.
-    return base.ident(identifier);
+  generateDartName(identifier: ts.EntityName, options: TypeDisplayOptions): string {
+    let ret = this.lookupCustomDartTypeName(identifier, options);
+
+    if (ret) {
+      return base.formatType(ret.name, ret.comment, options);
+    }
+    // TODO(jacobr): handle library import prefixes more robustly. This generally works
+    // but is fragile.
+    return this.maybeAddTypeArguments(base.ident(identifier), options);
   }
 
   /**
@@ -492,22 +599,41 @@ export class FacadeConverter extends base.TranspilerBase {
     return declaration;
   }
 
+  maybeAddTypeArguments(name: string, options: TypeDisplayOptions): string {
+    if (options.typeArguments) {
+      name +=
+          '<' + this.generateTypeList(options.typeArguments, setTypeArguments(options, null)) + '>';
+    }
+    return name;
+  }
+
   /**
    * Returns a custom Dart type name or null if the type isn't a custom Dart
    * type.
    */
-  lookupCustomDartTypeName(identifier: ts.EntityName, insideComment: boolean):
+  lookupCustomDartTypeName(identifier: ts.EntityName, options?: TypeDisplayOptions):
       {name?: string, comment?: string, keep?: boolean} {
+    if (!options) {
+      options = {
+        insideComment: this.insideCodeComment,
+        insideTypeArgument: this.insideTypeArgument
+      };
+    }
     let ident = base.ident(identifier);
-    let symbol: ts.Symbol;
-    if (!this.tc) return null;
-
-    symbol = this.tc.getSymbolAtLocation(identifier);
+    let symbol: ts.Symbol = this.tc.getSymbolAtLocation(identifier);
     let declaration = this.getSymbolDeclaration(symbol, identifier);
     if (symbol && symbol.flags & ts.SymbolFlags.TypeParameter) {
       let kind = declaration.parent.kind;
+      if (options.resolvedTypeArguments &&
+          Object.hasOwnProperty.call(options.resolvedTypeArguments, ident)) {
+        return {
+          name: this.generateDartTypeName(
+              options.resolvedTypeArguments[ident], removeResolvedTypeArguments(options))
+        };
+      }
       // Only kinds of TypeParameters supported by Dart.
-      if (kind !== ts.SyntaxKind.ClassDeclaration && kind !== ts.SyntaxKind.InterfaceDeclaration) {
+      if (kind !== ts.SyntaxKind.ClassDeclaration && kind !== ts.SyntaxKind.InterfaceDeclaration &&
+          kind !== ts.SyntaxKind.TypeAliasDeclaration) {
         return {name: 'dynamic', comment: ident};
       }
     }
@@ -527,30 +653,40 @@ export class FacadeConverter extends base.TranspilerBase {
             this.emitImport(DART_LIBRARIES_FOR_BROWSER_TYPES[fileAndName.qname]);
           }
           if (fileSubs.hasOwnProperty(fileAndName.qname)) {
-            return {name: fileSubs[fileAndName.qname]};
+            return {name: this.maybeAddTypeArguments(fileSubs[fileAndName.qname], options)};
           }
           if (dartBrowserType) {
             // Not a rename but has a dart core libraries definition.
-            return {name: fileAndName.qname};
+            return {name: this.maybeAddTypeArguments(fileAndName.qname, options)};
           }
         }
       }
     }
     if (symbol) {
+      this.emitImportForSourceFile(declaration.getSourceFile(), identifier.getSourceFile());
       if (symbol.flags & ts.SymbolFlags.Enum) {
         // We can't treat JavaScript enums as Dart enums in this case.
-        return {name: 'num', comment: ident};
+        return {name: 'num', comment: 'enum ' + ident};
       }
       // TODO(jacobr): we could choose to only support type alais declarations
       // for JS interop but it seems handling type alaises is generally helpful
       // without much risk of generating confusing Dart code.
       if (declaration.kind === ts.SyntaxKind.TypeAliasDeclaration) {
         let alias = <ts.TypeAliasDeclaration>declaration;
-        if (alias.typeParameters) {
-          // We can handle this case but currently do not.
-          this.reportError(declaration, 'Type parameters for type alaises are not supported');
+        if (base.supportedAliasType(alias)) {
+          return {
+            name: this.maybeAddTypeArguments(
+                this.nameRewriter.lookupName(<base.NamedDeclaration>declaration, identifier),
+                options),
+            keep: true
+          };
         }
-        return {name: this.generateDartTypeName(alias.type, insideComment)};
+        // Type alias we cannot support in Dart.
+        // Substitute the alias type and parameters directly in the destination.
+        return {
+          name: this.generateDartTypeName(
+              alias.type, resolveTypeArguments(options, alias.typeParameters))
+        };
       }
 
       let kind = declaration.kind;
@@ -566,7 +702,7 @@ export class FacadeConverter extends base.TranspilerBase {
           // case if we want to get the most from Dart type checking.
           return {name: 'Function', comment: name};
         }
-        return {name: name, keep: true};
+        return {name: this.maybeAddTypeArguments(name, options), keep: true};
       }
     }
     return null;
@@ -577,7 +713,7 @@ export class FacadeConverter extends base.TranspilerBase {
    * This method works around the lack of Dart support for union types
    * generating a valid Dart type that satisfies all the types passed in.
    */
-  toSimpleDartType(types: Array<ts.TypeNode>) {
+  toSimpleDartType(types: Array<ts.TypeNode>): ts.TypeNode {
     // We use MergeType to ensure that we have already deduped types that are
     // equivalent even if they aren't obviously identical.
     // MergedType will also follow typed aliases, etc which allows us to avoid
@@ -585,6 +721,9 @@ export class FacadeConverter extends base.TranspilerBase {
     let mergedType = new MergedType(this);
     types.forEach((type) => { mergedType.merge(type); });
     let merged = mergedType.toTypeNode();
+
+    if (merged == null) return null;
+
     if (merged.kind === ts.SyntaxKind.UnionType) {
       // For union types find a Dart type that satisfies all the types.
       types = (<ts.UnionTypeNode>merged).types;
@@ -596,48 +735,56 @@ export class FacadeConverter extends base.TranspilerBase {
       let common: ts.TypeNode = types[0];
       for (let i = 1; i < types.length && common != null; ++i) {
         let type = types[i];
-        if (common !== type) {
-          if (base.isCallableType(common, this.tc) && base.isCallableType(type, this.tc)) {
-            // Fall back to a generic Function type if both types are Function.
-            let fn = <ts.FunctionOrConstructorTypeNode>ts.createNode(ts.SyntaxKind.FunctionType);
-            fn.parameters = <ts.NodeArray<ts.ParameterDeclaration>>[];
-            let parameter = <ts.ParameterDeclaration>ts.createNode(ts.SyntaxKind.Parameter);
-            parameter.dotDotDotToken = ts.createNode(ts.SyntaxKind.DotDotDotToken);
-            let name = <ts.Identifier>ts.createNode(ts.SyntaxKind.Identifier);
-            name.text = 'args';
-            fn.parameters.push(parameter);
-            common = fn;
-          } else {
-            switch (type.kind) {
-              case ts.SyntaxKind.ArrayType:
-                if (common.kind !== ts.SyntaxKind.ArrayType) {
-                  return null;
-                }
-                let array = <ts.ArrayTypeNode>ts.createNode(ts.SyntaxKind.ArrayType);
-                array.elementType = this.toSimpleDartType([
-                  (common as ts.ArrayTypeNode).elementType, (type as ts.ArrayTypeNode).elementType
-                ]);
-                common = array;
-                break;
-              // case ts.SyntaxKind
-              case ts.SyntaxKind.TypeReference:
-                if (common.kind !== ts.SyntaxKind.TypeReference) {
-                  return null;
-                }
-                common = this.commonSupertype(common, type);
-                break;
-
-              default:
-                return null;
-            }
-          }
-        }
+        common = this.findCommonType(type, common);
       }
       return common;
     }
     return merged;
   }
 
+  findCommonType(type: ts.TypeNode, common: ts.TypeNode): ts.TypeNode {
+    if (common === type) return common;
+
+    // If both types generate the exact same Dart type name without comments then
+    // there is no need to do anything. The types
+    if (this.generateDartTypeName(common, {hideComment: true}) ===
+        this.generateDartTypeName(type, {hideComment: true})) {
+      return common;
+    }
+
+
+    if (type.kind === ts.SyntaxKind.ArrayType) {
+      if (common.kind !== ts.SyntaxKind.ArrayType) {
+        return null;
+      }
+      let array = <ts.ArrayTypeNode>ts.createNode(ts.SyntaxKind.ArrayType);
+      array.elementType = this.toSimpleDartType(
+          [(common as ts.ArrayTypeNode).elementType, (type as ts.ArrayTypeNode).elementType]);
+      return array;
+    }
+    if (type.kind === ts.SyntaxKind.TypeReference && common.kind === ts.SyntaxKind.TypeReference) {
+      let candidate = this.commonSupertype(common, type);
+      if (candidate !== null) {
+        return candidate;
+      }
+    }
+
+    if (base.isCallableType(common, this.tc) && base.isCallableType(type, this.tc)) {
+      // Fall back to a generic Function type if both types are Function.
+      // TODO(jacobr): this is a problematic fallback.
+      let fn = <ts.FunctionOrConstructorTypeNode>ts.createNode(ts.SyntaxKind.FunctionType);
+      fn.parameters = <ts.NodeArray<ts.ParameterDeclaration>>[];
+      let parameter = <ts.ParameterDeclaration>ts.createNode(ts.SyntaxKind.Parameter);
+      parameter.dotDotDotToken = ts.createNode(ts.SyntaxKind.DotDotDotToken);
+      let name = <ts.Identifier>ts.createNode(ts.SyntaxKind.Identifier);
+      name.text = 'args';
+      fn.parameters.push(parameter);
+      return fn;
+    }
+    // No common type found.
+    return null;
+  }
+
   toTypeNode(type: ts.Type): ts.TypeNode {
     if (!type) return null;
     let symbol = type.getSymbol();
@@ -685,12 +832,22 @@ export class FacadeConverter extends base.TranspilerBase {
 
   commonSupertype(nodeA: ts.TypeNode, nodeB: ts.TypeNode): ts.TypeNode {
     if (nodeA == null || nodeB == null) return null;
+    if (nodeA.kind === ts.SyntaxKind.TypeReference && nodeB.kind === ts.SyntaxKind.TypeReference) {
+      // Handle the trivial case where the types are identical except for type arguments.
+      // We could do a better job and actually attempt to merge type arguments.
+      let refA = nodeA as ts.TypeReferenceNode;
+      let refB = nodeB as ts.TypeReferenceNode;
+      if (base.ident(refA.typeName) === base.ident(refB.typeName)) {
+        let merge = <ts.TypeReferenceNode>ts.createNode(ts.SyntaxKind.TypeReference);
+        merge.typeName = refA.typeName;
+        return merge;
+      }
+    }
     return this.toTypeNode(this.getCommonSupertype(
         this.tc.getTypeAtLocation(nodeA), this.tc.getTypeAtLocation(nodeB)));
   }
 
   getCommonSupertype(a: ts.Type, b: ts.Type): ts.Type {
-    if (a === b) return a;
     // This logic was probably a mistake. It adds a lot of complexity and we can
     // do better performing these calculations in the Dart analyzer based
     // directly on the union types specified in comments.