emit full jsdoc for function types

Rather than emitting a type per parameter, emit a jsdoc
before the function declaration.  This allows us to mark optional
arguments properly with the = suffix.  Note the changed "optional.ts"
test; it now verifies that Closure accepts a missing argument when
that argument was missing before.

Fixes #43.

Author: evmar

src/sickle.ts

@@ -73,7 +73,7 @@ class Annotator {
     this.indent++;
     switch (node.kind) {
       case ts.SyntaxKind.VariableDeclaration:
-        this.maybeVisitType((<ts.VariableDeclaration>node).type);
+        this.maybeEmitJSDocType((<ts.VariableDeclaration>node).type);
         this.writeNode(node);
         break;
       case ts.SyntaxKind.ClassDeclaration: {
@@ -124,25 +124,33 @@ class Annotator {
       case ts.SyntaxKind.FunctionDeclaration:
       case ts.SyntaxKind.ArrowFunction:
         let fnDecl = <ts.FunctionLikeDeclaration>node;
-        this.maybeVisitType(fnDecl.type, '@return');
         let writeOffset = fnDecl.getFullStart();
+        writeOffset = this.writeRange(writeOffset, fnDecl.getStart());
+        // The first \n makes the output sometimes uglier than necessary,
+        // but it's needed to work around
+        // https://github.com/Microsoft/TypeScript/issues/6982
+        this.emit('\n/**\n');
         // Parameters.
         if (fnDecl.parameters.length) {
           for (let param of fnDecl.parameters) {
-            this.writeTextFromOffset(writeOffset, param);
-            writeOffset = param.getEnd();
-            let optional = param.initializer != null || param.questionToken != null;
-            this.maybeVisitType(param.type, null, optional);
-            this.visit(param);
+            if (param.type) {
+              let optional = param.initializer != null || param.questionToken != null;
+              this.emit(' * @param {');
+              this.emitType(param.type, optional);
+              this.emit('} ');
+              this.writeNode(param.name);
+              this.emit('\n');
+            }
           }
         }
         // Return type.
         if (fnDecl.type) {
-          this.writeTextFromOffset(writeOffset, fnDecl.type);
-          this.visit(fnDecl.type);
-          writeOffset = fnDecl.type.getEnd();
+          this.emit(' * @return {');
+          this.emitType(fnDecl.type);
+          this.emit('}\n');
         }
-        // Body.
+        this.emit(' */\n');
+
         this.writeTextFromOffset(writeOffset, fnDecl.body);
         this.visit(fnDecl.body);
         break;
@@ -203,7 +211,7 @@ class Annotator {
     if (existingAnnotation) {
       existingAnnotation += '\n';
     }
-    this.maybeVisitType(p.type, existingAnnotation + '@type');
+    this.maybeEmitJSDocType(p.type, existingAnnotation + '@type');
     this.emit('\nthis.');
     this.emit(p.name.getText());
     this.emit(';');
@@ -233,14 +241,22 @@ class Annotator {
     return '';
   }
 
-  private maybeVisitType(type: ts.TypeNode, jsDocTag?: string, optional?: boolean) {
+  private maybeEmitJSDocType(type: ts.TypeNode, jsDocTag?: string) {
     if (!type && !this.options.untyped) return;
     this.emit(' /**');
     if (jsDocTag) {
       this.emit(' ');
       this.emit(jsDocTag);
       this.emit(' {');
     }
+    this.emitType(type);
+    if (jsDocTag) {
+      this.emit('}');
+    }
+    this.emit(' */');
+  }
+
+  private emitType(type: ts.TypeNode, optional?: boolean) {
     if (this.options.untyped) {
       this.emit(' ?');
     } else {
@@ -249,10 +265,6 @@ class Annotator {
     if (optional) {
       this.emit('=');
     }
-    if (jsDocTag) {
-      this.emit('}');
-    }
-    this.emit(' */');
   }
 
   private visitTypeAlias(node: ts.TypeAliasDeclaration) {

test_files/es6/arrow_fn.js

@@ -1 +1,10 @@
-var fn3 = (/** number */ a) => 12;
+var fn3 = 
+/**
+ * @param { number} a
+ * @return { number}
+ */
+/**
+ * @param { number} a
+ * @return { number}
+ */
+    (a) => 12;

test_files/es6/basic.untyped.js

@@ -1,6 +1,10 @@
-/** @return { ?} */ // This test is just a random collection of typed code, to
+// This test is just a random collection of typed code, to
 // ensure the output is all with {?} annotations.
-function func(/** ? */ arg1) {
+/**
+ * @param { ?} arg1
+ * @return { ?}
+ */
+function func(arg1) {
     return [3];
 }
 class Foo {

test_files/es6/decorator.js

@@ -7,7 +7,11 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key,
 var __metadata = (this && this.__metadata) || function (k, v) {
     if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
 };
-function decorator(/** Object */ a, /** string */ b) { }
+/**
+ * @param { Object} a
+ * @param { string}  b
+ */
+function decorator(a, b) { }
 class DecoratorTest {
     // Sickle: begin synthetic ctor.
     constructor() {

test_files/es6/default.js

@@ -1,2 +1,6 @@
-function DefaultArgument(/** number */ x, /** string= */ y = 'hi') {
+/**
+ * @param { number} x
+ * @param { string=}  y
+ */
+function DefaultArgument(x, y = 'hi') {
 }

test_files/es6/file_comment.js

@@ -1,4 +1,7 @@
-/** @return { string} */ // This test verifies that initial comments don't confuse offsets.
+// This test verifies that initial comments don't confuse offsets.
+/**
+ * @return { string}
+ */
 function foo() {
     return 'foo';
 }

test_files/es6/functions.js

@@ -1,4 +1,12 @@
-/** @return { string} */ function fn1(/** number */ a) {
+/**
+ * @param { number} a
+ * @return { string}
+ */
+function fn1(a) {
     return "a";
 }
-function fn2(/** number */ a, /** number */ b) { }
+/**
+ * @param { number} a
+ * @param { number}  b
+ */
+function fn2(a, b) { }

test_files/es6/optional.js

@@ -1,2 +1,7 @@
-function optionalArgument(/** number */ x, /** string= */ y) {
+/**
+ * @param { number} x
+ * @param { string=}  y
+ */
+function optionalArgument(x, y) {
 }
+optionalArgument(1);

test_files/optional.ts

@@ -1,2 +1,3 @@
 function optionalArgument(x: number, y?: string) {
 }
+optionalArgument(1);

test_files/sickle/arrow_fn.ts

@@ -1 +1,6 @@
-var fn3 = /** @return { number} */ ( /** number */a: number): number => 12;
+var fn3 = 
+/**
+ * @param { number} a
+ * @return { number}
+ */
+(a: number): number => 12;

test_files/sickle/basic.untyped.ts

@@ -1,6 +1,11 @@
- /** @return { ?} */// This test is just a random collection of typed code, to
+// This test is just a random collection of typed code, to
 // ensure the output is all with {?} annotations.
-function func( /** ? */arg1: string): number[] {
+
+/**
+ * @param { ?} arg1
+ * @return { ?}
+ */
+function func(arg1: string): number[] {
   return [3];
 }
 

test_files/sickle/decorator.ts

@@ -1,4 +1,9 @@
-function decorator( /** Object */a: Object, /** string */ b: string) {}
+
+/**
+ * @param { Object} a
+ * @param { string}  b
+ */
+function decorator(a: Object, b: string) {}
 
 class DecoratorTest {
   @decorator

test_files/sickle/default.ts

@@ -1,2 +1,7 @@
-function DefaultArgument( /** number */x: number, /** string= */ y: string = 'hi') {
+
+/**
+ * @param { number} x
+ * @param { string=}  y
+ */
+function DefaultArgument(x: number, y: string = 'hi') {
 }

test_files/sickle/file_comment.ts

@@ -1,4 +1,8 @@
- /** @return { string} */// This test verifies that initial comments don't confuse offsets.
+// This test verifies that initial comments don't confuse offsets.
+
+/**
+ * @return { string}
+ */
 function foo(): string {
   return 'foo';
 }

test_files/sickle/functions.ts

@@ -1,4 +1,14 @@
- /** @return { string} */function fn1( /** number */a: number): string {
+
+/**
+ * @param { number} a
+ * @return { string}
+ */
+function fn1(a: number): string {
   return "a";
 }
-function fn2( /** number */a: number, /** number */ b: number) {}
+
+/**
+ * @param { number} a
+ * @param { number}  b
+ */
+function fn2(a: number, b: number) {}

test_files/sickle/optional.ts

@@ -1,2 +1,8 @@
-function optionalArgument( /** number */x: number, /** string= */ y?: string) {
+
+/**
+ * @param { number} x
+ * @param { string=}  y
+ */
+function optionalArgument(x: number, y?: string) {
 }
+optionalArgument(1);