Fix Long references in unions, fix Enums in typedefs

src/__tests__/fixtures/base.thrift

@@ -0,0 +1,14 @@
+enum Weekday {
+    UNKNOWN   = 0
+    SUNDAY    = 1
+    MONDAY    = 2
+    TUESDAY   = 3
+    WEDNESDAY = 4
+    THURSDAY  = 5
+    FRIDAY    = 6
+    SATURDAY  = 7
+}
+
+struct TimeRange {
+    1: optional Weekday weekDay
+}

src/__tests__/fixtures/typedef-enum-value-reference.thrift

@@ -0,0 +1,5 @@
+
+include 'base.thrift'
+
+typedef map<base.Weekday, list<base.TimeRange>> TimeRangeByDayOfWeek
+

src/__tests__/fixtures/typedef-long-import.thrift

@@ -0,0 +1 @@
+typedef i64 (js.type = 'Long') Long

src/__tests__/typedefs.test.js

@@ -30,6 +30,37 @@ import path from 'path';
 import fs from 'fs-extra';
 import tmp from 'tmp';
 
+test('Long module is imported when needed', () => {
+  const converter = new ThriftFileConverter(
+    `src/__tests__/fixtures/typedef-long-import.thrift`,
+    false
+  );
+  expect(converter.generateFlowFile()).toMatchInlineSnapshot(`
+"// @flow
+
+import thrift2flow$Long from \\"long\\";
+
+export type Long = thrift2flow$Long;
+"
+`);
+});
+
+test('typedefs should reference enum types not value', () => {
+  const converter = new ThriftFileConverter(
+    `src/__tests__/fixtures/typedef-enum-value-reference.thrift`,
+    false
+  );
+  expect(converter.generateFlowFile()).toMatchInlineSnapshot(`
+"// @flow
+
+import * as base from \\"./base\\";
+
+export type TimeRangeByDayOfWeek = {
+  [$Values<typeof base.Weekday>]: base.TimeRange[]
+};
+"
+`);
+});
 test('typedef Date', done => {
   flowResultTest(
     {
@@ -95,7 +126,7 @@ struct UserActivitiesRequest {
     .map(p => path.resolve(root, p))
     .forEach(p => {
       let output = new ThriftFileConverter(p, true).generateFlowFile();
-      let longIndex = output.indexOf('import Long');
+      let longIndex = output.indexOf('import thrift2flow$Long');
       expect(longIndex).not.toBe(-1);
     });
 });
@@ -115,7 +146,7 @@ typedef i64 (js.type = "Long") Points
     .map(p => path.resolve(root, p))
     .forEach(p => {
       let output = new ThriftFileConverter(p, true).generateFlowFile();
-      let longIndex = output.indexOf('import Long');
+      let longIndex = output.indexOf('import thrift2flow$Long');
       // Expected long definition but did not find one
       expect(longIndex).not.toBe(-1);
     });

src/__tests__/unions.test.js

@@ -34,14 +34,14 @@ test("Long module is imported when needed", () => {
   expect(converter.generateFlowFile()).toMatchInlineSnapshot(`
 "// @flow
 
-import Long from \\"long\\";
+import thrift2flow$Long from \\"long\\";
 
 export type RawValue =
   | {| binaryValue: Buffer |}
   | {| boolValue: boolean |}
   | {| doubleValue: number |}
   | {| int32Value: number |}
-  | {| int64Value: Long |}
+  | {| int64Value: thrift2flow$Long |}
   | {| stringValue: string |};
 "
 `);

src/main/convert.js

@@ -73,7 +73,10 @@ export class ThriftFileConverter {
     this.ast = this.thrift.asts[this.thrift.filename];
     this.initIdentifiersTable();
     this.thriftAstDefinitions = this.ast.definitions;
-    this.types = new TypeConverter(this.thriftAstDefinitions);
+    this.types = new TypeConverter(
+      this.thriftAstDefinitions,
+      this.identifiersTable
+    );
     this.withsource = withsource;
   }
 
@@ -300,7 +303,7 @@ export class ThriftFileConverter {
     });
 
     if (this.isLongDefined()) {
-      generatedImports.push("import Long from 'long'");
+      generatedImports.push("import thrift2flow$Long from 'long'");
     }
     return generatedImports.join('\n');
   };

src/main/thrift-types.js

@@ -26,6 +26,7 @@
 
 import {BaseType, Enum, ListType, MapType, SetType} from 'thriftrw/ast';
 import {id} from './identifier';
+import {type Definition} from './ast-types';
 
 export class TypeConverter {
   static primitives = {
@@ -42,14 +43,19 @@ export class TypeConverter {
   };
 
   static i64Mappings = {
-    Long: 'Long',
+    Long: 'thrift2flow$Long',
     Date: 'string',
   };
 
   thriftAstDefinitions: Array<any>;
+  identifiersTable: $ReadOnly<{[key: string]: Definition}>;
 
-  constructor(thriftAstDefinitions: Array<any>) {
+  constructor(
+    thriftAstDefinitions: Array<any>,
+    identifiersTable: $ReadOnly<{[key: string]: Definition}>
+  ) {
     this.thriftAstDefinitions = thriftAstDefinitions;
+    this.identifiersTable = identifiersTable;
   }
 
   annotation(t: BaseType): string {
@@ -62,10 +68,13 @@ export class TypeConverter {
   }
 
   convert = (t: BaseType): string => {
+    if (!t) {
+      throw new Error(`Assertion failed. t is not defined`);
+    }
     return (
       this.arrayType(t) ||
-      this.enumType(t) ||
       this.mapType(t) ||
+      this.enumType(t) ||
       this.annotation(t) ||
       TypeConverter.primitives[t.baseType] ||
       id(t.name)
@@ -74,10 +83,9 @@ export class TypeConverter {
 
   enumType = (thriftValueType: BaseType) => {
     if (this.isEnum(thriftValueType)) {
-      const name = thriftValueType.name;
       // Enums are values, not types. To refer to the type,
       // we use $Values<...>.
-      return `$Values<typeof ${name}>`;
+      return `$Values<typeof ${thriftValueType.name}>`;
     }
     return null;
   };
@@ -98,11 +106,9 @@ export class TypeConverter {
 
   isEnum(def: BaseType) {
     // Enums export const, not type
-    let defName = def.name;
-    let defValueType = this.thriftAstDefinitions.find(value => {
-      return value.id.name === defName;
-    });
-
-    return defValueType && defValueType instanceof Enum;
+    if (!def.name) {
+      return undefined;
+    }
+    return this.identifiersTable[def.name].type === 'Enum';
   }
 }