chore(cfnspec): CloudFormation documentation in L1 classes, again (#18190)

Re-draft of #18101. The previous implementation rendered invalid
escape sequences which would break the Java rendering.

Fixed in this iteration.


----

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

Author: rix0rrr

packages/@aws-cdk/cfnspec/build-tools/build.ts

@@ -17,6 +17,7 @@ async function main() {
 
   await generateResourceSpecification(inputDir, path.join(outDir, 'specification.json'));
   await mergeSpecificationFromDirs(path.join(inputDir, 'cfn-lint'), path.join(outDir, 'cfn-lint.json'));
+  await fs.copyFile(path.join(inputDir, 'cfn-docs', 'cfn-docs.json'), path.join(outDir, 'cfn-docs.json'));
 }
 
 /**

packages/@aws-cdk/cfnspec/lib/index.ts

@@ -12,6 +12,15 @@ export function specification(): schema.Specification {
   return require('../spec/specification.json');
 }
 
+/**
+ * The complete AWS CloudFormation Resource specification, having any CDK patches and enhancements included in it.
+ */
+export function docs(): schema.CloudFormationDocsFile {
+  // eslint-disable-next-line @typescript-eslint/no-require-imports
+  return require('../spec/cfn-docs.json');
+}
+
+
 /**
  * Return the resource specification for the given typename
  *
@@ -26,6 +35,21 @@ export function resourceSpecification(typeName: string): schema.ResourceType {
   return ret;
 }
 
+/**
+ * Return documentation for the given type
+ */
+export function typeDocs(resourceName: string, propertyTypeName?: string): schema.CloudFormationTypeDocs {
+  const key = propertyTypeName ? `${resourceName}.${propertyTypeName}` : resourceName;
+  const ret = docs().Types[key];
+  if (!ret) {
+    return {
+      description: '',
+      properties: {},
+    };
+  }
+  return ret;
+}
+
 /**
  * Get the resource augmentations for a given type
  */

packages/@aws-cdk/cfnspec/lib/schema/docs.ts

@@ -0,0 +1,23 @@
+/**
+ * Docs for a CloudFormation resource or property type
+ */
+export interface CloudFormationTypeDocs {
+  /**
+   * Description for this type
+   */
+  readonly description: string;
+
+  /**
+   * Descriptions for each of the type's properties
+   */
+  readonly properties: Record<string, string>;
+
+  /**
+   * Descriptions for each of the resource's attributes
+   */
+  readonly attributes?: Record<string, string>;
+}
+
+export interface CloudFormationDocsFile {
+  readonly Types: Record<string, CloudFormationTypeDocs>;
+}

packages/@aws-cdk/cfnspec/lib/schema/index.ts

@@ -4,3 +4,4 @@ export * from './resource-type';
 export * from './specification';
 export * from './augmentation';
 export * from './cfn-lint';
+export * from './docs';

packages/@aws-cdk/cfnspec/test/docs.test.ts

@@ -0,0 +1,15 @@
+import * as cfnspec from '../lib';
+
+test('spot-check resource docs', () => {
+  const bucketDocs = cfnspec.typeDocs('AWS::S3::Bucket');
+
+  expect(bucketDocs.description).toBeTruthy();
+  expect(bucketDocs.properties.BucketName).toBeTruthy();
+});
+
+test('spot-check property type docs', () => {
+  const destDocs = cfnspec.typeDocs('AWS::S3::Bucket.Destination');
+
+  expect(destDocs.description).toBeTruthy();
+  expect(destDocs.properties.Format).toBeTruthy();
+});

tools/@aws-cdk/cfn2ts/lib/codegen.ts

@@ -1,4 +1,4 @@
-import { schema, cfnLintAnnotations } from '@aws-cdk/cfnspec';
+import { schema, cfnLintAnnotations, typeDocs } from '@aws-cdk/cfnspec';
 import { CodeMaker } from 'codemaker';
 import * as genspec from './genspec';
 import { itemTypeNames, PropertyAttributeName, scalarTypeNames, SpecName } from './spec-utils';
@@ -115,7 +115,7 @@ export default class CodeGenerator {
     const name = genspec.CodeName.forResourceProperties(resourceContext);
 
     this.docLink(spec.Documentation,
-      `Properties for defining a \`${resourceContext.specName!.fqn}\``,
+      `Properties for defining a \`${resourceContext.className}\``,
       '',
       '@stability external');
     this.code.openBlock(`export interface ${name.className}`);
@@ -144,15 +144,17 @@ export default class CodeGenerator {
     container: Container): Dictionary<string> {
     const propertyMap: Dictionary<string> = {};
 
+    const docs = typeDocs(resource.specName?.fqn ?? '');
+
     Object.keys(propertiesSpec).sort(propertyComparator).forEach(propName => {
       this.code.line();
       const propSpec = propertiesSpec[propName];
-      const additionalDocs = resource.specName!.relativeName(propName).fqn;
+      const additionalDocs = docs.properties[propName] || quoteCode(resource.specName!.relativeName(propName).fqn);
       const newName = this.emitProperty({
         context: resource,
         propName,
         spec: propSpec,
-        additionalDocs: quoteCode(additionalDocs),
+        additionalDocs,
       },
       container,
       );
@@ -192,15 +194,20 @@ export default class CodeGenerator {
       this.code.line();
     }
 
+    const docs = typeDocs(cfnName);
+
     //
     // The class declaration representing this Resource
     //
 
-    this.docLink(spec.Documentation,
+    this.docLink(spec.Documentation, ...[
       `A CloudFormation \`${cfnName}\``,
       '',
+      ...docs.description.split('\n'),
+      '',
       `@cloudformationResource ${cfnName}`,
-      '@stability external');
+      '@stability external',
+    ]);
     this.openClass(resourceName, RESOURCE_BASE_CLASS);
 
     //
@@ -271,7 +278,9 @@ export default class CodeGenerator {
 
         this.code.line();
 
-        this.docLink(undefined, `@cloudformationAttribute ${attributeName}`);
+        this.docLink(undefined,
+          docs.attributes?.[attributeName] ?? '',
+          `@cloudformationAttribute ${attributeName}`);
         const attr = genspec.attributeDefinition(attributeName, attributeSpec);
 
         this.code.line(`public readonly ${attr.propertyName}: ${attr.attributeType};`);
@@ -847,18 +856,25 @@ export default class CodeGenerator {
     this.code.line();
     this.beginNamespace(typeName);
 
-    this.docLink(propTypeSpec.Documentation, '@stability external');
+    const docs = typeDocs(resourceContext.specName?.fqn ?? '', (typeName.specName as PropertyAttributeName | undefined)?.propAttrName);
+
+    this.docLink(
+      propTypeSpec.Documentation,
+      docs.description,
+      '@stability external',
+    );
     /*
     if (!propTypeSpec.Properties || Object.keys(propTypeSpec.Properties).length === 0) {
       this.code.line('// eslint-disable-next-line somethingsomething | A genuine empty-object type');
     }
     */
     this.code.openBlock(`export interface ${typeName.className}`);
     const conversionTable: Dictionary<string> = {};
+
     if (propTypeSpec.Properties) {
       Object.keys(propTypeSpec.Properties).forEach(propName => {
         const propSpec = propTypeSpec.Properties[propName];
-        const additionalDocs = quoteCode(`${typeName.fqn}.${propName}`);
+        const additionalDocs = docs.properties[propName] || quoteCode(`${typeName.fqn}.${propName}`);
         const newName = this.emitInterfaceProperty({
           context: resourceContext,
           propName,
@@ -950,12 +966,37 @@ export default class CodeGenerator {
   private docLink(link: string | undefined, ...before: string[]): void {
     if (!link && before.length === 0) { return; }
     this.code.line('/**');
-    before.forEach(line => this.code.line(` * ${line}`.trimRight()));
+    before.flatMap(x => x.split('\n')).forEach(line => this.code.line(` * ${escapeDocText(line)}`.trimRight()));
     if (link) {
+      if (before.length > 0) {
+        this.code.line(' *');
+      }
       this.code.line(` * @link ${link}`);
     }
     this.code.line(' */');
-    return;
+
+    /**
+     * Add escapes to the doc text to avoid text that breaks the parsing of the string
+     *
+     * We currently escape the following sequences:
+     *
+     * - <asterisk><slash> (* /): if this occurs somewhere in the doc text, it
+     *   will end the block comment in the wrong place. Break up those
+     *   characters by inserting a space. Would have loved to use a zero-width space,
+     *   but I'm very very afraid it will break codegen in subtle ways, and just using
+     *   a space feels safer.
+     * - \u: if this occurs in Java code, the Java compiler will try and parse the
+     *   following 4 characters as a unicode code point, and fail if the 4 characters
+     *   aren't hex digits. This is formally a bug in pacmak (it should do the escaping
+     *   while rendering, https://github.com/aws/jsii/issues/3302), but to
+     *   expedite the build fixing it here as well. Replace with '\ u' (tried using
+     *   `\\u` but for some reason that also doesn't carry over to codegen).
+     */
+    function escapeDocText(x: string) {
+      x = x.replace(/\*\//g, '* /');
+      x = x.replace(/\\u/g, '\\ u');
+      return x;
+    }
   }
 }