feat: add support for strict unions

creditkarma · Feb 24, 2019 · 30c5d3d · 30c5d3d
1 parent 6fba8ac
commit 30c5d3d
Show file tree

Hide file tree

Showing 36 changed files with 2,337 additions and 912 deletions.
diff --git a/README.md b/README.md
@@ -37,7 +37,8 @@ The available options are:
 * --outDir: The directory to save generated files to. Will be created if it doesn't exist. Defaults to 'codegen'.
 * --sourceDir: The directory to search for source Thrift files. Defaults to 'thrift'.
 * --target: The core library to generate for, either 'apache' or 'thrift-server'. Defaults to 'apache'.
-* --fallback-namespace: The namespace to fallback to if no 'js' namespace exists. Defaults to 'java'. Set to 'none' to use no namespace.
+* --strictUnions: Should we generate strict unions (Only available for target = 'thrift-server'. More on this below). Defaults to undefined.
+* --fallbackNamespace: The namespace to fallback to if no 'js' namespace exists. Defaults to 'java'. Set to 'none' to use no namespace.
 
 All other fields are assumed to be source files.
 
@@ -448,6 +449,91 @@ It's just an object that knows how to read the given object from a Thrift Protoc
 
 The codec will always follow this naming convention, just appending `Codec` onto the end of your struct name.
 
+### Strict Unions
+
+This is an option only available when generating for `thrift-server`. This option will generate Thrift unions as TypeScript unions. This changes the codegen if a few significant ways.
+
+Back with our example union definition:
+
+```c
+union MyUnion {
+    1: string option1
+    2: i32 option2
+}
+```
+
+When compiling with the `--strictUnions` flag we now generate TypeScript like this:
+
+```typescript
+enum MyUnionType {
+    MyUnionWithOption1 = "option1",
+    MyUnionWithOption2 = "option2"
+}
+type MyUnion = IMyUnionWithOption1 | IMyUnionWithOption2
+interface IMyUnionWithOption1 {
+    __type: MyUnionType.MyUnionWithOption1
+    option1: string
+    option2?: void
+}
+interface IMyUnionWithOption2 {
+    __type: MyUnionType.MyUnionWithOption2
+    option1?: void
+    option2: number
+}
+type MyUnionArgs = IMyUnionWithOption1Args | IMyUnionWithOption2Args
+interface IMyUnionWithOption1Args {
+    option1: string
+    option2?: void
+}
+interface IMyUnionWithOption2Args {
+    option1?: void
+    option2: number
+}
+```
+
+This output is more complex, but it allows us to do a number of things. It allows us to take advantage of discriminated unions in our application code:
+
+```typescript
+function processUnion(union: MyUnion) {
+    switch (union.__type) {
+        case MyUnionType.MyUnionWithOption1:
+            // Do something
+        case MyUnionType.MyUnionWithOption2:
+            // Do something
+        default:
+            const _exhaustiveCheck: never = union
+            throw new Error(`Non-exhaustive match for type: ${_exhaustiveCheck}`)
+    }
+}
+```
+
+It also provides compile-time checks that we are definition one and only one value for a union. Instead of a struct with optional fields we are defining a union of interfaces that each have one required field and any other fields must be of type `void`.
+
+This allows you to do things like check `union.option2 !== undefined` without a compiler error, but will give a compiler error if you try to use a value that shouldn't exist of a given union.
+
+Using this form will require that you prove to the compiler that one (and only one) field is set for your unions.
+
+In addition to the changed types output, the `--strictUnions` flag changes the output of the `Codec` object. The `Codec` object will have one additional method `create`. The `create` method takes one of the loose interfaces and coerces it into the strict interface (including the `__type` property).
+
+For the example `MyUnion` that would be defined as:
+
+```typescript
+const MyUnionCodec: thrift.IStructToolkit<IUserArgs, IUser> { = {
+    create(args: MyUnionArgs): MyUnion {
+        // ...
+    },
+    encode(obj: IUserArgs, output: thrift.TProtocol): void {
+        // ...
+    },
+    decode(input: thrift.TProtocol): IUser {
+        // ...
+    }
+}
+```
+
+Note: In a future breaking release all the `Codec` objects will be renamed to `Toolkit` as they will provide more utilities for working with defined Thrift objects.
+
+
 ## Apache Thrift
 
 The generated code can also work with the [Apache Thrift Library](https://github.com/apache/thrift/tree/master/lib/nodejs).

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -23,7 +23,7 @@
         "format": "prettier --write 'src/**/*.ts'",
         "move:fixtures": "rimraf dist/tests/unit/fixtures && cp -r src/tests/unit/fixtures dist/tests/unit/fixtures",
         "pretest": "npm run build:test && npm run move:fixtures",
-        "test": "npm run lint && npm run test:unit && npm run test:integration",
+        "test": "npm run test:unit",
         "test:unit": "NODE_ENV=test mocha --opts mocha.opts",
         "test:integration": "NODE_ENV=test mocha --opts mocha.integration.opts",
         "coverage": "NODE_ENV=test nyc mocha --opts mocha.cover.opts"

diff --git a/src/main/bin/resolveOptions.ts b/src/main/bin/resolveOptions.ts
@@ -78,8 +78,8 @@ export function resolveOptions(args: Array<string>): IMakeOptions {
                 break
 
             case '--strictUnions':
-                options.strictUnions = args[index + 1] === 'true'
-                index += 2
+                options.strictUnions = true
+                index += 1
                 break
 
             default:

diff --git a/src/main/defaults.ts b/src/main/defaults.ts
@@ -11,7 +11,7 @@ export const DEFAULT_OPTIONS: IMakeOptions = {
     sourceDir: './thrift',
     target: 'apache',
     files: [],
-    library: DEFAULT_APACHE_LIB,
+    library: '',
     strictUnions: false,
     fallbackNamespace: 'java',
 }

diff --git a/src/main/render/shared/identifiers.ts b/src/main/render/shared/identifiers.ts
@@ -23,6 +23,7 @@ export const COMMON_IDENTIFIERS = {
     this: ts.createIdentifier('this'),
     flush: ts.createIdentifier('flush'),
     process: ts.createIdentifier('process'),
+    create: ts.createIdentifier('create'),
     decode: ts.createIdentifier('decode'),
     encode: ts.createIdentifier('encode'),
     read: ts.createIdentifier('read'),

diff --git a/src/main/render/thrift-server/annotations.ts b/src/main/render/thrift-server/annotations.ts
@@ -59,7 +59,7 @@ export function renderServiceAnnotations(
         ts.createVariableDeclarationList(
             [
                 ts.createVariableDeclaration(
-                    ts.createIdentifier('annotations'),
+                    COMMON_IDENTIFIERS.annotations,
                     ts.createTypeReferenceNode(
                         THRIFT_IDENTIFIERS.IThriftAnnotations,
                         undefined,

diff --git a/src/main/render/thrift-server/identifiers.ts b/src/main/render/thrift-server/identifiers.ts
@@ -9,6 +9,7 @@ export const THRIFT_IDENTIFIERS = {
     IFieldAnnotations: ts.createIdentifier('thrift.IFieldAnnotations'),
     IMethodAnnotations: ts.createIdentifier('thrift.IMethodAnnotations'),
     IStructCodec: ts.createIdentifier('thrift.IStructCodec'),
+    IStructToolkit: ts.createIdentifier('thrift.IStructToolkit'),
     IThriftConnection: ts.createIdentifier('thrift.IThriftConnection'),
     ProtocolConstructor: ts.createIdentifier('thrift.IProtocolConstructor'),
     TransportConstructor: ts.createIdentifier('thrift.ITransportConstructor'),

diff --git a/src/main/render/thrift-server/names.ts b/src/main/render/thrift-server/names.ts
@@ -0,0 +1,3 @@
+// export function renderIdentifierName(
+//     node:
+// )
diff --git a/src/main/render/thrift-server/service/client.ts b/src/main/render/thrift-server/service/client.ts
@@ -48,7 +48,7 @@ import {
 } from '../annotations'
 
 import { createClassConstructor } from '../../shared/utils'
-import { codecName, looseName, strictName } from '../struct/utils'
+import { looseName, strictName, toolkitName } from '../struct/utils'
 
 function extendsAbstract(): ts.HeritageClause {
     return ts.createHeritageClause(ts.SyntaxKind.ExtendsKeyword, [
@@ -243,7 +243,7 @@ function createBaseMethodForDefinition(
                 ),
                 // args.write(output)
                 createMethodCallStatement(
-                    ts.createIdentifier(codecName(createStructArgsName(def))),
+                    ts.createIdentifier(toolkitName(createStructArgsName(def))),
                     'encode',
                     [COMMON_IDENTIFIERS.args, COMMON_IDENTIFIERS.output],
                 ),
@@ -515,7 +515,9 @@ function createNewResultInstance(
             ),
             ts.createCall(
                 ts.createPropertyAccess(
-                    ts.createIdentifier(codecName(createStructResultName(def))),
+                    ts.createIdentifier(
+                        toolkitName(createStructResultName(def)),
+                    ),
                     ts.createIdentifier('decode'),
                 ),
                 undefined,

diff --git a/src/main/render/thrift-server/service/processor.ts b/src/main/render/thrift-server/service/processor.ts
@@ -60,7 +60,7 @@ import {
     renderServiceAnnotationsStaticProperty,
 } from '../annotations'
 
-import { codecName, strictName } from '../struct/utils'
+import { strictName, toolkitName } from '../struct/utils'
 
 function objectLiteralForServiceFunctions(
     node: ThriftStatement,
@@ -396,7 +396,7 @@ function createProcessFunctionMethod(
                                         // StructCodec.encode(result, output)
                                         createMethodCallStatement(
                                             ts.createIdentifier(
-                                                codecName(
+                                                toolkitName(
                                                     createStructResultName(
                                                         funcDef,
                                                     ),
@@ -487,7 +487,7 @@ function createArgsVariable(
                 ts.createCall(
                     ts.createPropertyAccess(
                         ts.createIdentifier(
-                            codecName(createStructArgsName(funcDef)),
+                            toolkitName(createStructArgsName(funcDef)),
                         ),
                         ts.createIdentifier('decode'),
                     ),
@@ -610,7 +610,9 @@ function createThenForException(
             ),
             // StructCodec.encode(result, output)
             createMethodCallStatement(
-                ts.createIdentifier(codecName(createStructResultName(funcDef))),
+                ts.createIdentifier(
+                    toolkitName(createStructResultName(funcDef)),
+                ),
                 'encode',
                 [COMMON_IDENTIFIERS.result, COMMON_IDENTIFIERS.output],
             ),