Merge pull request dart-archive#49 from derekxu16/master

Fixed bug when making properties of top level members with anonymous types static
derekxu16 · Oct 11, 2019 · f4e05b9 · f4e05b9
2 parents 27b0a69 + d1b3ed3
commit f4e05b9
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 7 deletions.
diff --git a/README.md b/README.md
@@ -18,6 +18,11 @@ Dart interop facade file is written to stdout.
 ### Advanced
 `dart_js_facade_gen --destination=<destination-dir> --basePath=<input d.ts file directory> <input d.ts file> <input d.ts file> ...`
 
+#### Flags
+`--destination=<destination-dir>`: Output generated code to destination-dir
+`--generate-html`: Generate facades for dart:html types rather than importing them
+`--explicit-static`: Disables default assumption that properties declared on the anonymous types of top level variable declarations are static
+
 ### Example
 `dart_js_facade_gen --destination=/usr/foo/tmp/chartjs/lib --basePath=/usr/foo/git/DefinitelyTyped/chartjs /usr/foo/git/DefinitelyTyped/chartjs/chart.d.ts`
 

diff --git a/index.js b/index.js
@@ -4,8 +4,12 @@ const main = require('./build/lib/main.js');
 
 var args = require('minimist')(process.argv.slice(2), {
   base: 'string',
-  boolean: ['semantic-diagnostics', 'generate-html'],
-  alias: {'semantic-diagnostics': 'semanticDiagnostics', 'generate-html': 'generateHTML'}
+  boolean: ['semantic-diagnostics', 'generate-html', 'explicit-static'],
+  alias: {
+    'semantic-diagnostics': 'semanticDiagnostics',
+    'generate-html': 'generateHTML',
+    'explicit-static': 'explicitStatic'
+  }
 });
 try {
   var transpiler = new main.Transpiler(args);

diff --git a/lib/base.ts b/lib/base.ts
@@ -87,6 +87,7 @@ export interface ExtendedInterfaceDeclaration extends ts.InterfaceDeclaration {
 }
 
 export function ident(n: ts.Node): string {
+  if (!n) return null;
   if (ts.isIdentifier(n)) return n.text;
   if (n.kind === ts.SyntaxKind.FirstLiteralToken) return (n as ts.LiteralLikeNode).text;
   if (ts.isQualifiedName(n)) {

diff --git a/lib/main.ts b/lib/main.ts
@@ -45,6 +45,11 @@ export interface TranspilerOptions {
    * Generate browser API facades instead of importing them from dart:html.
    */
   generateHTML?: boolean;
+  /**
+   * Do not assume that all properties declared on the anonymous types of top level variable
+   * declarations are static.
+   */
+  explicitStatic?: boolean;
 
   /**
    * Experimental JS Interop specific option to promote properties with function
@@ -255,7 +260,7 @@ export class Transpiler {
     }
 
     this.lastCommentIdx = -1;
-    merge.normalizeSourceFile(sourceFile, this.fc);
+    merge.normalizeSourceFile(sourceFile, this.fc, this.options.explicitStatic);
     this.pushContext(OutputContext.Default);
     this.visit(sourceFile);
     this.popContext();

diff --git a/lib/merge.ts b/lib/merge.ts
@@ -227,7 +227,7 @@ export class MergedTypeParameters {
 /**
  * Normalize a SourceFile
  */
-export function normalizeSourceFile(f: ts.SourceFile, fc: FacadeConverter) {
+export function normalizeSourceFile(f: ts.SourceFile, fc: FacadeConverter, explicitStatic = false) {
   let modules: Map<string, ts.ModuleDeclaration> = new Map();
 
   // Merge top level modules.
@@ -374,9 +374,30 @@ export function normalizeSourceFile(f: ts.SourceFile, fc: FacadeConverter) {
                       if (base.ident(member.name) === 'prototype') {
                         break;
                       }
-                      addModifier(member, ts.createNode(ts.SyntaxKind.StaticKeyword));
-                      member.parent = existing;
-                      Array.prototype.push.call(members, member);
+
+                      if (!explicitStatic) {
+                        // Finds all existing declarations of this property in the inheritance
+                        // hierarchy of this class
+                        const existingDeclarations =
+                            findPropertyInHierarchy(base.ident(member.name), existing, classes);
+
+                        if (existingDeclarations.size) {
+                          for (const existingDecl of existingDeclarations) {
+                            addModifier(
+                                existingDecl, ts.createModifier(ts.SyntaxKind.StaticKeyword));
+                          }
+                        }
+                      }
+
+                      // If needed, add declaration of property to the interface that we are
+                      // currently handling
+                      if (!findPropertyInClass(base.ident(member.name), existing)) {
+                        if (!explicitStatic) {
+                          addModifier(member, ts.createModifier(ts.SyntaxKind.StaticKeyword));
+                        }
+                        member.parent = existing;
+                        Array.prototype.push.call(members, member);
+                      }
                       break;
                     case ts.SyntaxKind.IndexSignature:
                       member.parent = existing.parent;
@@ -402,6 +423,37 @@ export function normalizeSourceFile(f: ts.SourceFile, fc: FacadeConverter) {
     }
   }
 
+  function findPropertyInClass(propName: string, classLike: base.ClassLike): ts.ClassElement|
+      undefined {
+    const members = classLike.members as ts.NodeArray<ts.ClassElement>;
+    return members.find((member: ts.ClassElement) => {
+      if (base.ident(member.name) === propName) {
+        return true;
+      }
+    });
+  }
+
+  function findPropertyInHierarchy(
+      propName: string, classLike: base.ClassLike,
+      classes: Map<string, base.ClassLike>): Set<ts.ClassElement> {
+    const propertyDeclarations = new Set<ts.ClassElement>();
+    const declaration = findPropertyInClass(propName, classLike);
+    if (declaration) propertyDeclarations.add(declaration);
+
+    const heritageClauses = classLike.heritageClauses || ts.createNodeArray();
+    for (const clause of heritageClauses) {
+      if (clause.token !== ts.SyntaxKind.ExtendsKeyword) {
+        continue;
+      }
+      const name = base.ident(clause.types[0].expression);
+      const declarationsInAncestors = findPropertyInHierarchy(propName, classes.get(name), classes);
+      if (declarationsInAncestors.size) {
+        declarationsInAncestors.forEach(decl => propertyDeclarations.add(decl));
+      }
+    }
+    return propertyDeclarations;
+  }
+
   function removeFromArray(nodes: ts.NodeArray<ts.Node>, v: ts.Node) {
     for (let i = 0, len = nodes.length; i < len; ++i) {
       if (nodes[i] === v) {

diff --git a/test/declaration_test.ts b/test/declaration_test.ts
@@ -521,6 +521,79 @@ abstract class AbstractRange {
   external factory AbstractRange();
 }`);
   });
+
+  describe('emitting properties of top level variables with anonymous types as static', () => {
+    it('performs this by default', () => {
+      expectTranslate(`
+     declare interface CacheBase {
+      readonly CHECKING: number;
+      readonly DOWNLOADING: number;
+      readonly IDLE: number;
+    }
+    
+    declare interface MyCache extends CacheBase {}
+    
+    declare var MyCache: {
+      prototype: MyCache;
+      new (): MyCache;
+      readonly CHECKING: number;
+      readonly DOWNLOADING: number;
+      readonly IDLE: number;
+    };`).to.equal(`@anonymous
+@JS()
+abstract class CacheBase {
+  external static num get CHECKING;
+  external static num get DOWNLOADING;
+  external static num get IDLE;
+}
+
+@JS("MyCache")
+abstract class MyCache implements CacheBase {
+  external factory MyCache();
+  external static num get CHECKING;
+  external static num get DOWNLOADING;
+  external static num get IDLE;
+}`);
+    });
+
+    it('but not when the --explicit-static flag is set', () => {
+      const explicitStaticOpts = {failFast: true, explicitStatic: true};
+      expectTranslate(
+          `
+     declare interface CacheBase {
+      readonly CHECKING: number;
+      readonly DOWNLOADING: number;
+      readonly IDLE: number;
+    }
+    
+    declare interface MyCache extends CacheBase {}
+    
+    declare var MyCache: {
+      prototype: MyCache;
+      new (): MyCache;
+      readonly CHECKING: number;
+      readonly DOWNLOADING: number;
+      readonly IDLE: number;
+    };`,
+          explicitStaticOpts)
+          .to.equal(`@anonymous
+@JS()
+abstract class CacheBase {
+  external num get CHECKING;
+  external num get DOWNLOADING;
+  external num get IDLE;
+  external factory CacheBase({num CHECKING, num DOWNLOADING, num IDLE});
+}
+
+@JS("MyCache")
+abstract class MyCache implements CacheBase {
+  external factory MyCache();
+  external num get CHECKING;
+  external num get DOWNLOADING;
+  external num get IDLE;
+}`);
+    });
+  });
 });
 
 describe('single call signature interfaces', () => {