Responded to PR comments and prepared PR for review

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

@@ -37,9 +37,9 @@ export class CollectorEntity {
   }
 
   /**
-   * The declaration name that will be emitted in a .d.ts rollup.  For non-exported declarations,
-   * Collector._makeUniqueNames() may need to rename the declaration to avoid conflicts with other declarations
-   * in that module.
+   * The declaration name that will be emitted in the .d.ts rollup, .api.md, and .api.json files. Generated by
+   * `Collector._makeUniqueNames`. Be aware that the declaration may be renamed to avoid conflicts with (1)
+   * global names (e.g. `Promise`) and (2) if local, other local names across different files.
    */
   public get nameForEmit(): string | undefined {
     return this._nameForEmit;
@@ -97,7 +97,7 @@ export class CollectorEntity {
 
   /**
    * Indicates that this entity is exported from its parent module (i.e. either the package entry point or
-   * a local namespace).
+   * a local namespace). Compare to `CollectorEntity.consumable`.
    *
    * @remarks
    * In the example below:
@@ -115,23 +115,23 @@ export class CollectorEntity {
    * but not consumable.
    */
   public get exported(): boolean {
-    const exportedFromTopLevel: boolean = this.exportNames.size > 0;
+    // Exported from top-level?
+    if (this.exportNames.size > 0) return true;
 
-    let exportedFromParent: boolean = false;
+    // Exported from parent?
     for (const localExportNames of this._localExportNamesByParent.values()) {
       if (localExportNames.size > 0) {
-        exportedFromParent = true;
-        break;
+        return true;
       }
     }
 
-    return exportedFromTopLevel || exportedFromParent;
+    return false;
   }
 
   /**
-   * Indicates that it is possible for a consumer of the API to access this entity, either by importing
-   * it directly, or via some other alias such as a member of a namespace. If an entity is not consumable,
-   * then API Extractor will report an `ae-forgotten-export` warning.
+   * Indicates that it is possible for a consumer of the API to "consume" this entity, either by importing
+   * it directly or via a namespace. If an entity is not consumable, then API Extractor will report an
+   * `ae-forgotten-export` warning. Compare to `CollectorEntity.exported`.
    *
    * @remarks
    * An API item is consumable if:
@@ -155,21 +155,35 @@ export class CollectorEntity {
    * In this example, `add` is exported via the consumable `calculator` namespace.
    */
   public get consumable(): boolean {
-    const exportedFromTopLevel: boolean = this.exportNames.size > 0;
+    // Exported from top-level?
+    if (this.exportNames.size > 0) return true;
 
-    let exportedFromExportedParent: boolean = false;
+    // Exported from consumable parent?
     for (const [parent, localExportNames] of this._localExportNamesByParent) {
       if (localExportNames.size > 0 && parent.consumable) {
-        exportedFromExportedParent = true;
-        break;
+        return true;
       }
     }
 
-    return exportedFromTopLevel || exportedFromExportedParent;
+    return false;
   }
 
   /**
    * Whether the entity has any parent entities.
+   *
+   * @remarks
+   * In the example below:
+   *
+   * ```ts
+   * declare function add(): void;
+   * declare namespace calculator {
+   *  export {
+   *    add
+   *  }
+   * }
+   * ```
+   *
+   * The `CollectorEntity` for `calculator` is the parent of the `CollectorEntity` for `add`.
    */
   public get hasParents(): boolean {
     return this._localExportNamesByParent.size > 0;
@@ -193,6 +207,20 @@ export class CollectorEntity {
 
   /**
    * Adds a new local export name to the entity.
+   *
+   * @remarks
+   * In the example below:
+   *
+   * ```ts
+   * declare function add(): void;
+   * declare namespace calculator {
+   *  export {
+   *    add
+   *  }
+   * }
+   * ```
+   *
+   * `add` is the local export name for the `CollectorEntity` for `add`.
    */
   public addLocalExportName(localExportName: string, parent: CollectorEntity): void {
     const localExportNames: Set<string> = this._localExportNamesByParent.get(parent) || new Set();

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

@@ -28,7 +28,11 @@ export class DocCommentEnhancer {
   public analyze(): void {
     for (const entity of this._collector.entities) {
       if (entity.astEntity instanceof AstSymbol) {
-        if (entity.consumable || this._collector.extractorConfig.apiReportIncludeForgottenExports) {
+        if (
+          entity.consumable ||
+          this._collector.extractorConfig.apiReportIncludeForgottenExports ||
+          this._collector.extractorConfig.docModelIncludeForgottenExports
+        ) {
           entity.astEntity.forEachDeclarationRecursive((astDeclaration: AstDeclaration) => {
             this._analyzeApiItem(astDeclaration);
           });

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

@@ -21,7 +21,13 @@ export class ValidationEnhancer {
     const alreadyWarnedEntities: Set<AstEntity> = new Set<AstEntity>();
 
     for (const entity of collector.entities) {
-      if (!(entity.consumable || collector.extractorConfig.apiReportIncludeForgottenExports)) {
+      if (
+        !(
+          entity.consumable ||
+          collector.extractorConfig.apiReportIncludeForgottenExports ||
+          collector.extractorConfig.docModelIncludeForgottenExports
+        )
+      ) {
         continue;
       }
 

apps/api-extractor/src/generators/ApiModelGenerator.ts

@@ -1144,7 +1144,9 @@ export class ApiModelGenerator {
 
     if (!entity) {
       // This should never happen.
-      throw new InternalError('Failed to get collector entity for root symbol of declaration');
+      throw new InternalError(
+        `Failed to get collector entity for root symbol of declaration ${astDeclaration.astSymbol.localName}`
+      );
     }
 
     return entity.exported;

apps/api-extractor/src/generators/ApiReportGenerator.ts

@@ -172,20 +172,21 @@ export class ApiReportGenerator {
           writer.increaseIndent();
 
           const exportClauses: string[] = [];
-          for (const [exportName, astEntity] of astModuleExportInfo.exportedLocalEntities) {
-            const collectorEntity: CollectorEntity | undefined = collector.tryGetCollectorEntity(astEntity);
+          for (const [exportedName, exportedEntity] of astModuleExportInfo.exportedLocalEntities) {
+            const collectorEntity: CollectorEntity | undefined =
+              collector.tryGetCollectorEntity(exportedEntity);
             if (collectorEntity === undefined) {
               // This should never happen
               // top-level exports of local imported module should be added as collector entities before
               throw new InternalError(
-                `Cannot find collector entity for ${entity.nameForEmit}.${astEntity.localName}`
+                `Cannot find collector entity for ${entity.nameForEmit}.${exportedEntity.localName}`
               );
             }
 
-            if (collectorEntity.nameForEmit === exportName) {
+            if (collectorEntity.nameForEmit === exportedName) {
               exportClauses.push(collectorEntity.nameForEmit);
             } else {
-              exportClauses.push(`${collectorEntity.nameForEmit} as ${exportName}`);
+              exportClauses.push(`${collectorEntity.nameForEmit} as ${exportedName}`);
             }
           }
           writer.writeLine(exportClauses.join(',\n'));

build-tests/api-extractor-scenarios/etc/includeForgottenExports/api-extractor-scenarios.api.json

@@ -172,6 +172,38 @@
       "name": "",
       "preserveMemberOrder": false,
       "members": [
+        {
+          "kind": "Class",
+          "canonicalReference": "api-extractor-scenarios!~AnotherDuplicateName_2:class",
+          "docComment": "",
+          "excerptTokens": [
+            {
+              "kind": "Content",
+              "text": "declare class AnotherDuplicateName "
+            }
+          ],
+          "releaseTag": "None",
+          "name": "AnotherDuplicateName_2",
+          "preserveMemberOrder": false,
+          "members": [],
+          "implementsTokenRanges": []
+        },
+        {
+          "kind": "Class",
+          "canonicalReference": "api-extractor-scenarios!~AnotherDuplicateName:class",
+          "docComment": "/**\n * This forgotten item has the same name as another forgotten item in another file. They should be given unique names.\n */\n",
+          "excerptTokens": [
+            {
+              "kind": "Content",
+              "text": "declare class AnotherDuplicateName "
+            }
+          ],
+          "releaseTag": "None",
+          "name": "AnotherDuplicateName",
+          "preserveMemberOrder": false,
+          "members": [],
+          "implementsTokenRanges": []
+        },
         {
           "kind": "TypeAlias",
           "canonicalReference": "api-extractor-scenarios!~DuplicateName_2:type",
@@ -200,7 +232,7 @@
         {
           "kind": "TypeAlias",
           "canonicalReference": "api-extractor-scenarios!DuplicateName:type",
-          "docComment": "/**\n * This type is exported but has the same name as an unexported type in './internal.ts'. This unexported type is also included in the API report and doc model files. The unexported type will be renamed to avoid a name conflict.\n *\n * @public\n */\n",
+          "docComment": "/**\n * This type is exported but has the same name as a forgotten type in './internal.ts'. This forgotten type is also included in the API report and doc model files. The forgotten type will be renamed to avoid a name conflict.\n *\n * @public\n */\n",
           "excerptTokens": [
             {
               "kind": "Content",
@@ -225,7 +257,7 @@
         {
           "kind": "Class",
           "canonicalReference": "api-extractor-scenarios!~ForgottenExport1:class",
-          "docComment": "/**\n * This doc comment should be inherited by `ForgottenExport2`\n */\n",
+          "docComment": "/**\n * `ForgottenExport2` wants to inherit this doc comment, but unfortunately this isn't supported yet\n */\n",
           "excerptTokens": [
             {
               "kind": "Content",
@@ -484,6 +516,62 @@
           "parameters": [],
           "name": "someFunction5"
         },
+        {
+          "kind": "Function",
+          "canonicalReference": "api-extractor-scenarios!someFunction6:function(1)",
+          "docComment": "/**\n * @public\n */\n",
+          "excerptTokens": [
+            {
+              "kind": "Content",
+              "text": "export declare function someFunction6(): "
+            },
+            {
+              "kind": "Reference",
+              "text": "AnotherDuplicateName",
+              "canonicalReference": "api-extractor-scenarios!~AnotherDuplicateName:class"
+            },
+            {
+              "kind": "Content",
+              "text": ";"
+            }
+          ],
+          "returnTypeTokenRange": {
+            "startIndex": 1,
+            "endIndex": 2
+          },
+          "releaseTag": "Public",
+          "overloadIndex": 1,
+          "parameters": [],
+          "name": "someFunction6"
+        },
+        {
+          "kind": "Function",
+          "canonicalReference": "api-extractor-scenarios!someFunction7:function(1)",
+          "docComment": "/**\n * @public\n */\n",
+          "excerptTokens": [
+            {
+              "kind": "Content",
+              "text": "export declare function someFunction7(): "
+            },
+            {
+              "kind": "Reference",
+              "text": "AnotherDuplicateName",
+              "canonicalReference": "api-extractor-scenarios!~AnotherDuplicateName:class"
+            },
+            {
+              "kind": "Content",
+              "text": ";"
+            }
+          ],
+          "returnTypeTokenRange": {
+            "startIndex": 1,
+            "endIndex": 2
+          },
+          "releaseTag": "Public",
+          "overloadIndex": 1,
+          "parameters": [],
+          "name": "someFunction7"
+        },
         {
           "kind": "Namespace",
           "canonicalReference": "api-extractor-scenarios!SomeNamespace1:namespace",

build-tests/api-extractor-scenarios/etc/includeForgottenExports/api-extractor-scenarios.api.md

@@ -4,6 +4,13 @@
 
 ```ts
 
+class AnotherDuplicateName {
+}
+
+// (undocumented)
+class AnotherDuplicateName_2 {
+}
+
 // @public
 export type DuplicateName = boolean;
 
@@ -59,6 +66,16 @@ export function someFunction4(): ForgottenExport4.ForgottenExport5;
 // @public (undocumented)
 export function someFunction5(): internal2.ForgottenExport6;
 
+// Warning: (ae-forgotten-export) The symbol "AnotherDuplicateName" needs to be exported by the entry point index.d.ts
+//
+// @public (undocumented)
+export function someFunction6(): AnotherDuplicateName;
+
+// Warning: (ae-forgotten-export) The symbol "AnotherDuplicateName_2" needs to be exported by the entry point index.d.ts
+//
+// @public (undocumented)
+export function someFunction7(): AnotherDuplicateName_2;
+
 // @public (undocumented)
 export namespace SomeNamespace1 {
     // (undocumented)

build-tests/api-extractor-scenarios/etc/includeForgottenExports/rollup.d.ts

@@ -1,6 +1,16 @@
 /**
- * This type is exported but has the same name as an unexported type in './internal.ts'. This
- * unexported type is also included in the API report and doc model files. The unexported type
+ * This forgotten item has the same name as another forgotten item in another
+ * file. They should be given unique names.
+ */
+declare class AnotherDuplicateName {
+}
+
+declare class AnotherDuplicateName_2 {
+}
+
+/**
+ * This type is exported but has the same name as a forgotten type in './internal.ts'. This
+ * forgotten type is also included in the API report and doc model files. The forgotten type
  * will be renamed to avoid a name conflict.
  * @public
  */
@@ -12,7 +22,10 @@ export declare type DuplicateName = boolean;
  */
 declare type DuplicateName_2 = number;
 
-/** This doc comment should be inherited by `ForgottenExport2` */
+/**
+ * `ForgottenExport2` wants to inherit this doc comment, but unfortunately this isn't
+ * supported yet
+ */
 declare class ForgottenExport1 {
     prop?: ForgottenExport2;
     constructor();
@@ -47,6 +60,12 @@ export declare function someFunction4(): ForgottenExport4.ForgottenExport5;
 /** @public */
 export declare function someFunction5(): internal2.ForgottenExport6;
 
+/** @public */
+export declare function someFunction6(): AnotherDuplicateName;
+
+/** @public */
+export declare function someFunction7(): AnotherDuplicateName_2;
+
 /** @public */
 export declare namespace SomeNamespace1 {
     export class ForgottenExport3 {