Implement the missing ae-forgotten-export analysis for AstNamespaceIm…

…port references

apps/api-extractor/.vscode/launch.json

@@ -74,7 +74,13 @@
       "name": "scenario",
       "program": "${workspaceFolder}/lib/start.js",
       "cwd": "${workspaceFolder}/../../build-tests/api-extractor-scenarios",
-      "args": ["--debug", "run", "--local", "--config", "./temp/configs/api-extractor-typeof.json"],
+      "args": [
+        "--debug",
+        "run",
+        "--local",
+        "--config",
+        "./temp/configs/api-extractor-exportImportStarAs2.json"
+      ],
       "sourceMaps": true
     }
   ]

apps/api-extractor/src/analyzer/AstNamespaceImport.ts

@@ -3,8 +3,9 @@
 
 import * as ts from 'typescript';
 
-import { AstModule } from './AstModule';
+import { AstModule, AstModuleExportInfo } from './AstModule';
 import { AstSyntheticEntity } from './AstEntity';
+import { Collector } from '../collector/Collector';
 
 export interface IAstNamespaceImportOptions {
   readonly astModule: AstModule;
@@ -78,4 +79,11 @@ export class AstNamespaceImport extends AstSyntheticEntity {
     // abstract
     return this.namespaceName;
   }
+
+  public fetchAstModuleExportInfo(collector: Collector): AstModuleExportInfo {
+    const astModuleExportInfo: AstModuleExportInfo = collector.astSymbolTable.fetchAstModuleExportInfo(
+      this.astModule
+    );
+    return astModuleExportInfo;
+  }
 }

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

@@ -234,6 +234,7 @@ export class Collector {
 
     const astModuleExportInfo: AstModuleExportInfo =
       this.astSymbolTable.fetchAstModuleExportInfo(astEntryPoint);
+
     for (const [exportName, astEntity] of astModuleExportInfo.exportedLocalEntities) {
       this._createCollectorEntity(astEntity, exportName);
 
@@ -440,18 +441,14 @@ export class Collector {
     }
 
     if (astEntity instanceof AstNamespaceImport) {
-      const astModuleExportInfo: AstModuleExportInfo = this.astSymbolTable.fetchAstModuleExportInfo(
-        astEntity.astModule
-      );
+      const astModuleExportInfo: AstModuleExportInfo = astEntity.fetchAstModuleExportInfo(this);
 
       for (const exportedEntity of astModuleExportInfo.exportedLocalEntities.values()) {
         // Create a CollectorEntity for each top-level export of AstImportInternal entity
         const entity: CollectorEntity = this._createCollectorEntity(exportedEntity, undefined);
         entity.addAstNamespaceImports(astEntity);
 
         this._createEntityForIndirectReferences(exportedEntity, alreadySeenAstEntities);
-
-        // TODO - create entity for module export
       }
     }
   }

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

@@ -13,26 +13,52 @@ import { CollectorEntity } from '../collector/CollectorEntity';
 import { ExtractorMessageId } from '../api/ExtractorMessageId';
 import { ReleaseTag } from '@microsoft/api-extractor-model';
 import { AstNamespaceImport } from '../analyzer/AstNamespaceImport';
+import { AstModuleExportInfo } from '../analyzer/AstModule';
+import { AstEntity } from '../analyzer/AstEntity';
 
 export class ValidationEnhancer {
   public static analyze(collector: Collector): void {
-    const alreadyWarnedSymbols: Set<AstSymbol> = new Set<AstSymbol>();
+    const alreadyWarnedEntities: Set<AstEntity> = new Set<AstEntity>();
 
     for (const entity of collector.entities) {
-      if (entity.astEntity instanceof AstSymbol) {
-        if (entity.consumable) {
-          entity.astEntity.forEachDeclarationRecursive((astDeclaration: AstDeclaration) => {
-            ValidationEnhancer._checkReferences(collector, astDeclaration, alreadyWarnedSymbols);
-          });
-
-          const symbolMetadata: SymbolMetadata = collector.fetchSymbolMetadata(entity.astEntity);
-          ValidationEnhancer._checkForInternalUnderscore(collector, entity, entity.astEntity, symbolMetadata);
-          ValidationEnhancer._checkForInconsistentReleaseTags(collector, entity.astEntity, symbolMetadata);
-        }
+      if (!entity.consumable) {
+        continue;
       }
 
-      if (entity.astEntity instanceof AstNamespaceImport) {
-        // TODO [MA]: validation for local module import
+      if (entity.astEntity instanceof AstSymbol) {
+        // A regular exported AstSymbol
+
+        const astSymbol: AstSymbol = entity.astEntity;
+
+        astSymbol.forEachDeclarationRecursive((astDeclaration: AstDeclaration) => {
+          ValidationEnhancer._checkReferences(collector, astDeclaration, alreadyWarnedEntities);
+        });
+
+        const symbolMetadata: SymbolMetadata = collector.fetchSymbolMetadata(astSymbol);
+        ValidationEnhancer._checkForInternalUnderscore(collector, entity, astSymbol, symbolMetadata);
+        ValidationEnhancer._checkForInconsistentReleaseTags(collector, astSymbol, symbolMetadata);
+      } else if (entity.astEntity instanceof AstNamespaceImport) {
+        // A namespace created using "import * as ___ from ___"
+        const astNamespaceImport: AstNamespaceImport = entity.astEntity;
+
+        const astModuleExportInfo: AstModuleExportInfo =
+          astNamespaceImport.fetchAstModuleExportInfo(collector);
+
+        for (const namespaceMemberAstEntity of astModuleExportInfo.exportedLocalEntities.values()) {
+          if (namespaceMemberAstEntity instanceof AstSymbol) {
+            const astSymbol: AstSymbol = namespaceMemberAstEntity;
+
+            astSymbol.forEachDeclarationRecursive((astDeclaration: AstDeclaration) => {
+              ValidationEnhancer._checkReferences(collector, astDeclaration, alreadyWarnedEntities);
+            });
+
+            const symbolMetadata: SymbolMetadata = collector.fetchSymbolMetadata(astSymbol);
+
+            // (Don't apply ValidationEnhancer._checkForInternalUnderscore() for AstNamespaceImport members)
+
+            ValidationEnhancer._checkForInconsistentReleaseTags(collector, astSymbol, symbolMetadata);
+          }
+        }
       }
     }
   }
@@ -163,63 +189,78 @@ export class ValidationEnhancer {
   private static _checkReferences(
     collector: Collector,
     astDeclaration: AstDeclaration,
-    alreadyWarnedSymbols: Set<AstSymbol>
+    alreadyWarnedEntities: Set<AstEntity>
   ): void {
     const apiItemMetadata: ApiItemMetadata = collector.fetchApiItemMetadata(astDeclaration);
     const declarationReleaseTag: ReleaseTag = apiItemMetadata.effectiveReleaseTag;
 
     for (const referencedEntity of astDeclaration.referencedAstEntities) {
+      let collectorEntity: CollectorEntity | undefined;
+      let referencedReleaseTag: ReleaseTag;
+      let localName: string;
+
       if (referencedEntity instanceof AstSymbol) {
         // If this is e.g. a member of a namespace, then we need to be checking the top-level scope to see
         // whether it's exported.
         //
         // TODO: Technically we should also check each of the nested scopes along the way.
         const rootSymbol: AstSymbol = referencedEntity.rootAstSymbol;
 
-        if (!rootSymbol.isExternal) {
-          // TODO: consider exported by local module import
-          const collectorEntity: CollectorEntity | undefined = collector.tryGetCollectorEntity(rootSymbol);
-
-          if (collectorEntity && collectorEntity.consumable) {
-            const referencedMetadata: SymbolMetadata = collector.fetchSymbolMetadata(referencedEntity);
-            const referencedReleaseTag: ReleaseTag = referencedMetadata.maxEffectiveReleaseTag;
-
-            if (ReleaseTag.compare(declarationReleaseTag, referencedReleaseTag) > 0) {
-              collector.messageRouter.addAnalyzerIssue(
-                ExtractorMessageId.IncompatibleReleaseTags,
-                `The symbol "${astDeclaration.astSymbol.localName}"` +
-                  ` is marked as ${ReleaseTag.getTagName(declarationReleaseTag)},` +
-                  ` but its signature references "${referencedEntity.localName}"` +
-                  ` which is marked as ${ReleaseTag.getTagName(referencedReleaseTag)}`,
-                astDeclaration
-              );
-            }
+        if (rootSymbol.isExternal) {
+          continue;
+        }
+
+        localName = rootSymbol.localName;
+
+        collectorEntity = collector.tryGetCollectorEntity(rootSymbol);
+
+        const referencedMetadata: SymbolMetadata = collector.fetchSymbolMetadata(referencedEntity);
+        referencedReleaseTag = referencedMetadata.maxEffectiveReleaseTag;
+      } else if (referencedEntity instanceof AstNamespaceImport) {
+        collectorEntity = collector.tryGetCollectorEntity(referencedEntity);
+
+        // TODO: Currently the "import * as ___ from ___" syntax does not yet support doc comments
+        referencedReleaseTag = ReleaseTag.Public;
+
+        localName = referencedEntity.localName;
+      } else {
+        continue;
+      }
+
+      if (collectorEntity && collectorEntity.consumable) {
+        if (ReleaseTag.compare(declarationReleaseTag, referencedReleaseTag) > 0) {
+          collector.messageRouter.addAnalyzerIssue(
+            ExtractorMessageId.IncompatibleReleaseTags,
+            `The symbol "${astDeclaration.astSymbol.localName}"` +
+              ` is marked as ${ReleaseTag.getTagName(declarationReleaseTag)},` +
+              ` but its signature references "${referencedEntity.localName}"` +
+              ` which is marked as ${ReleaseTag.getTagName(referencedReleaseTag)}`,
+            astDeclaration
+          );
+        }
+      } else {
+        const entryPointFilename: string = path.basename(
+          collector.workingPackage.entryPointSourceFile.fileName
+        );
+
+        if (!alreadyWarnedEntities.has(referencedEntity)) {
+          alreadyWarnedEntities.add(referencedEntity);
+
+          if (
+            referencedEntity instanceof AstSymbol &&
+            ValidationEnhancer._isEcmaScriptSymbol(referencedEntity)
+          ) {
+            // The main usage scenario for ECMAScript symbols is to attach private data to a JavaScript object,
+            // so as a special case, we do NOT report them as forgotten exports.
           } else {
-            const entryPointFilename: string = path.basename(
-              collector.workingPackage.entryPointSourceFile.fileName
+            collector.messageRouter.addAnalyzerIssue(
+              ExtractorMessageId.ForgottenExport,
+              `The symbol "${localName}" needs to be exported by the entry point ${entryPointFilename}`,
+              astDeclaration
             );
-
-            if (!alreadyWarnedSymbols.has(referencedEntity)) {
-              alreadyWarnedSymbols.add(referencedEntity);
-
-              // The main usage scenario for ECMAScript symbols is to attach private data to a JavaScript object,
-              // so as a special case, we do NOT report them as forgotten exports.
-              if (!ValidationEnhancer._isEcmaScriptSymbol(referencedEntity)) {
-                collector.messageRouter.addAnalyzerIssue(
-                  ExtractorMessageId.ForgottenExport,
-                  `The symbol "${rootSymbol.localName}" needs to be exported` +
-                    ` by the entry point ${entryPointFilename}`,
-                  astDeclaration
-                );
-              }
-            }
           }
         }
       }
-
-      if (referencedEntity instanceof AstNamespaceImport) {
-        // TODO [MA]: add validation for local import
-      }
     }
   }
 

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

@@ -102,6 +102,20 @@ export class ApiModelGenerator {
     }
 
     if (astEntity instanceof AstNamespaceImport) {
+      // Note that a single API item can belong to two different AstNamespaceImport namespaces.  For example:
+      //
+      //   // file.ts defines "thing()"
+      //   import * as example1 from "./file";
+      //   import * as example2 from "./file";
+      //
+      //   // ...so here we end up with example1.thing() and example2.thing()
+      //   export { example1, example2 }
+      //
+      // The current logic does not try to associate "thing()" with a specific parent.  Instead
+      // the API documentation will show duplicated entries for example1.thing() and example2.thing()./
+      //
+      // This could be improved in the future, but it requires a stable mechanism for choosing an associated parent.
+      // For thoughts about this:  https://github.com/microsoft/rushstack/issues/1308
       this._processAstModule(astEntity.astModule, exportedName, parentApiItem);
       return;
     }

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

@@ -146,9 +146,7 @@ export class ApiReportGenerator {
         }
 
         if (astEntity instanceof AstNamespaceImport) {
-          const astModuleExportInfo: AstModuleExportInfo = collector.astSymbolTable.fetchAstModuleExportInfo(
-            astEntity.astModule
-          );
+          const astModuleExportInfo: AstModuleExportInfo = astEntity.fetchAstModuleExportInfo(collector);
 
           if (entity.nameForEmit === undefined) {
             // This should never happen

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

@@ -155,9 +155,7 @@ export class DtsRollupGenerator {
       }
 
       if (astEntity instanceof AstNamespaceImport) {
-        const astModuleExportInfo: AstModuleExportInfo = collector.astSymbolTable.fetchAstModuleExportInfo(
-          astEntity.astModule
-        );
+        const astModuleExportInfo: AstModuleExportInfo = astEntity.fetchAstModuleExportInfo(collector);
 
         if (entity.nameForEmit === undefined) {
           // This should never happen

build-tests/api-extractor-scenarios/etc/test-outputs/exportImportStarAs2/api-extractor-scenarios.api.md

@@ -4,6 +4,8 @@
 
 ```ts
 
+// Warning: (ae-forgotten-export) The symbol "forgottenNs" needs to be exported by the entry point index.d.ts
+//
 // @public (undocumented)
 function exportedApi(): forgottenNs.ForgottenClass;