fix(docs): improve docs rendering (#303)

Fixes #301: reject non-optional arguments after optional arguments
Fixes #298: show default values for optional properties
Fixes #302: omit "abstract" from interface properties (they are by definition abstract)
Fixes #300: indicate optionals with "optional" instead of "or undefined"
Fixes #299: render multiple jsdoc lines as multiple lines in sphinx

packages/jsii-calc/lib/compliance.ts

@@ -241,7 +241,7 @@ export class RuntimeTypeChecking {
         arg1; arg2; arg3;
     }
 
-    public methodWithDefaultedArguments(arg1: number = 2, arg2: string, arg3: Date = new Date()) {
+    public methodWithDefaultedArguments(arg1: number = 2, arg2?: string, arg3: Date = new Date()) {
         arg1; arg2; arg3;
     }
 
@@ -258,7 +258,7 @@ export class OptionalConstructorArgument {
 
 export class DefaultedConstructorArgument {
     public constructor(public readonly arg1: number = 2,
-                       public readonly arg2: string,
+                       public readonly arg2?: string,
                        public readonly arg3: Date = new Date()) {}
 }
 
@@ -994,3 +994,65 @@ export interface NullShouldBeTreatedAsUndefinedData {
     arrayWithThreeElementsAndUndefinedAsSecondArgument: any[];
 }
 
+/**
+ * jsii#298: show default values in sphinx documentation, and respect newlines.
+ **/
+export interface LoadBalancedFargateServiceProps {
+    /**
+     * The number of cpu units used by the task.
+     * Valid values, which determines your range of valid values for the memory parameter:
+     * 256 (.25 vCPU) - Available memory values: 0.5GB, 1GB, 2GB
+     * 512 (.5 vCPU) - Available memory values: 1GB, 2GB, 3GB, 4GB
+     * 1024 (1 vCPU) - Available memory values: 2GB, 3GB, 4GB, 5GB, 6GB, 7GB, 8GB
+     * 2048 (2 vCPU) - Available memory values: Between 4GB and 16GB in 1GB increments
+     * 4096 (4 vCPU) - Available memory values: Between 8GB and 30GB in 1GB increments
+     *
+     * This default is set in the underlying FargateTaskDefinition construct.
+     *
+     * @default 256
+     */
+    cpu?: string;
+
+    /**
+     * The amount (in MiB) of memory used by the task.
+     *
+     * This field is required and you must use one of the following values, which determines your range of valid values
+     * for the cpu parameter:
+     *
+     * 0.5GB, 1GB, 2GB - Available cpu values: 256 (.25 vCPU)
+     *
+     * 1GB, 2GB, 3GB, 4GB - Available cpu values: 512 (.5 vCPU)
+     *
+     * 2GB, 3GB, 4GB, 5GB, 6GB, 7GB, 8GB - Available cpu values: 1024 (1 vCPU)
+     *
+     * Between 4GB and 16GB in 1GB increments - Available cpu values: 2048 (2 vCPU)
+     *
+     * Between 8GB and 30GB in 1GB increments - Available cpu values: 4096 (4 vCPU)
+     *
+     * This default is set in the underlying FargateTaskDefinition construct.
+     *
+     * @default 512
+     */
+    memoryMiB?: string;
+
+    /**
+     * The container port of the application load balancer attached to your Fargate service. Corresponds to container port mapping.
+     *
+     * @default 80
+     */
+    containerPort?: number;
+
+    /**
+     * Determines whether the Application Load Balancer will be internet-facing
+     *
+     * @default true
+     */
+    publicLoadBalancer?: boolean;
+
+    /**
+     * Determines whether your Fargate Service will be assigned a public IP address.
+     *
+     * @default false
+     */
+    publicTasks?: boolean;
+}

packages/jsii-calc/test/assembly.jsii

@@ -1114,6 +1114,7 @@
           {
             "name": "arg2",
             "type": {
+              "optional": true,
               "primitive": "string"
             }
           },
@@ -1138,16 +1139,17 @@
         },
         {
           "immutable": true,
-          "name": "arg2",
+          "name": "arg3",
           "type": {
-            "primitive": "string"
+            "primitive": "date"
           }
         },
         {
           "immutable": true,
-          "name": "arg3",
+          "name": "arg2",
           "type": {
-            "primitive": "date"
+            "optional": true,
+            "primitive": "string"
           }
         }
       ]
@@ -1996,6 +1998,78 @@
         }
       ]
     },
+    "jsii-calc.LoadBalancedFargateServiceProps": {
+      "assembly": "jsii-calc",
+      "datatype": true,
+      "docs": {
+        "comment": "jsii#298: show default values in sphinx documentation, and respect newlines."
+      },
+      "fqn": "jsii-calc.LoadBalancedFargateServiceProps",
+      "kind": "interface",
+      "name": "LoadBalancedFargateServiceProps",
+      "properties": [
+        {
+          "abstract": true,
+          "docs": {
+            "comment": "The container port of the application load balancer attached to your Fargate service. Corresponds to container port mapping.",
+            "default": "80"
+          },
+          "name": "containerPort",
+          "type": {
+            "optional": true,
+            "primitive": "number"
+          }
+        },
+        {
+          "abstract": true,
+          "docs": {
+            "comment": "The number of cpu units used by the task.\nValid values, which determines your range of valid values for the memory parameter:\n256 (.25 vCPU) - Available memory values: 0.5GB, 1GB, 2GB\n512 (.5 vCPU) - Available memory values: 1GB, 2GB, 3GB, 4GB\n1024 (1 vCPU) - Available memory values: 2GB, 3GB, 4GB, 5GB, 6GB, 7GB, 8GB\n2048 (2 vCPU) - Available memory values: Between 4GB and 16GB in 1GB increments\n4096 (4 vCPU) - Available memory values: Between 8GB and 30GB in 1GB increments\n\nThis default is set in the underlying FargateTaskDefinition construct.",
+            "default": "256"
+          },
+          "name": "cpu",
+          "type": {
+            "optional": true,
+            "primitive": "string"
+          }
+        },
+        {
+          "abstract": true,
+          "docs": {
+            "comment": "The amount (in MiB) of memory used by the task.\n\nThis field is required and you must use one of the following values, which determines your range of valid values\nfor the cpu parameter:\n\n0.5GB, 1GB, 2GB - Available cpu values: 256 (.25 vCPU)\n\n1GB, 2GB, 3GB, 4GB - Available cpu values: 512 (.5 vCPU)\n\n2GB, 3GB, 4GB, 5GB, 6GB, 7GB, 8GB - Available cpu values: 1024 (1 vCPU)\n\nBetween 4GB and 16GB in 1GB increments - Available cpu values: 2048 (2 vCPU)\n\nBetween 8GB and 30GB in 1GB increments - Available cpu values: 4096 (4 vCPU)\n\nThis default is set in the underlying FargateTaskDefinition construct.",
+            "default": "512"
+          },
+          "name": "memoryMiB",
+          "type": {
+            "optional": true,
+            "primitive": "string"
+          }
+        },
+        {
+          "abstract": true,
+          "docs": {
+            "comment": "Determines whether the Application Load Balancer will be internet-facing",
+            "default": "true"
+          },
+          "name": "publicLoadBalancer",
+          "type": {
+            "optional": true,
+            "primitive": "boolean"
+          }
+        },
+        {
+          "abstract": true,
+          "docs": {
+            "comment": "Determines whether your Fargate Service will be assigned a public IP address.",
+            "default": "false"
+          },
+          "name": "publicTasks",
+          "type": {
+            "optional": true,
+            "primitive": "boolean"
+          }
+        }
+      ]
+    },
     "jsii-calc.Multiply": {
       "assembly": "jsii-calc",
       "base": {
@@ -2713,6 +2787,7 @@
             {
               "name": "arg2",
               "type": {
+                "optional": true,
                 "primitive": "string"
               }
             },
@@ -3537,5 +3612,5 @@
     }
   },
   "version": "0.7.8",
-  "fingerprint": "FZk0ePQ2XUte84CmnOjU3PPCl6QUA88ke6wHIJKhyzo="
+  "fingerprint": "3UAOWGBZzphiNp40hKAbXcsM27uWdwmXnbiPU74ZroU="
 }

packages/jsii-pacmak/lib/targets/sphinx.ts

@@ -254,12 +254,12 @@ class SphinxDocsGenerator extends Generator {
     //
     // Properties
 
-    protected onProperty(_cls: spec.ClassType, prop: spec.Property) {
-        this.renderProperty(prop);
+    protected onProperty(cls: spec.ClassType, prop: spec.Property) {
+        this.renderProperty(cls, prop);
     }
 
-    protected onStaticProperty(_cls: spec.ClassType, prop: spec.Property) {
-        this.renderProperty(prop);
+    protected onStaticProperty(cls: spec.ClassType, prop: spec.Property) {
+        this.renderProperty(cls, prop);
     }
 
     //
@@ -357,8 +357,8 @@ class SphinxDocsGenerator extends Generator {
         this.onInterfaceMethod(ifc, overload);
     }
 
-    protected onInterfaceProperty(_ifc: spec.InterfaceType, property: spec.Property) {
-        this.renderProperty(property);
+    protected onInterfaceProperty(ifc: spec.InterfaceType, property: spec.Property) {
+        this.renderProperty(ifc, property);
     }
 
     private renderInheritedMembers(entity: spec.ClassType | spec.InterfaceType) {
@@ -373,7 +373,7 @@ class SphinxDocsGenerator extends Generator {
                 }
             }
             for (const property of entities.properties) {
-                this.renderProperty(property, source);
+                this.renderProperty(entity, property, source);
             }
         }
     }
@@ -543,13 +543,13 @@ class SphinxDocsGenerator extends Generator {
     }
 
     private renderDocsLine(element: spec.Documentable) {
-        const doclines = this.renderDocs(element, false);
+        const doclines = this.renderDocs(element, true);
         if (doclines.length === 0) {
             return;
         }
 
         this.code.line();
-        doclines.split('\n').forEach(line => this.code.line(line));
+        doclines.split('\n').forEach(line => this.code.line(line + '\n'));
 
         if (element.docs && element.docs.link) {
             this.code.line(element.docs.link);
@@ -618,7 +618,7 @@ class SphinxDocsGenerator extends Generator {
         } else {
             throw new Error('Unexpected type ref');
         }
-        if (type.optional) { result.ref = `${result.ref} or \`\`undefined\`\``; }
+        if (type.optional) { result.ref = `${result.ref} *(optional)*`; }
         return result;
 
         // Wrap a string between parenthesis if it contains " or "
@@ -628,7 +628,7 @@ class SphinxDocsGenerator extends Generator {
         }
     }
 
-    private renderProperty(prop: spec.Property, inheritedFrom?: string) {
+    private renderProperty(parent: spec.TypeBase, prop: spec.Property, inheritedFrom?: string) {
         this.code.line();
         const type = this.renderTypeRef(prop.type);
         this.code.openBlock(`.. py:attribute:: ${prop.name}`);
@@ -651,9 +651,14 @@ class SphinxDocsGenerator extends Generator {
             this.code.line();
         }
         const readonly = prop.immutable ? ' *(readonly)*' : '';
-        const abs = prop.abstract ? ' *(abstract)*' : '';
+        const abs = (parent.kind !== spec.TypeKind.Interface &&  prop.abstract) ? ' *(abstract)*' : '';
         const stat = prop.static ? ' *(static)*' : '';
         this.code.line(`:type: ${type.ref}${readonly}${abs}${stat}`);
+
+        if (prop.docs && prop.docs.default) {
+            this.code.line(`:default: ${prop.docs.default}`);
+        }
+
         this.code.closeBlock();
     }
 

packages/jsii-pacmak/test/expected.jsii-calc-base/sphinx/_scope_jsii-calc-base.rst

@@ -153,6 +153,7 @@ Base
    A base class.
 
 
+
    :abstract: Yes
 
    .. py:method:: typeName() -> any
@@ -193,13 +194,13 @@ BaseProps (interface)
 
    .. py:attribute:: bar
 
-      :type: string *(abstract)*
+      :type: string
 
 
    .. py:attribute:: foo
 
       *Inherited from* :py:attr:`@scope/jsii-calc-base-of-base.VeryBaseProps <@scope/jsii-calc-base-of-base.VeryBaseProps.foo>`
 
-      :type: :py:class:`@scope/jsii-calc-base-of-base.Very`\  *(abstract)*
+      :type: :py:class:`@scope/jsii-calc-base-of-base.Very`\