feat(jsii-pacmak): document union types for C# (#4928)

In some places union types are rendered to overloaded setters, and in other places the type is collapsed to `Object`. Whenever the type is collapsed to `Object`, any other type information is lost for the user.

In this PR, emit the union type information to XML docs so users can find it again.

Also update the JavaDoc handler to handle type intersections.


---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

Author: rix0rrr

packages/@jsii/spec/src/assembly.ts

@@ -844,15 +844,25 @@ export interface Method extends Callable {
    */
   static?: boolean;
 }
+
 /**
- * Determines whether a Callable is a Method or not.
+ * Determines whether a Callable is a Method or not (if not, it's an initializer)
  *
  * @param callable the callable to be checked.
  */
 export function isMethod(callable: Callable): callable is Method {
   return !!(callable as Method).name;
 }
 
+/**
+ * Returns whether an API element looks like a property
+ */
+export function isProperty(
+  member: Type | Callable | Property,
+): member is Property {
+  return !!(member as Property).name && !!(member as Property).type;
+}
+
 /**
  * Represents a type definition (not a type reference).
  */

packages/jsii-pacmak/lib/targets/dotnet/dotnetdocgenerator.ts

@@ -10,8 +10,21 @@ import {
 import * as xmlbuilder from 'xmlbuilder';
 
 import { renderSummary } from '../_utils';
+import { DotNetTypeResolver } from './dotnettyperesolver';
 import { DotNetNameUtils } from './nameutils';
 import { assertSpecIsRosettaCompatible } from '../../rosetta-assembly';
+import { containsUnionType } from '../../type-utils';
+import { visitTypeReference } from '../../type-visitor';
+
+// Define some tokens that will be turned into literal < and > in XML comments.
+// This will be used by a function later on that needs to output literal tokens,
+// in a string where they would usually be escaped into &lt; and &gt;
+//
+// We use a random string in here so the actual token values cannot be predicted
+// in advance, so that an attacker can not use this knowledge to inject the tokens
+// literally into  doc comments, and perform an XSS attack that way.
+const L_ANGLE = `@l${Math.random()}@`;
+const R_ANGLE = `@r${Math.random()}@`;
 
 /**
  * Generates the Jsii attributes and calls for the .NET runtime
@@ -26,6 +39,7 @@ export class DotNetDocGenerator {
     code: CodeMaker,
     private readonly rosetta: RosettaTabletReader,
     private readonly assembly: spec.Assembly,
+    private readonly resolver: DotNetTypeResolver,
   ) {
     this.code = code;
   }
@@ -54,32 +68,57 @@ export class DotNetDocGenerator {
         const paramName = this.nameutils
           .convertParameterName(param.name)
           .replace(/^@/, '');
-        this.emitXmlDoc('param', param.docs?.summary ?? '', {
-          attributes: { name: paramName },
-        });
+
+        const unionHint = containsUnionType(param.type)
+          ? `Type union: ${this.renderTypeForDocs(param.type)}`
+          : '';
+
+        this.emitXmlDoc(
+          'param',
+          combineSentences(param.docs?.summary, unionHint),
+          {
+            attributes: { name: paramName },
+          },
+        );
       });
     }
 
-    // At this pdocfx namespacedocd a valid instance of docs
-    if (!docs) {
-      return;
-    }
+    const returnUnionHint =
+      objMethod.returns && containsUnionType(objMethod.returns.type)
+        ? `Type union: ${this.renderTypeForDocs(objMethod.returns.type)}`
+        : '';
 
-    if (docs.returns) {
-      this.emitXmlDoc('returns', docs.returns);
+    if (docs?.returns || returnUnionHint) {
+      this.emitXmlDoc(
+        'returns',
+        combineSentences(docs?.returns, returnUnionHint),
+      );
     }
 
+    const propUnionHint =
+      spec.isProperty(obj) && containsUnionType(obj.type)
+        ? `Type union: ${this.renderTypeForDocs(obj.type)}`
+        : '';
+
     // Remarks does not use emitXmlDoc() because the remarks can contain code blocks
     // which are fenced with <code> tags, which would be escaped to
     // &lt;code&gt; if we used the xml builder.
-    const remarks = this.renderRemarks(docs, apiLocation);
-    if (remarks.length > 0) {
+    const remarks = this.renderRemarks(docs ?? {}, apiLocation);
+    if (remarks.length > 0 || propUnionHint) {
       this.code.line('/// <remarks>');
       remarks.forEach((r) => this.code.line(`/// ${r}`.trimRight()));
+
+      if (propUnionHint) {
+        // Very likely to contain < and > from `Dictionary<...>`, but we also want the literal angle brackets
+        // from `<see cref="...">`.
+        this.code.line(
+          `/// <para>${unescapeAngleMarkers(escapeAngleBrackets(propUnionHint))}</para>`,
+        );
+      }
       this.code.line('/// </remarks>');
     }
 
-    if (docs.example) {
+    if (docs?.example) {
       this.code.line('/// <example>');
       this.emitXmlDoc('code', this.convertExample(docs.example, apiLocation));
       this.code.line('/// </example>');
@@ -193,14 +232,39 @@ export class DotNetDocGenerator {
     for (const [name, value] of Object.entries(attributes)) {
       xml.att(name, value);
     }
-    const xmlstring = xml.end({ allowEmpty: true, pretty: false });
+
+    // Unescape angle brackets that may have been injected by `renderTypeForDocs`
+    const xmlstring = unescapeAngleMarkers(
+      xml.end({ allowEmpty: true, pretty: false }),
+    );
+
     const trimLeft = tag !== 'code';
     for (const line of xmlstring
       .split('\n')
       .map((x) => (trimLeft ? x.trim() : x.trimRight()))) {
       this.code.line(`/// ${line}`);
     }
   }
+
+  private renderTypeForDocs(x: spec.TypeReference): string {
+    return visitTypeReference<string>(x, {
+      named: (ref) =>
+        `${L_ANGLE}see cref="${this.resolver.toNativeFqn(ref.fqn)}" /${R_ANGLE}`,
+      primitive: (ref) => this.resolver.toDotNetType(ref),
+      collection: (ref) => {
+        switch (ref.collection.kind) {
+          case spec.CollectionKind.Array:
+            return `(${this.renderTypeForDocs(ref.collection.elementtype)})[]`;
+          case spec.CollectionKind.Map:
+            return `Dictionary<string, ${this.renderTypeForDocs(ref.collection.elementtype)}>`;
+        }
+      },
+      union: (ref) =>
+        `either ${ref.union.types.map((x) => this.renderTypeForDocs(x)).join(' or ')}`,
+      intersection: (ref) =>
+        `${ref.intersection.types.map((x) => this.renderTypeForDocs(x)).join(' + ')}`,
+    });
+  }
 }
 
 /**
@@ -214,3 +278,20 @@ function shouldMentionStability(s: spec.Stability) {
   // Don't render "stable" or "external", those are both stable by implication
   return s === spec.Stability.Deprecated || s === spec.Stability.Experimental;
 }
+
+function combineSentences(...xs: Array<string | undefined>): string {
+  return xs.filter((x) => x).join('. ');
+}
+
+function escapeAngleBrackets(x: string) {
+  return x.replace(/</g, '&lt;').replace(/>/g, '&gt;');
+}
+
+/**
+ * Replace the special angle markers produced by renderTypeForDocs with literal angle brackets
+ */
+function unescapeAngleMarkers(x: string) {
+  return x
+    .replace(new RegExp(L_ANGLE, 'g'), '<')
+    .replace(new RegExp(R_ANGLE, 'g'), '>');
+}

packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts

@@ -78,6 +78,7 @@ export class DotNetGenerator extends Generator {
       this.code,
       this.rosetta,
       this.assembly,
+      this.typeresolver,
     );
 
     this.emitAssemblyDocs();

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

@@ -31,6 +31,8 @@ import { VERSION, VERSION_DESC } from '../version';
 import { stabilityPrefixFor, renderSummary } from './_utils';
 import { toMavenVersionRange, toReleaseVersion } from './version-utils';
 import { assertSpecIsRosettaCompatible } from '../rosetta-assembly';
+import { containsUnionType } from '../type-utils';
+import { visitTypeReference } from '../type-visitor';
 
 import { TargetName } from './index';
 
@@ -3061,24 +3063,22 @@ class JavaGenerator extends Generator {
    * Render a type reference to something human readable for use in JavaDocs
    */
   private renderTypeReference(x: spec.TypeReference): string {
-    if (spec.isPrimitiveTypeReference(x)) {
-      return `{@link ${this.toJavaPrimitive(x.primitive)}}`;
-    }
-    if (spec.isNamedTypeReference(x)) {
-      return `{@link ${this.toNativeFqn(x.fqn)}}`;
-    }
-    if (spec.isCollectionTypeReference(x)) {
-      switch (x.collection.kind) {
-        case spec.CollectionKind.Array:
-          return `List<${this.renderTypeReference(x.collection.elementtype)}>`;
-        case spec.CollectionKind.Map:
-          return `Map<String, ${this.renderTypeReference(x.collection.elementtype)}>`;
-      }
-    }
-    if (spec.isUnionTypeReference(x)) {
-      return `either ${x.union.types.map((x) => this.renderTypeReference(x)).join(' or ')}`;
-    }
-    throw new Error(`Unknown type reference: ${JSON.stringify(x)}`);
+    return visitTypeReference<string>(x, {
+      primitive: (x) => `{@link ${this.toJavaPrimitive(x.primitive)}}`,
+      named: (x) => `{@link ${this.toNativeFqn(x.fqn)}}`,
+      collection: (x) => {
+        switch (x.collection.kind) {
+          case spec.CollectionKind.Array:
+            return `List<${this.renderTypeReference(x.collection.elementtype)}>`;
+          case spec.CollectionKind.Map:
+            return `Map<String, ${this.renderTypeReference(x.collection.elementtype)}>`;
+        }
+      },
+      union: (x) =>
+        `either ${x.union.types.map((x) => this.renderTypeReference(x)).join(' or ')}`,
+      intersection: (x) =>
+        `${x.intersection.types.map((x) => this.renderTypeReference(x)).join(' + ')}`,
+    });
   }
 
   private renderMethodCallArguments(method: spec.Callable) {
@@ -3732,16 +3732,6 @@ function escape(s: string) {
   return s.replace(/["\\<>&]/g, (c) => `&#${c.charCodeAt(0)};`);
 }
 
-function containsUnionType(
-  typeRef: spec.TypeReference,
-): typeRef is spec.UnionTypeReference | spec.CollectionTypeReference {
-  return (
-    spec.isUnionTypeReference(typeRef) ||
-    (spec.isCollectionTypeReference(typeRef) &&
-      containsUnionType(typeRef.collection.elementtype))
-  );
-}
-
 function myMarkDownToJavaDoc(source: string) {
   if (source.includes('{@link') || source.includes('{@code')) {
     // Slightly dirty hack: if we are seeing this, it means the docstring was provided literally

packages/jsii-pacmak/lib/type-utils.ts

@@ -0,0 +1,11 @@
+import * as spec from '@jsii/spec';
+
+export function containsUnionType(
+  typeRef: spec.TypeReference,
+): typeRef is spec.UnionTypeReference | spec.CollectionTypeReference {
+  return (
+    spec.isUnionTypeReference(typeRef) ||
+    (spec.isCollectionTypeReference(typeRef) &&
+      containsUnionType(typeRef.collection.elementtype))
+  );
+}

packages/jsii-pacmak/lib/type-visitor.ts

@@ -0,0 +1,57 @@
+import * as spec from '@jsii/spec';
+
+export interface TypeReferenceVisitor<A = void> {
+  named(ref: spec.NamedTypeReference): A;
+  primitive(ref: spec.PrimitiveTypeReference): A;
+  collection(ref: spec.CollectionTypeReference): A;
+  union(ref: spec.UnionTypeReference): A;
+  intersection(ref: spec.IntersectionTypeReference): A;
+}
+
+export function visitTypeReference<A>(
+  typeRef: spec.TypeReference,
+  visitor: TypeReferenceVisitor<A>,
+) {
+  if (spec.isNamedTypeReference(typeRef)) {
+    return visitor.named(typeRef);
+  } else if (spec.isPrimitiveTypeReference(typeRef)) {
+    return visitor.primitive(typeRef);
+  } else if (spec.isCollectionTypeReference(typeRef)) {
+    return visitor.collection(typeRef);
+  } else if (spec.isUnionTypeReference(typeRef)) {
+    return visitor.union(typeRef);
+  } else if (spec.isIntersectionTypeReference(typeRef)) {
+    return visitor.intersection(typeRef);
+  }
+  throw new Error(`Unknown type reference: ${JSON.stringify(typeRef)}`);
+}
+
+export interface TypeVisitor<A = void> {
+  classType(t: spec.ClassType): A;
+  interfaceType(t: spec.InterfaceType): A;
+  dataType(t: spec.InterfaceType): A;
+  enumType(t: spec.EnumType): A;
+}
+
+export function visitType<A>(t: spec.Type, visitor: TypeVisitor<A>) {
+  if (spec.isClassType(t)) {
+    return visitor.classType(t);
+  } else if (spec.isInterfaceType(t) && t.datatype) {
+    return visitor.dataType(t);
+  } else if (spec.isInterfaceType(t)) {
+    return visitor.interfaceType(t);
+  } else if (spec.isEnumType(t)) {
+    return visitor.enumType(t);
+  }
+  throw new Error(`Unknown type: ${JSON.stringify(t)}`);
+}
+
+export function isDataType(t: spec.Type): t is spec.InterfaceType {
+  return spec.isInterfaceType(t) && !!t.datatype;
+}
+
+export function isBehavioralInterfaceType(
+  t: spec.Type,
+): t is spec.InterfaceType {
+  return spec.isInterfaceType(t) && !t.datatype;
+}