Mark TypeScript types as non-nullable

src/tsickle.ts

@@ -310,12 +310,12 @@ class ClosureRewriter extends Rewriter {
   }
 
   /** Emits a type annotation in JSDoc, or {?} if the type is unavailable. */
-  emitJSDocType(node: ts.Node, additionalDocTag?: string) {
+  emitJSDocType(node: ts.Node, additionalDocTag?: string, type?: ts.Type) {
     this.emit(' /**');
     if (additionalDocTag) {
       this.emit(' ' + additionalDocTag);
     }
-    this.emit(` @type {${this.typeToClosure(node)}} */`);
+    this.emit(` @type {${this.typeToClosure(node, type)}} */`);
   }
 
   /**
@@ -328,7 +328,7 @@ class ClosureRewriter extends Rewriter {
    *     object/array types.  This is a workaround specifically for destructuring
    *     bind patterns.
    */
-  typeToClosure(context: ts.Node, type?: ts.Type, destructuring?: boolean): string {
+  typeToClosure(context: ts.Node, type?: ts.Type, destructuring = false): string {
     if (this.options.untyped) {
       return '?';
     }
@@ -339,7 +339,7 @@ class ClosureRewriter extends Rewriter {
     }
     let translator = new TypeTranslator(typeChecker, context, this.options.typeBlackListPaths);
     translator.warn = msg => this.debugWarn(context, msg);
-    return translator.translate(type, destructuring);
+    return translator.translate(type, true);
   }
 
   /**
@@ -532,10 +532,30 @@ class Annotator extends ClosureRewriter {
         // When TypeScript emits JS, it removes one layer of "redundant"
         // parens, but we need them for the Closure type assertion.  Work
         // around this by using two parens.  See test_files/coerce.*.
+        // TODO: the comment is currently dropped from pure assignments due to
+        //   https://github.com/Microsoft/TypeScript/issues/9873
         this.emit('((');
         this.writeNode(node);
         this.emit('))');
         return true;
+      case ts.SyntaxKind.NonNullExpression:
+        const nnexpr = node as ts.NonNullExpression;
+        let type = this.program.getTypeChecker().getTypeAtLocation(nnexpr.expression);
+        if (type.flags & ts.TypeFlags.Union) {
+          const nonNullUnion =
+              (type as ts.UnionType)
+                  .types.filter(
+                      t => (t.flags & (ts.TypeFlags.Null | ts.TypeFlags.Undefined)) === 0);
+          const typeCopy = Object.assign({}, type as ts.UnionType);
+          typeCopy.types = nonNullUnion;
+          type = typeCopy;
+        }
+        this.emitJSDocType(nnexpr, undefined, type);
+        // See comment above.
+        this.emit('((');
+        this.writeNode(nnexpr.expression);
+        this.emit('))');
+        return true;
       default:
         break;
     }

src/type-translator.ts

@@ -153,7 +153,7 @@ export class TypeTranslator {
    * @param notNull When true, insert a ! before any type references.  This
    *    is to work around the difference between TS and Closure destructuring.
    */
-  translate(type: ts.Type, notNull = false): string {
+  translate(type: ts.Type, notNull: boolean): string {
     // See the function `buildTypeDisplay` in the TypeScript compiler source
     // for guidance on a similar operation.
 
@@ -199,7 +199,7 @@ export class TypeTranslator {
         this.warn('class has no symbol');
         return '?';
       }
-      return this.symbolToString(type.symbol);
+      return notNullPrefix + this.symbolToString(type.symbol);
     } else if (type.flags & ts.TypeFlags.Interface) {
       // Note: ts.InterfaceType has a typeParameters field, but that
       // specifies the parameters that the interface type *expects*
@@ -221,7 +221,7 @@ export class TypeTranslator {
           return '?';
         }
       }
-      return this.symbolToString(type.symbol);
+      return notNullPrefix + this.symbolToString(type.symbol);
     } else if (type.flags & ts.TypeFlags.Reference) {
       // A reference to another type, e.g. Array<number> refers to Array.
       // Emit the referenced type and any type arguments.
@@ -241,11 +241,11 @@ export class TypeTranslator {
           throw new Error(
               `reference loop in ${typeToDebugString(referenceType)} ${referenceType.flags}`);
         }
-        typeStr += notNullPrefix + this.translate(referenceType.target);
+        typeStr += this.translate(referenceType.target, notNull);
       }
       if (referenceType.typeArguments) {
-        let params = referenceType.typeArguments.map(t => this.translate(t, notNull));
-        typeStr += isTuple ? `Array` : `<${params.join(', ')}>`;
+        let params = referenceType.typeArguments.map(t => this.translate(t, true));
+        typeStr += isTuple ? notNullPrefix + `Array` : `<${params.join(', ')}>`;
       }
       return typeStr;
     } else if (type.flags & ts.TypeFlags.Anonymous) {
@@ -259,7 +259,7 @@ export class TypeTranslator {
       }
 
       if (type.symbol.flags === ts.SymbolFlags.TypeLiteral) {
-        return notNullPrefix + this.translateTypeLiteral(type);
+        return this.translateTypeLiteral(type, notNull);
       } else if (
           type.symbol.flags === ts.SymbolFlags.Function ||
           type.symbol.flags === ts.SymbolFlags.Method) {
@@ -272,7 +272,7 @@ export class TypeTranslator {
       return '?';
     } else if (type.flags & ts.TypeFlags.Union) {
       let unionType = type as ts.UnionType;
-      let parts = unionType.types.map(t => this.translate(t));
+      let parts = unionType.types.map(t => this.translate(t, true));
       // In union types that include boolean literal and other literals can
       // end up repeating the same closure type. For example: true | boolean
       // will be translated to boolean | boolean. Remove duplicates to produce
@@ -284,7 +284,8 @@ export class TypeTranslator {
     return '?';
   }
 
-  private translateTypeLiteral(type: ts.Type): string {
+  private translateTypeLiteral(type: ts.Type, notNull: boolean): string {
+    const notNullPrefix = notNull ? '!' : '';
     // Avoid infinite loops on recursive types.
     // It would be nice to just emit the name of the recursive type here,
     // but type.symbol doesn't seem to have the name here (perhaps something
@@ -308,7 +309,8 @@ export class TypeTranslator {
       // (not expressible in Closure), nor multiple constructors (same).
       const params = this.convertParams(ctors[0]);
       const paramsStr = params.length ? (', ' + params.join(', ')) : '';
-      return `function(new: ${this.translate(ctors[0].getReturnType())}${paramsStr}): ?`;
+      const constructedType = this.translate(ctors[0].getReturnType(), false);
+      return `function(new: ${constructedType}${paramsStr}): ?`;
     }
 
     for (let field of Object.keys(type.symbol.members)) {
@@ -321,12 +323,9 @@ export class TypeTranslator {
           break;
         default:
           let member = type.symbol.members[field];
-          let isOptional = member.flags & ts.SymbolFlags.Optional;
+          // optional members are handled by the type including |undefined in a union type.
           let memberType =
-              this.translate(this.typeChecker.getTypeOfSymbolAtLocation(member, this.node));
-          if (isOptional) {
-            memberType = `(${memberType}|undefined)`;
-          }
+              this.translate(this.typeChecker.getTypeOfSymbolAtLocation(member, this.node), true);
           fields.push(`${field}: ${memberType}`);
       }
     }
@@ -349,13 +348,13 @@ export class TypeTranslator {
         }
         if (!valType) {
           this.warn('unknown index key type');
-          return `Object<?,?>`;
+          return notNullPrefix + `Object<?,?>`;
         }
-        return `Object<${keyType},${this.translate(valType)}>`;
+        return notNullPrefix + `Object<${keyType},${this.translate(valType, true)}>`;
       } else if (!callable && !indexable) {
         // Special-case the empty object {} because Closure doesn't like it.
         // TODO(evanm): revisit this if it is a problem.
-        return 'Object';
+        return notNullPrefix + 'Object';
       }
     }
 
@@ -373,7 +372,7 @@ export class TypeTranslator {
     let params = this.convertParams(sig);
     let typeStr = `function(${params.join(', ')})`;
 
-    let retType = this.translate(this.typeChecker.getReturnTypeOfSignature(sig));
+    let retType = this.translate(this.typeChecker.getReturnTypeOfSignature(sig), true);
     if (retType) {
       typeStr += `: ${retType}`;
     }
@@ -382,8 +381,10 @@ export class TypeTranslator {
   }
 
   private convertParams(sig: ts.Signature): string[] {
-    return sig.parameters.map(
-        param => this.translate(this.typeChecker.getTypeOfSymbolAtLocation(param, this.node)));
+    return sig.parameters.map(param => {
+      let paramType = this.typeChecker.getTypeOfSymbolAtLocation(param, this.node);
+      return this.translate(paramType, true);
+    });
   }
 
   warn(msg: string) {

test/test_support.ts

@@ -27,6 +27,7 @@ const compilerOptions: ts.CompilerOptions = {
   // Flags below are needed to make sure source paths are correctly set on write calls.
   rootDir: path.resolve(process.cwd()),
   outDir: '.',
+  strictNullChecks: true,
 };
 
 const {cachedLibPath, cachedLib} = (function() {

test_files/abstract/abstract.js

@@ -14,7 +14,7 @@ class Base {
     publicAbstract() { }
     /**
      * @abstract
-     * @param {Array<number>} x
+     * @param {!Array<number>} x
      * @return {void}
      */
     params(x) { }
@@ -52,7 +52,7 @@ class Derived extends Base {
      */
     publicAbstract() { }
     /**
-     * @param {Array<number>} x
+     * @param {!Array<number>} x
      * @return {void}
      */
     params(x) { }
@@ -65,4 +65,4 @@ class Derived extends Base {
      */
     hasReturnType() { return 3; }
 }
-let /** @type {Base} */ x = new Derived();
+let /** @type {!Base} */ x = new Derived();

test_files/abstract/abstract.tsickle.ts

@@ -15,7 +15,7 @@ simple() {}
 publicAbstract() {}
 /**
  * @abstract
- * @param {Array<number>} x
+ * @param {!Array<number>} x
  * @return {void}
  */
 params(x: number[]) {}
@@ -54,7 +54,7 @@ simple() {}
  */
 publicAbstract() {}
 /**
- * @param {Array<number>} x
+ * @param {!Array<number>} x
  * @return {void}
  */
 params(x: number[]): void { }
@@ -68,4 +68,4 @@ noReturnType() {}
 hasReturnType(): number { return 3; }
 }
 
-let /** @type {Base} */ x: Base = new Derived();
+let /** @type {!Base} */ x: Base = new Derived();

test_files/basic.untyped/basic.untyped.js

@@ -24,7 +24,7 @@ function Foo_tsickle_Closure_declarations() {
 // regardless of untyped.
 (function () {
     // With a type annotation:
-    let { a, b } = { a: null, b: null };
+    let { a, b } = { a: '', b: 0 };
 })();
 (function () {
     // Without a type annotation:

test_files/basic.untyped/basic.untyped.ts

@@ -16,9 +16,9 @@ class Foo {
 // regardless of untyped.
 (function() {
   // With a type annotation:
-  let {a, b}: {a:string, b:number} = {a:null, b:null};
+  let {a, b}: {a: string, b: number} = {a: '', b: 0};
 })();
 (function() {
   // Without a type annotation:
-  let {a, b} = {a:null, b:null};
+  let {a, b} = {a: null, b: null};
 })();

test_files/basic.untyped/basic.untyped.tsickle.ts

@@ -29,9 +29,9 @@ Foo.prototype.ctorArg;
 // regardless of untyped.
 (function() {
   // With a type annotation:
-  let {a, b}: {a:string, b:number} = {a:null, b:null};
+  let {a, b}: {a: string, b: number} = {a: '', b: 0};
 })();
 (function() {
   // Without a type annotation:
-  let {a, b} = {a:null, b:null};
+  let {a, b} = {a: null, b: null};
 })();

test_files/declare/externs.js

@@ -84,14 +84,14 @@ Object.prototype.myMethod = function() {};
 
 /**
  * @param {string} x
- * @return {CodeMirror.Editor}
+ * @return {!CodeMirror.Editor}
  */
 function CodeMirror(x) {}
 
 /**
  * @param {number} y
  * @param {string} x
- * @return {CodeMirror.Editor}
+ * @return {!CodeMirror.Editor}
  */
 function CodeMirror(y, x) {}
 

test_files/declare_class_overloads/externs.js

@@ -31,23 +31,23 @@ function MultipleConstructorsNamesAndTypes(a_or_b) {}
  * @param {number} a
  * @param {number} b
  * @param {number} c
- * @param {string|Array<string>} normal_or_vertexNormals
- * @param {boolean|Array<boolean>} color_or_vertexColors
- * @param {number} materialIndex
+ * @param {(undefined|string)|(undefined|!Array<string>)} normal_or_vertexNormals
+ * @param {(undefined|boolean)|(undefined|!Array<boolean>)} color_or_vertexColors
+ * @param {(undefined|number)} materialIndex
  */
 function MultipleConstructorsComplexMatrix(a, b, c, normal_or_vertexNormals, color_or_vertexColors, materialIndex) {}
 
 /**
  * @constructor
  * @struct
- * @param {number|Array<number>} a
+ * @param {number|!Array<number>} a
  */
 function MultipleConstructorsVariadic(a) {}
 
 /**
  * @constructor
  * @struct
- * @param {Array<string>|Array<number>|string|number} points
+ * @param {!Array<string>|!Array<number>|string|number} points
  */
 function MultipleConstructorsVariadicNames(points) {}
 
@@ -97,7 +97,7 @@ OverloadReturnTypesWithVoid.prototype.overloaded = function(a, opt_b, opt_c) {};
 function OverloadBigMix() {}
 
 /**
- * @param {string|number|Array<OverloadBigMix>|OverloadBigMix} a_or_c_or_e_or_f
+ * @param {string|number|!Array<!OverloadBigMix>|!OverloadBigMix} a_or_c_or_e_or_f
  * @param {number} opt_b
  * @return {void|number|boolean}
  */
@@ -115,7 +115,7 @@ OverloadValueOf.prototype.valueOf = function() {};
 function Merged() {}
 
 /**
- * @param {(string|number|Array<OverloadBigMix>)} a_or_c_or_e_or_f
+ * @param {(string|number|!Array<!OverloadBigMix>)} a_or_c_or_e_or_f
  * @param {number} opt_b
  * @return {(number|boolean|void)}
  */

test_files/decorator/decorator.js

@@ -1,11 +1,11 @@
 goog.module('test_files.decorator.decorator'); exports = {}; var module = module || {id: 'test_files/decorator/decorator.js'};/**
- * @param {Object} a
+ * @param {!Object} a
  * @param {string} b
  * @return {void}
  */
 function decorator(a, b) { }
 /**
- * @param {Object} a
+ * @param {!Object} a
  * @param {string} b
  * @return {void}
  */
@@ -35,12 +35,12 @@ __decorate([
     __metadata('design:type', Number)
 ], DecoratorTest.prototype, "x", void 0);
 function DecoratorTest_tsickle_Closure_declarations() {
-    /** @type {Array<DecoratorInvocation>} */
+    /** @type {!Array<!DecoratorInvocation>} */
     DecoratorTest.decorators;
     /** @nocollapse
-    @type {Array<{type: ?, decorators: (Array<DecoratorInvocation>|undefined)}>} */
+    @type {!Array<(null|{type: ?, decorators: (undefined|!Array<!DecoratorInvocation>)})>} */
     DecoratorTest.ctorParameters;
-    /** @type {Object<string,Array<DecoratorInvocation>>} */
+    /** @type {!Object<string,!Array<!DecoratorInvocation>>} */
     DecoratorTest.propDecorators;
     /** @type {number} */
     DecoratorTest.prototype.x;
@@ -59,7 +59,7 @@ function DecoratedClass_tsickle_Closure_declarations() {
 }
 /** @record */
 function DecoratorInvocation() { }
-/** @type {Function} */
+/** @type {!Function} */
 DecoratorInvocation.prototype.type;
-/** @type {Array<?>} */
+/** @type {(undefined|!Array<?>)} */
 DecoratorInvocation.prototype.args;