Merge pull request #1192 from Microsoft/ae-misc-doc-fixes

[api-extractor] Fix a couple documentation issues found during testing

apps/api-documenter/src/cli/BaseAction.ts

@@ -2,13 +2,21 @@
 // See LICENSE in the project root for license information.
 
 import * as path from 'path';
+import * as tsdoc from '@microsoft/tsdoc';
+import * as colors from 'colors';
 
 import {
   CommandLineAction,
   CommandLineStringParameter
 } from '@microsoft/ts-command-line';
 import { FileSystem } from '@microsoft/node-core-library';
-import { ApiModel } from '@microsoft/api-extractor-model';
+import {
+  ApiModel,
+  ApiItem,
+  ApiItemContainerMixin,
+  ApiDocumentedItem,
+  IResolveDeclarationReferenceResult
+} from '@microsoft/api-extractor-model';
 
 export abstract class BaseAction extends CommandLineAction {
   protected inputFolder: string;
@@ -55,6 +63,66 @@ export abstract class BaseAction extends CommandLineAction {
       }
     }
 
+    this._applyInheritDoc(apiModel, apiModel);
+
     return apiModel;
   }
+
+  // TODO: This is a temporary workaround.  The long term plan is for API Extractor's DocCommentEnhancer
+  // to apply all @inheritDoc tags before the .api.json file is written.
+  // See DocCommentEnhancer._applyInheritDoc() for more info.
+  private _applyInheritDoc(apiItem: ApiItem, apiModel: ApiModel): void {
+
+    if (apiItem instanceof ApiDocumentedItem) {
+      if (apiItem.tsdocComment) {
+        const inheritDocTag: tsdoc.DocInheritDocTag | undefined = apiItem.tsdocComment.inheritDocTag;
+
+        if (inheritDocTag && inheritDocTag.declarationReference) {
+          // Attempt to resolve the declaration reference
+          const result: IResolveDeclarationReferenceResult
+            = apiModel.resolveDeclarationReference(inheritDocTag.declarationReference, apiItem);
+
+          if (result.errorMessage) {
+            console.log(colors.yellow(`Warning: Unresolved @inheritDoc tag for ${apiItem.displayName}: `
+              + result.errorMessage));
+          } else {
+            if (result.resolvedApiItem instanceof ApiDocumentedItem
+              && result.resolvedApiItem.tsdocComment
+              && result.resolvedApiItem !== apiItem) {
+              this._copyInheritedDocs(apiItem.tsdocComment, result.resolvedApiItem.tsdocComment);
+            }
+          }
+        }
+
+      }
+    }
+
+    // Recurse members
+    if (ApiItemContainerMixin.isBaseClassOf(apiItem)) {
+      for (const member of apiItem.members) {
+        this._applyInheritDoc(member, apiModel);
+      }
+    }
+  }
+
+  /**
+   * Copy the content from `sourceDocComment` to `targetDocComment`.
+   * This code is borrowed from DocCommentEnhancer as a temporary workaround.
+   */
+  private _copyInheritedDocs(targetDocComment: tsdoc.DocComment, sourceDocComment: tsdoc.DocComment): void {
+    targetDocComment.summarySection = sourceDocComment.summarySection;
+    targetDocComment.remarksBlock = sourceDocComment.remarksBlock;
+
+    targetDocComment.params.clear();
+    for (const param of sourceDocComment.params) {
+      targetDocComment.params.add(param);
+    }
+    for (const typeParam of sourceDocComment.typeParams) {
+      targetDocComment.typeParams.add(typeParam);
+    }
+    targetDocComment.returnsBlock = sourceDocComment.returnsBlock;
+
+    targetDocComment.inheritDocTag = undefined;
+  }
+
 }

apps/api-documenter/src/markdown/CustomMarkdownEmitter.ts

@@ -177,11 +177,12 @@ export class CustomMarkdownEmitter extends MarkdownEmitter {
           context.writer.write(encodedLinkText);
           context.writer.write(`](${filename!})`);
         } else {
-          console.log(colors.red('WARNING: Unable to determine link text'));
+          console.log(colors.yellow('WARNING: Unable to determine link text'));
         }
       }
     } else if (result.errorMessage) {
-      console.log(colors.red('WARNING: Unable to resolve reference: ' + result.errorMessage));
+      console.log(colors.yellow(`WARNING: Unable to resolve reference "${docLinkTag.codeDestination!.emitAsTsdoc()}": `
+        + result.errorMessage));
     }
   }
 

apps/api-extractor/src/collector/Collector.ts

@@ -526,7 +526,7 @@ export class Collector {
 
               this.messageRouter.addAnalyzerIssue(
                 ExtractorMessageId.MissingReleaseTag,
-                `"${entity.nameForEmit}" is exported by the package, but it is missing `
+                `"${entity.astEntity.localName}" is exported by the package, but it is missing `
                 + `a release tag (@alpha, @beta, @public, or @internal)`,
                 astSymbol
               );

apps/api-extractor/src/enhancers/DocCommentEnhancer.ts

@@ -8,10 +8,11 @@ import { Collector } from '../collector/Collector';
 import { AstSymbol } from '../analyzer/AstSymbol';
 import { AstDeclaration } from '../analyzer/AstDeclaration';
 import { DeclarationMetadata } from '../collector/DeclarationMetadata';
-import { AedocDefinitions } from '@microsoft/api-extractor-model';
+import { AedocDefinitions, ReleaseTag } from '@microsoft/api-extractor-model';
 import { ExtractorMessageId } from '../api/ExtractorMessageId';
 import { VisitorState } from '../collector/VisitorState';
 import { ResolverFailure } from '../analyzer/AstReferenceResolver';
+import { SymbolMetadata } from '../collector/SymbolMetadata';
 
 export class DocCommentEnhancer {
   private readonly _collector: Collector;
@@ -54,7 +55,7 @@ export class DocCommentEnhancer {
     metadata.docCommentEnhancerVisitorState = VisitorState.Visiting;
 
     if (metadata.tsdocComment && metadata.tsdocComment.inheritDocTag) {
-      this._analyzeInheritDoc(astDeclaration, metadata.tsdocComment, metadata.tsdocComment.inheritDocTag);
+      this._applyInheritDoc(astDeclaration, metadata.tsdocComment, metadata.tsdocComment.inheritDocTag);
     }
 
     this._analyzeNeedsDocumentation(astDeclaration, metadata);
@@ -72,6 +73,9 @@ export class DocCommentEnhancer {
       // will auto-generate one.
       metadata.needsDocumentation = false;
 
+      // The class that contains this constructor
+      const classDeclaration: AstDeclaration = astDeclaration.parent!;
+
       const configuration: tsdoc.TSDocConfiguration = AedocDefinitions.tsdocConfiguration;
 
       if (!metadata.tsdocComment) {
@@ -83,12 +87,49 @@ export class DocCommentEnhancer {
           new tsdoc.DocPlainText({ configuration, text: 'Constructs a new instance of the ' }),
           new tsdoc.DocCodeSpan({
             configuration,
-            code: astDeclaration.astSymbol.parentAstSymbol!.localName
+            code: classDeclaration.astSymbol.localName
           }),
           new tsdoc.DocPlainText({ configuration, text: ' class' })
         ]);
       }
 
+      const symbolMetadata: SymbolMetadata = this._collector.fetchMetadata(astDeclaration.astSymbol);
+
+      if (symbolMetadata.releaseTag === ReleaseTag.Internal) {
+        // If the constructor is marked as internal, then add a boilerplate notice for the containing class
+        const classMetadata: DeclarationMetadata = this._collector.fetchMetadata(classDeclaration);
+
+        if (!classMetadata.tsdocComment) {
+          classMetadata.tsdocComment = new tsdoc.DocComment({ configuration });
+        }
+
+        if (classMetadata.tsdocComment.remarksBlock === undefined) {
+          classMetadata.tsdocComment.remarksBlock = new tsdoc.DocBlock({
+            configuration,
+            blockTag: new tsdoc.DocBlockTag({
+              configuration,
+              tagName: tsdoc.StandardTags.remarks.tagName
+            })
+          });
+        }
+
+        classMetadata.tsdocComment.remarksBlock.content.appendNode(
+          new tsdoc.DocParagraph({ configuration }, [
+            new tsdoc.DocPlainText({
+              configuration,
+              text: `The constructor for this class is marked as internal. Third-party code should not`
+                + ` call the constructor directly or create subclasses that extend the `
+            }),
+            new tsdoc.DocCodeSpan({
+              configuration,
+              code: classDeclaration.astSymbol.localName
+            }),
+            new tsdoc.DocPlainText({ configuration, text: ' class.' })
+          ])
+        );
+
+      }
+
     } else if (metadata.tsdocComment) {
       // Require the summary to contain at least 10 non-spacing characters
       metadata.needsDocumentation = !tsdoc.PlainTextEmitter.hasAnyTextContent(
@@ -109,9 +150,9 @@ export class DocCommentEnhancer {
     if (node instanceof tsdoc.DocLinkTag) {
       if (node.codeDestination) {
 
-        // Is it referring to the working package?  If so, we don't do any link validation, because
+        // Is it referring to the working package?  If not, we don't do any link validation, because
         // AstReferenceResolver doesn't support it yet (but ModelReferenceResolver does of course).
-        // TODO: We need to come back and fix this.
+        // Tracked by:  https://github.com/Microsoft/web-build-tools/issues/1195
         if (node.codeDestination.packageName === undefined
           || node.codeDestination.packageName === this._collector.workingPackage.name) {
 
@@ -135,7 +176,7 @@ export class DocCommentEnhancer {
   /**
    * Follow an `{@inheritDoc ___}` reference and copy the content that we find in the referenced comment.
    */
-  private _analyzeInheritDoc(astDeclaration: AstDeclaration, docComment: tsdoc.DocComment,
+  private _applyInheritDoc(astDeclaration: AstDeclaration, docComment: tsdoc.DocComment,
     inheritDocTag: tsdoc.DocInheritDocTag): void {
 
     if (!inheritDocTag.declarationReference) {
@@ -145,6 +186,16 @@ export class DocCommentEnhancer {
       return;
     }
 
+    // Is it referring to the working package?
+    if (!(inheritDocTag.declarationReference.packageName === undefined
+      || inheritDocTag.declarationReference.packageName === this._collector.workingPackage.name)) {
+
+      // It's referencing an external package, so skip this inheritDoc tag, since AstReferenceResolver doesn't
+      // support it yet.  As a workaround, this tag will get handled later by api-documenter.
+      // Tracked by:  https://github.com/Microsoft/web-build-tools/issues/1195
+      return;
+    }
+
     const referencedAstDeclaration: AstDeclaration | ResolverFailure = this._collector.astReferenceResolver
       .resolve(inheritDocTag.declarationReference);
 

build-tests/api-documenter-test/etc/api-documenter-test.api.json

@@ -40,7 +40,7 @@
         {
           "kind": "Class",
           "canonicalReference": "(DocClass1:class)",
-          "docComment": "/**\n * This is an example class.\n *\n * @remarks\n *\n * These are some remarks.\n *\n * @defaultValue\n *\n * a default value for this function\n *\n * @public\n */\n",
+          "docComment": "/**\n * This is an example class.\n *\n * @remarks\n *\n * These are some remarks.\n *\n * The constructor for this class is marked as internal. Third-party code should not call the constructor directly or create subclasses that extend the `DocClass1` class.\n *\n * @defaultValue\n *\n * a default value for this function\n *\n * @public\n */\n",
           "excerptTokens": [
             {
               "kind": "Content",
@@ -86,45 +86,6 @@
           "releaseTag": "Public",
           "name": "DocClass1",
           "members": [
-            {
-              "kind": "Constructor",
-              "canonicalReference": "(:constructor,instance,0)",
-              "docComment": "/**\n * The class constructor\n */\n",
-              "excerptTokens": [
-                {
-                  "kind": "Content",
-                  "text": "constructor("
-                },
-                {
-                  "kind": "Reference",
-                  "text": "name"
-                },
-                {
-                  "kind": "Content",
-                  "text": ": "
-                },
-                {
-                  "kind": "Content",
-                  "text": "string"
-                },
-                {
-                  "kind": "Content",
-                  "text": ");"
-                }
-              ],
-              "isStatic": false,
-              "releaseTag": "Public",
-              "overloadIndex": 0,
-              "parameters": [
-                {
-                  "parameterName": "name",
-                  "parameterTypeTokenRange": {
-                    "startIndex": 3,
-                    "endIndex": 4
-                  }
-                }
-              ]
-            },
             {
               "kind": "Method",
               "canonicalReference": "(deprecatedExample:instance,0)",

build-tests/api-documenter-test/etc/api-documenter-test.api.md

@@ -13,6 +13,7 @@ export class DocBaseClass {
 
 // @public
 export class DocClass1 extends DocBaseClass implements IDocInterface1, IDocInterface2 {
+    // @internal
     constructor(name: string);
     // @deprecated (undocumented)
     deprecatedExample(): void;

build-tests/api-documenter-test/etc/markdown/api-documenter-test.docclass1.md

@@ -38,3 +38,5 @@ export declare class DocClass1 extends DocBaseClass implements IDocInterface1, I
 
 These are some remarks.
 
+The constructor for this class is marked as internal. Third-party code should not call the constructor directly or create subclasses that extend the `DocClass1` class.
+

build-tests/api-documenter-test/etc/yaml/api-documenter-test/docclass1.yml

@@ -2,7 +2,12 @@
 items:
   - uid: api-documenter-test.DocClass1
     summary: This is an example class.
-    remarks: These are some remarks.
+    remarks: >-
+      These are some remarks.
+
+
+      The constructor for this class is marked as internal. Third-party code should not call the constructor directly or
+      create subclasses that extend the `DocClass1` class.
     name: DocClass1
     fullName: DocClass1
     langs:
@@ -15,7 +20,6 @@ items:
       - api-documenter-test.IDocInterface2
     package: api-documenter-test
     children:
-      - api-documenter-test.DocClass1.(constructor)
       - api-documenter-test.DocClass1.deprecatedExample
       - api-documenter-test.DocClass1.exampleFunction
       - api-documenter-test.DocClass1.exampleFunction_1
@@ -25,20 +29,6 @@ items:
       - api-documenter-test.DocClass1.regularProperty
       - api-documenter-test.DocClass1.sumWithExample
       - api-documenter-test.DocClass1.tableExample
-  - uid: api-documenter-test.DocClass1.(constructor)
-    summary: The class constructor
-    name: (constructor)(name)
-    fullName: (constructor)(name)
-    langs:
-      - typeScript
-    type: constructor
-    syntax:
-      content: 'constructor(name: string);'
-      parameters:
-        - id: name
-          description: ''
-          type:
-            - string
   - uid: api-documenter-test.DocClass1.deprecatedExample
     deprecated:
       content: Use `otherThing()` instead.

build-tests/api-documenter-test/src/DocClass1.ts

@@ -98,7 +98,8 @@ export interface IDocInterface4 {
  */
 export class DocClass1 extends DocBaseClass implements IDocInterface1, IDocInterface2 {
   /**
-   * The class constructor
+   * An internal class constructor.
+   * @internal
    */
   public constructor(name: string) {
     super();

build-tests/api-extractor-scenarios/etc/test-outputs/docReferences/api-extractor-scenarios.api.json

@@ -187,6 +187,41 @@
             }
           ]
         },
+        {
+          "kind": "Function",
+          "canonicalReference": "(succeedForNow:0)",
+          "docComment": "/**\n * @privateRemarks\n *\n * succeedForNow() should fail due to a broken link, but it's ignored until we fix this issue: https://github.com/Microsoft/web-build-tools/issues/1195\n * {@inheritDoc nonexistent-package#MyNamespace.MyClass.nonExistentMethod}\n *\n * @public\n */\n",
+          "excerptTokens": [
+            {
+              "kind": "Content",
+              "text": "export declare function "
+            },
+            {
+              "kind": "Reference",
+              "text": "succeedForNow"
+            },
+            {
+              "kind": "Content",
+              "text": "(): "
+            },
+            {
+              "kind": "Content",
+              "text": "void"
+            },
+            {
+              "kind": "Content",
+              "text": ";"
+            }
+          ],
+          "returnTypeTokenRange": {
+            "startIndex": 3,
+            "endIndex": 4
+          },
+          "releaseTag": "Public",
+          "overloadIndex": 0,
+          "parameters": [],
+          "name": "succeedForNow"
+        },
         {
           "kind": "Function",
           "canonicalReference": "(testSimple:0)",