Switched back to the old behavior of upgrading variables that expose …

…constructors to abstract classes instead of concrete classes (dart-archive#69)

The only exception to this is when the type returned by the constructor is a class and not an interface/type literal. In that case we can safely make the emitted class concrete.

lib/base.ts

@@ -78,11 +78,11 @@ export type ClassLike = ts.ClassLikeDeclaration|ts.InterfaceDeclaration;
  */
 export interface ExtendedInterfaceDeclaration extends ts.InterfaceDeclaration {
   /**
-   * VariableDeclaration associated with this interface that we want to treat as the concrete
-   * location of this interface to enable interfaces that act like constructors.
-   * Because Dart does not permit calling objects like constructors we have to add this workaround.
+   * The type associated with this interface that we want to treat as the concrete location of this
+   * interface to enable interfaces that act like constructors. Because Dart does not permit calling
+   * objects like constructors we have to add this workaround.
    */
-  classLikeVariableDeclaration?: ts.VariableDeclaration;
+  constructedType?: ts.InterfaceDeclaration|ts.TypeLiteralNode;
 }
 
 export function ident(n: ts.Node): string {

lib/declaration.ts

@@ -60,18 +60,7 @@ export default class DeclarationTranspiler extends base.TranspilerBase {
 
     let classDecl = base.getEnclosingClass(node);
     if (classDecl) {
-      if (ts.isInterfaceDeclaration(classDecl)) {
-        let interfaceDecl = classDecl as base.ExtendedInterfaceDeclaration;
-        if (interfaceDecl.classLikeVariableDeclaration) {
-          // We upgrade these variable interface declarations to behave more
-          // like class declarations as we have a valid concrete JS class to
-          // an appropriate class object.
-          return this.getJsPath(interfaceDecl.classLikeVariableDeclaration, false);
-        }
-        return '';
-      } else {
-        path.push(classDecl.name.text);
-      }
+      path.push(classDecl.name.text);
     }
 
     if (ts.isModuleDeclaration(node)) {
@@ -104,7 +93,7 @@ export default class DeclarationTranspiler extends base.TranspilerBase {
       const extendedInterfaceDecl = node as base.ExtendedInterfaceDeclaration;
       // If we were able to associate a variable declaration with the interface definition then the
       // interface isn't actually anonymous.
-      return !extendedInterfaceDecl.classLikeVariableDeclaration;
+      return !extendedInterfaceDecl.constructedType;
     } else if (this.trustJSTypes && ts.isClassLike(node)) {
       // If the trust-js-types flag is set, @anonymous tags are emitted on all classes that don't
       // have any constructors or any static members.
@@ -198,7 +187,7 @@ export default class DeclarationTranspiler extends base.TranspilerBase {
   }
 
   /**
-   * Returns whether all members of the class and all base classes
+   * Returns whether all members of the class and all base classes are properties.
    */
   hasOnlyProperties(decl: ts.InterfaceDeclaration, outProperties: ts.PropertyDeclaration[]):
       boolean {
@@ -243,7 +232,21 @@ export default class DeclarationTranspiler extends base.TranspilerBase {
     let properties: ts.PropertyDeclaration[] = [];
     let isPropertyBag = false;
     if (ts.isInterfaceDeclaration(decl)) {
-      isPropertyBag = this.hasOnlyProperties(decl, properties);
+      const extendedInterfaceDecl = decl as base.ExtendedInterfaceDeclaration;
+      const constructedType = extendedInterfaceDecl.constructedType;
+      if (constructedType) {
+        // If this interface is the upgraded version of a variable whose type contained a
+        // constructor, we must check the type hierarchy of the original type, as well as the
+        // properties of the new merged interface.
+        if (ts.isInterfaceDeclaration(constructedType)) {
+          isPropertyBag = this.hasOnlyProperties(constructedType, properties) &&
+              this.hasOnlyPropertiesHelper(decl.members, properties);
+        } else if (ts.isTypeLiteralNode(constructedType)) {
+          isPropertyBag = this.hasOnlyPropertiesHelper(decl.members, properties);
+        }
+      } else {
+        isPropertyBag = this.hasOnlyProperties(decl, properties);
+      }
     } else if (ts.isTypeLiteralNode(decl)) {
       isPropertyBag = this.hasOnlyPropertiesHelper(decl.members, properties);
     }

lib/facade_converter.ts

@@ -611,9 +611,10 @@ export class FacadeConverter extends base.TranspilerBase {
   }
 
   replaceNode(original: ts.Node, replacement: ts.Node) {
-    if (ts.isVariableDeclaration(original) && ts.isClassDeclaration(replacement)) {
+    if (ts.isVariableDeclaration(original) &&
+        (ts.isInterfaceDeclaration(replacement) || ts.isClassDeclaration(replacement))) {
       // Handle the speical case in mergeVariablesIntoClasses where we upgrade variable declarations
-      // to classes.
+      // to interfaces or classes.
       const symbol = this.tc.getSymbolAtLocation(original.name);
       symbol.declarations = symbol.getDeclarations().map((declaration: ts.Declaration) => {
         // TODO(derekx): Changing the declarations of a symbol like this is a hack. It would be

lib/merge.ts

@@ -401,28 +401,40 @@ export function normalizeSourceFile(
 
             if (classes.has(name)) {
               const existing = classes.get(name);
-              // If a class with the same name as the variable already exists, we should suppress
-              // that declaration because it will be cloned into a stub class below.
+              // If a class or interface with the same name as the variable already exists, we
+              // should suppress that declaration because it will be cloned into a stub class or
+              // interface below.
               fc.suppressNode(existing);
             }
 
             // These properties do not exist on TypeLiteralNodes.
-            let clazzTypeParameters, clazzHeritageClauses, clazzMembers;
+            let clazzTypeParameters, clazzHeritageClauses;
             if (ts.isClassDeclaration(constructedType) ||
                 ts.isInterfaceDeclaration(constructedType)) {
               clazzTypeParameters = base.cloneNodeArray(constructedType.typeParameters);
               clazzHeritageClauses = base.cloneNodeArray(constructedType.heritageClauses);
-              clazzMembers =
-                  base.cloneNodeArray(constructedType.members as ts.NodeArray<ts.ClassElement>);
             }
-            const clazz = ts.createClassDeclaration(
-                base.cloneNodeArray(constructedType.decorators),
-                base.cloneNodeArray(constructedType.modifiers),
-                ts.getMutableClone(variableDecl.name),
-                base.cloneNodeArray(clazzTypeParameters),
-                base.cloneNodeArray(clazzHeritageClauses),
-                base.cloneNodeArray(clazzMembers),
-            );
+
+            let clazz: ts.InterfaceDeclaration|ts.ClassDeclaration;
+            if (ts.isClassDeclaration(constructedType)) {
+              clazz = ts.createClassDeclaration(
+                  base.cloneNodeArray(constructedType.decorators),
+                  base.cloneNodeArray(constructedType.modifiers),
+                  ts.getMutableClone(variableDecl.name), base.cloneNodeArray(clazzTypeParameters),
+                  base.cloneNodeArray(clazzHeritageClauses),
+                  base.cloneNodeArray(constructedType.members));
+            } else if (
+                ts.isTypeLiteralNode(constructedType) ||
+                ts.isInterfaceDeclaration(constructedType)) {
+              clazz = ts.createInterfaceDeclaration(
+                  base.cloneNodeArray(constructedType.decorators),
+                  base.cloneNodeArray(constructedType.modifiers),
+                  ts.getMutableClone(variableDecl.name), base.cloneNodeArray(clazzTypeParameters),
+                  base.cloneNodeArray(clazzHeritageClauses),
+                  base.cloneNodeArray(constructedType.members));
+              (clazz as base.ExtendedInterfaceDeclaration).constructedType = constructedType;
+            }
+
             base.copyLocation(variableDecl, clazz);
             clazz.flags = variableDecl.flags;
             fc.replaceNode(variableDecl, clazz);
@@ -434,9 +446,9 @@ export function normalizeSourceFile(
               // with the same name as the variable already exists. If it does, we must rename it.
               // That type is not directly associated with this variable, so they cannot be
               // combined.
-              const existingSymbol = fc.tc.getSymbolAtLocation(variableDecl.name);
-              if (existingSymbol.getDeclarations()) {
-                for (const declaration of existingSymbol.getDeclarations()) {
+              const variableSymbol = fc.getSymbolAtLocation(variableDecl.name);
+              if (variableSymbol.getDeclarations()) {
+                for (const declaration of variableSymbol.getDeclarations()) {
                   if (ts.isInterfaceDeclaration(declaration) ||
                       ts.isTypeAliasDeclaration(declaration)) {
                     declaration.name.escapedText = ts.escapeLeadingUnderscores(name + 'Type');

test/declaration_test.ts

@@ -777,9 +777,7 @@ abstract class XType {
 }
 
 @JS()
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external String get a;
   external set a(String v);
   external num get b;
@@ -812,9 +810,7 @@ abstract class XType {
 }
 
 @JS()
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external String get a;
   external set a(String v);
   external static num get b;
@@ -831,9 +827,7 @@ declare var C: {new(): C; CHECKING: number; }`)
           .to.equal(`import "dart:html" show Event;
 
 @JS()
-class C {
-  // @Ignore
-  C.fakeConstructor$();
+abstract class C {
   external dynamic Function(Event) get oncached;
   external set oncached(dynamic Function(Event) v);
   external factory C();
@@ -854,9 +848,7 @@ declare var X: {
   prototype: X,
   new(a: string, b: number): X
 };`).to.equal(`@JS()
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external String get a;
   external set a(String v);
   external num get b;
@@ -877,9 +869,7 @@ declare var X: {
   prototype: X,
   new(a: string, b: number): X
 };`).to.equal(`@JS()
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external String get a;
   external set a(String v);
   external static num get b;
@@ -895,9 +885,7 @@ interface Y { c: number; }
 declare var X: {prototype: X, new (): X, b: string};
 declare var Y: {prototype: Y, new (): Y, d: string};
 `).to.equal(`@JS()
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external num get a;
   external set a(num v);
   external factory X();
@@ -906,9 +894,7 @@ class X {
 }
 
 @JS()
-class Y {
-  // @Ignore
-  Y.fakeConstructor$();
+abstract class Y {
   external num get c;
   external set c(num v);
   external factory Y();
@@ -950,9 +936,7 @@ abstract class Y {
 }
 
 @JS()
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external String get a;
   external set a(String v);
   external num get b;
@@ -987,9 +971,7 @@ abstract class XType {
 
 // Module m1
 @JS("m1.X")
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external String get a;
   external set a(String v);
   external num get b;
@@ -1027,9 +1009,7 @@ abstract class XType {
 }
 
 @JS()
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external String get a;
   external set a(String v);
   external num get b;
@@ -1064,9 +1044,7 @@ abstract class XType {
 }
 
 @JS()
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external String get a;
   external set a(String v);
   external static num get b;
@@ -1109,9 +1087,7 @@ abstract class X {
 }
 
 @JS()
-class Y {
-  // @Ignore
-  Y.fakeConstructor$();
+abstract class Y {
   external String get a;
   external set a(String v);
   external num get b;
@@ -1146,9 +1122,7 @@ abstract class XType {
 
 // Module m1
 @JS("m1.X")
-class X {
-  // @Ignore
-  X.fakeConstructor$();
+abstract class X {
   external String get a;
   external set a(String v);
   external num get b;
@@ -1224,9 +1198,7 @@ describe('flags', () => {
             }`,
         generateHTMLOpts)
         .to.equal(`@JS()
-class AbstractRange {
-  // @Ignore
-  AbstractRange.fakeConstructor$();
+abstract class AbstractRange {
   external bool get collapsed;
   external num get endOffset;
   external num get startOffset;
@@ -1260,9 +1232,7 @@ abstract class CacheBase {
 }
 
 @JS()
-class MyCache implements CacheBase {
-  // @Ignore
-  MyCache.fakeConstructor$();
+abstract class MyCache implements CacheBase {
   external factory MyCache();
   external static num get CHECKING;
   external static num get DOWNLOADING;
@@ -1300,9 +1270,7 @@ abstract class CacheBase {
 }
 
 @JS()
-class MyCache implements CacheBase {
-  // @Ignore
-  MyCache.fakeConstructor$();
+abstract class MyCache implements CacheBase {
   external factory MyCache();
   external num get CHECKING;
   external num get DOWNLOADING;