diff --git a/apps/api-extractor/.vscode/launch.json b/apps/api-extractor/.vscode/launch.json index f81af6c2ef3..2a4aa769b6b 100644 --- a/apps/api-extractor/.vscode/launch.json +++ b/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 } ] diff --git a/apps/api-extractor/src/analyzer/AstNamespaceImport.ts b/apps/api-extractor/src/analyzer/AstNamespaceImport.ts index 1ba6985beef..955ef903d27 100644 --- a/apps/api-extractor/src/analyzer/AstNamespaceImport.ts +++ b/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; + } } diff --git a/apps/api-extractor/src/collector/Collector.ts b/apps/api-extractor/src/collector/Collector.ts index 625bb5778ab..c963a448fe4 100644 --- a/apps/api-extractor/src/collector/Collector.ts +++ b/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,9 +441,7 @@ 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 @@ -450,8 +449,6 @@ export class Collector { entity.addAstNamespaceImports(astEntity); this._createEntityForIndirectReferences(exportedEntity, alreadySeenAstEntities); - - // TODO - create entity for module export } } } diff --git a/apps/api-extractor/src/enhancers/ValidationEnhancer.ts b/apps/api-extractor/src/enhancers/ValidationEnhancer.ts index 31969391b54..302480ee86d 100644 --- a/apps/api-extractor/src/enhancers/ValidationEnhancer.ts +++ b/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 = new Set(); + const alreadyWarnedEntities: Set = new Set(); 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,12 +189,16 @@ export class ValidationEnhancer { private static _checkReferences( collector: Collector, astDeclaration: AstDeclaration, - alreadyWarnedSymbols: Set + alreadyWarnedEntities: Set ): 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. @@ -176,50 +206,61 @@ export class ValidationEnhancer { // 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 - } } } diff --git a/apps/api-extractor/src/generators/ApiModelGenerator.ts b/apps/api-extractor/src/generators/ApiModelGenerator.ts index 5b6557b6149..648a762a0b6 100644 --- a/apps/api-extractor/src/generators/ApiModelGenerator.ts +++ b/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; } diff --git a/apps/api-extractor/src/generators/ApiReportGenerator.ts b/apps/api-extractor/src/generators/ApiReportGenerator.ts index c8eb23b54ca..e9a40e283e0 100644 --- a/apps/api-extractor/src/generators/ApiReportGenerator.ts +++ b/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 diff --git a/apps/api-extractor/src/generators/DtsRollupGenerator.ts b/apps/api-extractor/src/generators/DtsRollupGenerator.ts index f088fdee383..452dcd3b37e 100644 --- a/apps/api-extractor/src/generators/DtsRollupGenerator.ts +++ b/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 diff --git a/build-tests/api-extractor-scenarios/etc/test-outputs/exportImportStarAs2/api-extractor-scenarios.api.md b/build-tests/api-extractor-scenarios/etc/test-outputs/exportImportStarAs2/api-extractor-scenarios.api.md index 67e52795bee..cdb87e341e9 100644 --- a/build-tests/api-extractor-scenarios/etc/test-outputs/exportImportStarAs2/api-extractor-scenarios.api.md +++ b/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;