From d61f74f81ca0b08f10b8433fad97fca468740b2b Mon Sep 17 00:00:00 2001 From: Zack Elliott Date: Thu, 11 Aug 2022 17:23:13 -0700 Subject: [PATCH 1/3] Improve DeclarationReferenceGenerator logic to handle more cases of unexported symbols --- .../src/generators/ApiModelGenerator.ts | 8 +- .../DeclarationReferenceGenerator.ts | 154 +++++++----------- .../api-extractor-scenarios.api.json | 2 +- .../api-extractor-scenarios.api.json | 2 +- .../api-extractor-scenarios.api.json | 2 +- .../api-extractor-scenarios.api.json | 2 +- .../api-extractor-scenarios.api.json | 6 +- .../api-extractor-scenarios.api.json | 29 ++++ .../api-extractor-scenarios.api.md | 6 + .../etc/referenceTokens/rollup.d.ts | 10 ++ .../src/referenceTokens/index.ts | 7 + .../src/referenceTokens/internal.ts | 1 + .../workspace/common/pnpm-lock.yaml | 16 +- .../api-extractor/refs_2022-08-12-00-18.json | 10 ++ 14 files changed, 139 insertions(+), 116 deletions(-) create mode 100644 build-tests/api-extractor-scenarios/src/referenceTokens/internal.ts create mode 100644 common/changes/@microsoft/api-extractor/refs_2022-08-12-00-18.json diff --git a/apps/api-extractor/src/generators/ApiModelGenerator.ts b/apps/api-extractor/src/generators/ApiModelGenerator.ts index 5fae347ee5b..8d7e9df895a 100644 --- a/apps/api-extractor/src/generators/ApiModelGenerator.ts +++ b/apps/api-extractor/src/generators/ApiModelGenerator.ts @@ -54,13 +54,7 @@ export class ApiModelGenerator { public constructor(collector: Collector) { this._collector = collector; this._apiModel = new ApiModel(); - this._referenceGenerator = new DeclarationReferenceGenerator( - collector.packageJsonLookup, - collector.workingPackage.name, - collector.program, - collector.typeChecker, - collector.bundledPackageNames - ); + this._referenceGenerator = new DeclarationReferenceGenerator(collector); } public get apiModel(): ApiModel { diff --git a/apps/api-extractor/src/generators/DeclarationReferenceGenerator.ts b/apps/api-extractor/src/generators/DeclarationReferenceGenerator.ts index 11fc3777df7..5cd34914f98 100644 --- a/apps/api-extractor/src/generators/DeclarationReferenceGenerator.ts +++ b/apps/api-extractor/src/generators/DeclarationReferenceGenerator.ts @@ -10,38 +10,26 @@ import { Navigation, Meaning } from '@microsoft/tsdoc/lib-commonjs/beta/DeclarationReference'; -import { PackageJsonLookup, INodePackageJson, InternalError } from '@rushstack/node-core-library'; +import { INodePackageJson, InternalError } from '@rushstack/node-core-library'; import { TypeScriptHelpers } from '../analyzer/TypeScriptHelpers'; import { TypeScriptInternals } from '../analyzer/TypeScriptInternals'; +import { Collector } from '../collector/Collector'; +import { CollectorEntity } from '../collector/CollectorEntity'; export class DeclarationReferenceGenerator { public static readonly unknownReference: string = '?'; - private _packageJsonLookup: PackageJsonLookup; - private _workingPackageName: string; - private _program: ts.Program; - private _typeChecker: ts.TypeChecker; - private _bundledPackageNames: ReadonlySet; - - public constructor( - packageJsonLookup: PackageJsonLookup, - workingPackageName: string, - program: ts.Program, - typeChecker: ts.TypeChecker, - bundledPackageNames: ReadonlySet - ) { - this._packageJsonLookup = packageJsonLookup; - this._workingPackageName = workingPackageName; - this._program = program; - this._typeChecker = typeChecker; - this._bundledPackageNames = bundledPackageNames; + private _collector: Collector; + + public constructor(collector: Collector) { + this._collector = collector; } /** * Gets the UID for a TypeScript Identifier that references a type. */ public getDeclarationReferenceForIdentifier(node: ts.Identifier): DeclarationReference | undefined { - const symbol: ts.Symbol | undefined = this._typeChecker.getSymbolAtLocation(node); + const symbol: ts.Symbol | undefined = this._collector.typeChecker.getSymbolAtLocation(node); if (symbol !== undefined) { const isExpression: boolean = DeclarationReferenceGenerator._isInExpressionContext(node); return ( @@ -99,68 +87,48 @@ export class DeclarationReferenceGenerator { ); } - private static _getNavigationToSymbol(symbol: ts.Symbol): Navigation | 'global' { + private _getNavigationToSymbol(symbol: ts.Symbol): Navigation { + const declaration: ts.Declaration | undefined = TypeScriptHelpers.tryGetADeclaration(symbol); + const sourceFile: ts.SourceFile | undefined = declaration?.getSourceFile(); const parent: ts.Symbol | undefined = TypeScriptInternals.getSymbolParent(symbol); - // First, try to determine navigation to symbol via its parent. - if (parent) { - if ( - parent.exports && - DeclarationReferenceGenerator._isSameSymbol(parent.exports.get(symbol.escapedName), symbol) - ) { - return Navigation.Exports; - } - if ( - parent.members && - DeclarationReferenceGenerator._isSameSymbol(parent.members.get(symbol.escapedName), symbol) - ) { - return Navigation.Members; - } - if ( - parent.globalExports && - DeclarationReferenceGenerator._isSameSymbol(parent.globalExports.get(symbol.escapedName), symbol) - ) { - return 'global'; - } + + // If it's a global, then use an Exports navigation. + if (sourceFile && !ts.isExternalModule(sourceFile)) { + return Navigation.Exports; } - // Next, try determining navigation to symbol by its node - if (symbol.valueDeclaration) { - const declaration: ts.Declaration = ts.isBindingElement(symbol.valueDeclaration) - ? ts.walkUpBindingElementsAndPatterns(symbol.valueDeclaration) - : symbol.valueDeclaration; - if (ts.isClassElement(declaration) && ts.isClassLike(declaration.parent)) { - // class members are an "export" if they have the static modifier. - return ts.getCombinedModifierFlags(declaration) & ts.ModifierFlags.Static - ? Navigation.Exports - : Navigation.Members; - } - if (ts.isTypeElement(declaration) || ts.isObjectLiteralElement(declaration)) { - // type and object literal element members are just members - return Navigation.Members; - } - if (ts.isEnumMember(declaration)) { - // enum members are exports - return Navigation.Exports; - } - if ( - ts.isExportSpecifier(declaration) || - ts.isExportAssignment(declaration) || - ts.isExportSpecifier(declaration) || - ts.isExportDeclaration(declaration) || - ts.isNamedExports(declaration) - ) { - return Navigation.Exports; + // If it's from an external library, then use an Exports navigation. + if (sourceFile && this._collector.program.isSourceFileFromExternalLibrary(sourceFile)) { + return Navigation.Exports; + } + + // Otherwise, this symbol is from the current package. + if (parent) { + // If we've found an exported CollectorEntity, then it's exported from the package entry point, so + // use an Exports navigation. + const namedDeclaration: ts.DeclarationName | undefined = ( + declaration as ts.NamedDeclaration | undefined + )?.name; + if (namedDeclaration && ts.isIdentifier(namedDeclaration)) { + const collectorEntity: CollectorEntity | undefined = + this._collector.tryGetEntityForNode(namedDeclaration); + if (collectorEntity && collectorEntity.exported) { + return Navigation.Exports; + } } - // declarations are exports if they have an `export` modifier. - if (ts.getCombinedModifierFlags(declaration) & ts.ModifierFlags.Export) { + + // If its parent symbol is not a source file, then use an Exports navigation. If the parent symbol is + // a source file, but it wasn't exported from the package entry point (in the check above), then the symbol + // is a local, so fall through below. + if (!DeclarationReferenceGenerator._isExternalModuleSymbol(parent)) { return Navigation.Exports; } - if (ts.isSourceFile(declaration.parent) && !ts.isExternalModule(declaration.parent)) { - // declarations in a source file are global if the source file is not a module. - return 'global'; - } } - // all other declarations are locals + + // Otherwise, we have a local symbol, so use a Locals navigation. These are either: + // + // 1. Symbols that are exported from a file module but not the package entry point. + // 2. Symbols that are not exported from their parent module. return Navigation.Locals; } @@ -218,20 +186,21 @@ export class DeclarationReferenceGenerator { meaning: ts.SymbolFlags, includeModuleSymbols: boolean ): DeclarationReference | undefined { + const declaration: ts.Node | undefined = TypeScriptHelpers.tryGetADeclaration(symbol); + const sourceFile: ts.SourceFile | undefined = declaration?.getSourceFile(); + let followedSymbol: ts.Symbol = symbol; if (followedSymbol.flags & ts.SymbolFlags.ExportValue) { - followedSymbol = this._typeChecker.getExportSymbolOfSymbol(followedSymbol); + followedSymbol = this._collector.typeChecker.getExportSymbolOfSymbol(followedSymbol); } if (followedSymbol.flags & ts.SymbolFlags.Alias) { - followedSymbol = this._typeChecker.getAliasedSymbol(followedSymbol); + followedSymbol = this._collector.typeChecker.getAliasedSymbol(followedSymbol); } if (DeclarationReferenceGenerator._isExternalModuleSymbol(followedSymbol)) { if (!includeModuleSymbols) { return undefined; } - const declaration: ts.Node | undefined = TypeScriptHelpers.tryGetADeclaration(symbol); - const sourceFile: ts.SourceFile | undefined = declaration?.getSourceFile(); return new DeclarationReference(this._sourceFileToModuleSource(sourceFile)); } @@ -270,13 +239,11 @@ export class DeclarationReferenceGenerator { } } - let navigation: Navigation | 'global' = - DeclarationReferenceGenerator._getNavigationToSymbol(followedSymbol); - if (navigation === 'global') { - if (parentRef.source !== GlobalSource.instance) { - parentRef = new DeclarationReference(GlobalSource.instance); - } - navigation = Navigation.Exports; + const navigation: Navigation = this._getNavigationToSymbol(followedSymbol); + + // If the symbol is a global, ensure the source is global. + if (sourceFile && !ts.isExternalModule(sourceFile) && parentRef.source !== GlobalSource.instance) { + parentRef = new DeclarationReference(GlobalSource.instance); } return parentRef @@ -313,7 +280,7 @@ export class DeclarationReferenceGenerator { if (grandParent && ts.isModuleDeclaration(grandParent)) { const grandParentSymbol: ts.Symbol | undefined = TypeScriptInternals.tryGetSymbolForDeclaration( grandParent, - this._typeChecker + this._collector.typeChecker ); if (grandParentSymbol) { return this._symbolToDeclarationReference( @@ -334,28 +301,27 @@ export class DeclarationReferenceGenerator { } private _getPackageName(sourceFile: ts.SourceFile): string { - if (this._program.isSourceFileFromExternalLibrary(sourceFile)) { - const packageJson: INodePackageJson | undefined = this._packageJsonLookup.tryLoadNodePackageJsonFor( - sourceFile.fileName - ); + if (this._collector.program.isSourceFileFromExternalLibrary(sourceFile)) { + const packageJson: INodePackageJson | undefined = + this._collector.packageJsonLookup.tryLoadNodePackageJsonFor(sourceFile.fileName); if (packageJson && packageJson.name) { return packageJson.name; } return DeclarationReferenceGenerator.unknownReference; } - return this._workingPackageName; + return this._collector.workingPackage.name; } private _sourceFileToModuleSource(sourceFile: ts.SourceFile | undefined): GlobalSource | ModuleSource { if (sourceFile && ts.isExternalModule(sourceFile)) { const packageName: string = this._getPackageName(sourceFile); - if (this._bundledPackageNames.has(packageName)) { + if (this._collector.bundledPackageNames.has(packageName)) { // The api-extractor.json config file has a "bundledPackages" setting, which causes imports from // certain NPM packages to be treated as part of the working project. In this case, we need to // substitute the working package name. - return new ModuleSource(this._workingPackageName); + return new ModuleSource(this._collector.workingPackage.name); } else { return new ModuleSource(packageName); } diff --git a/build-tests/api-extractor-scenarios/etc/ambientNameConflict/api-extractor-scenarios.api.json b/build-tests/api-extractor-scenarios/etc/ambientNameConflict/api-extractor-scenarios.api.json index 15d21093905..334ad0d65a5 100644 --- a/build-tests/api-extractor-scenarios/etc/ambientNameConflict/api-extractor-scenarios.api.json +++ b/build-tests/api-extractor-scenarios/etc/ambientNameConflict/api-extractor-scenarios.api.json @@ -197,7 +197,7 @@ { "kind": "Reference", "text": "MyPromise", - "canonicalReference": "api-extractor-scenarios!Promise:class" + "canonicalReference": "api-extractor-scenarios!~Promise:class" }, { "kind": "Content", diff --git a/build-tests/api-extractor-scenarios/etc/dynamicImportType/api-extractor-scenarios.api.json b/build-tests/api-extractor-scenarios/etc/dynamicImportType/api-extractor-scenarios.api.json index 22c6d529f22..d83bbf5d8df 100644 --- a/build-tests/api-extractor-scenarios/etc/dynamicImportType/api-extractor-scenarios.api.json +++ b/build-tests/api-extractor-scenarios/etc/dynamicImportType/api-extractor-scenarios.api.json @@ -304,7 +304,7 @@ { "kind": "Reference", "text": "Options", - "canonicalReference": "api-extractor-scenarios!Options:interface" + "canonicalReference": "api-extractor-scenarios!~Options:interface" }, { "kind": "Content", diff --git a/build-tests/api-extractor-scenarios/etc/exportImportStarAs2/api-extractor-scenarios.api.json b/build-tests/api-extractor-scenarios/etc/exportImportStarAs2/api-extractor-scenarios.api.json index fc67ce054b7..b2c90df8919 100644 --- a/build-tests/api-extractor-scenarios/etc/exportImportStarAs2/api-extractor-scenarios.api.json +++ b/build-tests/api-extractor-scenarios/etc/exportImportStarAs2/api-extractor-scenarios.api.json @@ -197,7 +197,7 @@ { "kind": "Reference", "text": "ForgottenClass", - "canonicalReference": "api-extractor-scenarios!ForgottenClass:class" + "canonicalReference": "api-extractor-scenarios!~ForgottenClass:class" }, { "kind": "Content", diff --git a/build-tests/api-extractor-scenarios/etc/exportImportedExternalDefault/api-extractor-scenarios.api.json b/build-tests/api-extractor-scenarios/etc/exportImportedExternalDefault/api-extractor-scenarios.api.json index 6270a70daef..82f4eca1655 100644 --- a/build-tests/api-extractor-scenarios/etc/exportImportedExternalDefault/api-extractor-scenarios.api.json +++ b/build-tests/api-extractor-scenarios/etc/exportImportedExternalDefault/api-extractor-scenarios.api.json @@ -184,7 +184,7 @@ { "kind": "Reference", "text": "Base", - "canonicalReference": "api-extractor-lib2-test!~DefaultClass:class" + "canonicalReference": "api-extractor-lib2-test!DefaultClass:class" }, { "kind": "Content", diff --git a/build-tests/api-extractor-scenarios/etc/namedDefaultImport/api-extractor-scenarios.api.json b/build-tests/api-extractor-scenarios/etc/namedDefaultImport/api-extractor-scenarios.api.json index 75915bd6498..6393d519bf2 100644 --- a/build-tests/api-extractor-scenarios/etc/namedDefaultImport/api-extractor-scenarios.api.json +++ b/build-tests/api-extractor-scenarios/etc/namedDefaultImport/api-extractor-scenarios.api.json @@ -202,7 +202,7 @@ { "kind": "Reference", "text": "default", - "canonicalReference": "api-extractor-lib2-test!~DefaultClass:class" + "canonicalReference": "api-extractor-lib2-test!DefaultClass:class" }, { "kind": "Content", @@ -230,7 +230,7 @@ { "kind": "Reference", "text": "DefaultClass_namedImport", - "canonicalReference": "api-extractor-lib2-test!~DefaultClass:class" + "canonicalReference": "api-extractor-lib2-test!DefaultClass:class" }, { "kind": "Content", @@ -258,7 +258,7 @@ { "kind": "Reference", "text": "DefaultClass_reExport", - "canonicalReference": "api-extractor-lib2-test!~DefaultClass:class" + "canonicalReference": "api-extractor-lib2-test!DefaultClass:class" }, { "kind": "Content", diff --git a/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.json b/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.json index cb43f150764..ab57b111a36 100644 --- a/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.json +++ b/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.json @@ -454,6 +454,35 @@ }, "implementsTokenRanges": [] }, + { + "kind": "Class", + "canonicalReference": "api-extractor-scenarios!SomeClass4:class", + "docComment": "/**\n * Reference to a symbol exported from another file, but not exported from the package.\n *\n * @public\n */\n", + "excerptTokens": [ + { + "kind": "Content", + "text": "export declare class SomeClass4 extends " + }, + { + "kind": "Reference", + "text": "SomeClass5", + "canonicalReference": "api-extractor-scenarios!~SomeClass5:class" + }, + { + "kind": "Content", + "text": " " + } + ], + "releaseTag": "Public", + "name": "SomeClass4", + "preserveMemberOrder": false, + "members": [], + "extendsTokenRange": { + "startIndex": 1, + "endIndex": 2 + }, + "implementsTokenRanges": [] + }, { "kind": "Enum", "canonicalReference": "api-extractor-scenarios!SomeEnum:enum", diff --git a/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.md b/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.md index 1f9c9929712..05cdc818d64 100644 --- a/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.md +++ b/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.md @@ -42,6 +42,12 @@ export class SomeClass1 { export class SomeClass3 extends SomeClass2 { } +// Warning: (ae-forgotten-export) The symbol "SomeClass5" needs to be exported by the entry point index.d.ts +// +// @public +export class SomeClass4 extends SomeClass5 { +} + // @public (undocumented) export enum SomeEnum { // (undocumented) diff --git a/build-tests/api-extractor-scenarios/etc/referenceTokens/rollup.d.ts b/build-tests/api-extractor-scenarios/etc/referenceTokens/rollup.d.ts index f27d291c0ad..09921ab39b7 100644 --- a/build-tests/api-extractor-scenarios/etc/referenceTokens/rollup.d.ts +++ b/build-tests/api-extractor-scenarios/etc/referenceTokens/rollup.d.ts @@ -34,6 +34,16 @@ declare class SomeClass2 { export declare class SomeClass3 extends SomeClass2 { } +/** + * Reference to a symbol exported from another file, but not exported from the package. + * @public + */ +export declare class SomeClass4 extends SomeClass5 { +} + +declare class SomeClass5 { +} + /** @public */ export declare enum SomeEnum { A = "A", diff --git a/build-tests/api-extractor-scenarios/src/referenceTokens/index.ts b/build-tests/api-extractor-scenarios/src/referenceTokens/index.ts index cd7e15d6750..4bacc687db0 100644 --- a/build-tests/api-extractor-scenarios/src/referenceTokens/index.ts +++ b/build-tests/api-extractor-scenarios/src/referenceTokens/index.ts @@ -2,6 +2,7 @@ // See LICENSE in the project root for license information. import { Lib2Class } from 'api-extractor-lib2-test'; +import { SomeClass5 } from './internal'; /** * Various namespace scenarios. @@ -86,3 +87,9 @@ export function someFunction7(): Promise { export function someFunction8(): Lib2Class { return new Lib2Class(); } + +/** + * Reference to a symbol exported from another file, but not exported from the package. + * @public + */ +export class SomeClass4 extends SomeClass5 {} diff --git a/build-tests/api-extractor-scenarios/src/referenceTokens/internal.ts b/build-tests/api-extractor-scenarios/src/referenceTokens/internal.ts new file mode 100644 index 00000000000..0261cad60e4 --- /dev/null +++ b/build-tests/api-extractor-scenarios/src/referenceTokens/internal.ts @@ -0,0 +1 @@ +export class SomeClass5 {} diff --git a/build-tests/install-test-workspace/workspace/common/pnpm-lock.yaml b/build-tests/install-test-workspace/workspace/common/pnpm-lock.yaml index 0edc8cb3e66..6cfa89d2909 100644 --- a/build-tests/install-test-workspace/workspace/common/pnpm-lock.yaml +++ b/build-tests/install-test-workspace/workspace/common/pnpm-lock.yaml @@ -5,13 +5,13 @@ importers: typescript-newest-test: specifiers: '@rushstack/eslint-config': file:rushstack-eslint-config-3.0.0.tgz - '@rushstack/heft': file:rushstack-heft-0.47.0.tgz + '@rushstack/heft': file:rushstack-heft-0.47.2.tgz eslint: ~8.7.0 tslint: ~5.20.1 typescript: ~4.7.4 devDependencies: '@rushstack/eslint-config': file:../temp/tarballs/rushstack-eslint-config-3.0.0.tgz_eslint@8.7.0+typescript@4.7.4 - '@rushstack/heft': file:../temp/tarballs/rushstack-heft-0.47.0.tgz + '@rushstack/heft': file:../temp/tarballs/rushstack-heft-0.47.2.tgz eslint: 8.7.0 tslint: 5.20.1_typescript@4.7.4 typescript: 4.7.4 @@ -19,13 +19,13 @@ importers: typescript-v3-test: specifiers: '@rushstack/eslint-config': file:rushstack-eslint-config-3.0.0.tgz - '@rushstack/heft': file:rushstack-heft-0.47.0.tgz + '@rushstack/heft': file:rushstack-heft-0.47.2.tgz eslint: ~8.7.0 tslint: ~5.20.1 typescript: ~4.7.4 devDependencies: '@rushstack/eslint-config': file:../temp/tarballs/rushstack-eslint-config-3.0.0.tgz_eslint@8.7.0+typescript@4.7.4 - '@rushstack/heft': file:../temp/tarballs/rushstack-heft-0.47.0.tgz + '@rushstack/heft': file:../temp/tarballs/rushstack-heft-0.47.2.tgz eslint: 8.7.0 tslint: 5.20.1_typescript@4.7.4 typescript: 4.7.4 @@ -328,7 +328,7 @@ packages: dev: true /argparse/2.0.1: - resolution: {integrity: sha1-JG9Q88p4oyQPbJl+ipvR6sSeSzg=} + resolution: {integrity: sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==} dev: true /array-includes/3.1.5: @@ -1803,10 +1803,10 @@ packages: - typescript dev: true - file:../temp/tarballs/rushstack-heft-0.47.0.tgz: - resolution: {tarball: file:../temp/tarballs/rushstack-heft-0.47.0.tgz} + file:../temp/tarballs/rushstack-heft-0.47.2.tgz: + resolution: {tarball: file:../temp/tarballs/rushstack-heft-0.47.2.tgz} name: '@rushstack/heft' - version: 0.47.0 + version: 0.47.2 engines: {node: '>=10.13.0'} hasBin: true dependencies: diff --git a/common/changes/@microsoft/api-extractor/refs_2022-08-12-00-18.json b/common/changes/@microsoft/api-extractor/refs_2022-08-12-00-18.json new file mode 100644 index 00000000000..fa2c7e534f4 --- /dev/null +++ b/common/changes/@microsoft/api-extractor/refs_2022-08-12-00-18.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/api-extractor", + "comment": "Fix incorrect declaration references for symbols not exported from the package's entry point.", + "type": "patch" + } + ], + "packageName": "@microsoft/api-extractor" +} \ No newline at end of file From b68fec6086358cb96511f4677ca52a7959dca004 Mon Sep 17 00:00:00 2001 From: Zack Elliott Date: Wed, 24 Aug 2022 17:09:06 -0700 Subject: [PATCH 2/3] Handle some Members navigation step edge cases --- .../DeclarationReferenceGenerator.ts | 34 +++- .../etc/api-extractor-lib2-test.api.md | 4 +- .../api-extractor-lib2-test/src/index.ts | 4 +- .../api-extractor-scenarios.api.json | 171 +++++++++++++++++- .../api-extractor-scenarios.api.md | 13 +- .../etc/referenceTokens/rollup.d.ts | 15 +- .../src/referenceTokens/index.ts | 19 +- 7 files changed, 230 insertions(+), 30 deletions(-) diff --git a/apps/api-extractor/src/generators/DeclarationReferenceGenerator.ts b/apps/api-extractor/src/generators/DeclarationReferenceGenerator.ts index 5cd34914f98..ccfe9347d26 100644 --- a/apps/api-extractor/src/generators/DeclarationReferenceGenerator.ts +++ b/apps/api-extractor/src/generators/DeclarationReferenceGenerator.ts @@ -92,20 +92,27 @@ export class DeclarationReferenceGenerator { const sourceFile: ts.SourceFile | undefined = declaration?.getSourceFile(); const parent: ts.Symbol | undefined = TypeScriptInternals.getSymbolParent(symbol); - // If it's a global, then use an Exports navigation. - if (sourceFile && !ts.isExternalModule(sourceFile)) { - return Navigation.Exports; - } + // If it's global or from an external library, then use either Members or Exports. It's not possible for + // global symbols or external library symbols to be Locals. + const isGlobal: boolean = !!sourceFile && !ts.isExternalModule(sourceFile); + const isFromExternalLibrary: boolean = + !!sourceFile && this._collector.program.isSourceFileFromExternalLibrary(sourceFile); + if (isGlobal || isFromExternalLibrary) { + if ( + parent && + parent.members && + DeclarationReferenceGenerator._isSameSymbol(parent.members.get(symbol.escapedName), symbol) + ) { + return Navigation.Members; + } - // If it's from an external library, then use an Exports navigation. - if (sourceFile && this._collector.program.isSourceFileFromExternalLibrary(sourceFile)) { return Navigation.Exports; } // Otherwise, this symbol is from the current package. if (parent) { // If we've found an exported CollectorEntity, then it's exported from the package entry point, so - // use an Exports navigation. + // use Exports. const namedDeclaration: ts.DeclarationName | undefined = ( declaration as ts.NamedDeclaration | undefined )?.name; @@ -117,10 +124,17 @@ export class DeclarationReferenceGenerator { } } - // If its parent symbol is not a source file, then use an Exports navigation. If the parent symbol is - // a source file, but it wasn't exported from the package entry point (in the check above), then the symbol - // is a local, so fall through below. + // If its parent symbol is not a source file, then use either Exports or Members. If the parent symbol + // is a source file, but it wasn't exported from the package entry point (in the check above), then the + // symbol is a local, so fall through below. if (!DeclarationReferenceGenerator._isExternalModuleSymbol(parent)) { + if ( + parent.members && + DeclarationReferenceGenerator._isSameSymbol(parent.members.get(symbol.escapedName), symbol) + ) { + return Navigation.Members; + } + return Navigation.Exports; } } diff --git a/build-tests/api-extractor-lib2-test/etc/api-extractor-lib2-test.api.md b/build-tests/api-extractor-lib2-test/etc/api-extractor-lib2-test.api.md index c44758dcb4f..e89ae310395 100644 --- a/build-tests/api-extractor-lib2-test/etc/api-extractor-lib2-test.api.md +++ b/build-tests/api-extractor-lib2-test/etc/api-extractor-lib2-test.api.md @@ -7,16 +7,16 @@ // @public (undocumented) class DefaultClass { } - export default DefaultClass; // @public (undocumented) export class Lib2Class { + // (undocumented) + prop: number; } // @alpha (undocumented) export interface Lib2Interface { } - ``` diff --git a/build-tests/api-extractor-lib2-test/src/index.ts b/build-tests/api-extractor-lib2-test/src/index.ts index f9276753a68..6fd2ffc71cb 100644 --- a/build-tests/api-extractor-lib2-test/src/index.ts +++ b/build-tests/api-extractor-lib2-test/src/index.ts @@ -11,7 +11,9 @@ */ /** @public */ -export class Lib2Class {} +export class Lib2Class { + prop: number; +} /** @alpha */ export interface Lib2Interface {} diff --git a/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.json b/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.json index ab57b111a36..4bc5e0d37a6 100644 --- a/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.json +++ b/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.json @@ -629,7 +629,16 @@ "excerptTokens": [ { "kind": "Content", - "text": "export declare function someFunction7(): " + "text": "export declare function someFunction7({ " + }, + { + "kind": "Reference", + "text": "then", + "canonicalReference": "!Promise#then" + }, + { + "kind": "Content", + "text": ": then2 }: " }, { "kind": "Reference", @@ -640,18 +649,40 @@ "kind": "Content", "text": "" }, + { + "kind": "Content", + "text": "): " + }, + { + "kind": "Content", + "text": "typeof " + }, + { + "kind": "Reference", + "text": "Date.prototype.getDate", + "canonicalReference": "!Date#getDate:member" + }, { "kind": "Content", "text": ";" } ], "returnTypeTokenRange": { - "startIndex": 1, - "endIndex": 3 + "startIndex": 6, + "endIndex": 8 }, "releaseTag": "Public", "overloadIndex": 1, - "parameters": [], + "parameters": [ + { + "parameterName": "{ then: then2 }", + "parameterTypeTokenRange": { + "startIndex": 3, + "endIndex": 5 + }, + "isOptional": false + } + ], "name": "someFunction7" }, { @@ -661,26 +692,150 @@ "excerptTokens": [ { "kind": "Content", - "text": "export declare function someFunction8(): " + "text": "export declare function someFunction8({ " + }, + { + "kind": "Reference", + "text": "prop", + "canonicalReference": "api-extractor-lib2-test!Lib2Class#prop" + }, + { + "kind": "Content", + "text": ": prop2 }: " }, { "kind": "Reference", "text": "Lib2Class", "canonicalReference": "api-extractor-lib2-test!Lib2Class:class" }, + { + "kind": "Content", + "text": "): " + }, + { + "kind": "Content", + "text": "void" + }, { "kind": "Content", "text": ";" } ], "returnTypeTokenRange": { - "startIndex": 1, - "endIndex": 2 + "startIndex": 5, + "endIndex": 6 }, "releaseTag": "Public", "overloadIndex": 1, - "parameters": [], + "parameters": [ + { + "parameterName": "{ prop: prop2 }", + "parameterTypeTokenRange": { + "startIndex": 3, + "endIndex": 4 + }, + "isOptional": false + } + ], "name": "someFunction8" + }, + { + "kind": "Function", + "canonicalReference": "api-extractor-scenarios!someFunction9:function(1)", + "docComment": "/**\n * Interface member reference.\n *\n * @public\n */\n", + "excerptTokens": [ + { + "kind": "Content", + "text": "export declare function someFunction9({ " + }, + { + "kind": "Reference", + "text": "prop", + "canonicalReference": "api-extractor-scenarios!SomeInterface1#prop" + }, + { + "kind": "Content", + "text": ": prop2 }: " + }, + { + "kind": "Reference", + "text": "SomeInterface1", + "canonicalReference": "api-extractor-scenarios!SomeInterface1:interface" + }, + { + "kind": "Content", + "text": "): " + }, + { + "kind": "Content", + "text": "void" + }, + { + "kind": "Content", + "text": ";" + } + ], + "returnTypeTokenRange": { + "startIndex": 5, + "endIndex": 6 + }, + "releaseTag": "Public", + "overloadIndex": 1, + "parameters": [ + { + "parameterName": "{ prop: prop2 }", + "parameterTypeTokenRange": { + "startIndex": 3, + "endIndex": 4 + }, + "isOptional": false + } + ], + "name": "someFunction9" + }, + { + "kind": "Interface", + "canonicalReference": "api-extractor-scenarios!SomeInterface1:interface", + "docComment": "/**\n * @public\n */\n", + "excerptTokens": [ + { + "kind": "Content", + "text": "export interface SomeInterface1 " + } + ], + "releaseTag": "Public", + "name": "SomeInterface1", + "preserveMemberOrder": false, + "members": [ + { + "kind": "PropertySignature", + "canonicalReference": "api-extractor-scenarios!SomeInterface1#prop:member", + "docComment": "", + "excerptTokens": [ + { + "kind": "Content", + "text": "prop: " + }, + { + "kind": "Content", + "text": "number" + }, + { + "kind": "Content", + "text": ";" + } + ], + "isReadonly": false, + "isOptional": false, + "releaseTag": "Public", + "name": "prop", + "propertyTypeTokenRange": { + "startIndex": 1, + "endIndex": 2 + } + } + ], + "extendsTokenRanges": [] } ] } diff --git a/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.md b/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.md index 05cdc818d64..3e30f5fb6ec 100644 --- a/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.md +++ b/build-tests/api-extractor-scenarios/etc/referenceTokens/api-extractor-scenarios.api.md @@ -65,10 +65,19 @@ export function someFunction5(): SomeEnum.A; export function someFunction6(): typeof SomeClass1.staticProp; // @public -export function someFunction7(): Promise; +export function someFunction7({ then: then2 }: Promise): typeof Date.prototype.getDate; // @public -export function someFunction8(): Lib2Class; +export function someFunction8({ prop: prop2 }: Lib2Class): void; + +// @public +export function someFunction9({ prop: prop2 }: SomeInterface1): void; + +// @public (undocumented) +export interface SomeInterface1 { + // (undocumented) + prop: number; +} // (No @packageDocumentation comment for this package) diff --git a/build-tests/api-extractor-scenarios/etc/referenceTokens/rollup.d.ts b/build-tests/api-extractor-scenarios/etc/referenceTokens/rollup.d.ts index 09921ab39b7..90ea87b8f9f 100644 --- a/build-tests/api-extractor-scenarios/etc/referenceTokens/rollup.d.ts +++ b/build-tests/api-extractor-scenarios/etc/referenceTokens/rollup.d.ts @@ -67,12 +67,23 @@ export declare function someFunction6(): typeof SomeClass1.staticProp; * Global symbol reference. * @public */ -export declare function someFunction7(): Promise; +export declare function someFunction7({ then: then2 }: Promise): typeof Date.prototype.getDate; /** * External symbol reference. * @public */ -export declare function someFunction8(): Lib2Class; +export declare function someFunction8({ prop: prop2 }: Lib2Class): void; + +/** + * Interface member reference. + * @public + */ +export declare function someFunction9({ prop: prop2 }: SomeInterface1): void; + +/** @public */ +export declare interface SomeInterface1 { + prop: number; +} export { } diff --git a/build-tests/api-extractor-scenarios/src/referenceTokens/index.ts b/build-tests/api-extractor-scenarios/src/referenceTokens/index.ts index 4bacc687db0..3ce006f9077 100644 --- a/build-tests/api-extractor-scenarios/src/referenceTokens/index.ts +++ b/build-tests/api-extractor-scenarios/src/referenceTokens/index.ts @@ -64,6 +64,17 @@ export function someFunction6(): typeof SomeClass1.staticProp { return 5; } +/** @public */ +export interface SomeInterface1 { + prop: number; +} + +/** + * Interface member reference. + * @public + */ +export function someFunction9({ prop: prop2 }: SomeInterface1): void {} + class SomeClass2 {} /** @@ -76,17 +87,15 @@ export class SomeClass3 extends SomeClass2 {} * Global symbol reference. * @public */ -export function someFunction7(): Promise { - return Promise.resolve(); +export function someFunction7({ then: then2 }: Promise): typeof Date.prototype.getDate { + return () => 5; } /** * External symbol reference. * @public */ -export function someFunction8(): Lib2Class { - return new Lib2Class(); -} +export function someFunction8({ prop: prop2 }: Lib2Class): void {} /** * Reference to a symbol exported from another file, but not exported from the package. From 7f2b01ad365c4f9579496d6cf77d7be050b55949 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Fri, 2 Sep 2022 00:07:14 -0700 Subject: [PATCH 3/3] Convert this to a MINOR release; technically it is a bug fix, but the logic change is nontrivial --- .../changes/@microsoft/api-extractor/refs_2022-08-12-00-18.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/changes/@microsoft/api-extractor/refs_2022-08-12-00-18.json b/common/changes/@microsoft/api-extractor/refs_2022-08-12-00-18.json index fa2c7e534f4..543c6afd607 100644 --- a/common/changes/@microsoft/api-extractor/refs_2022-08-12-00-18.json +++ b/common/changes/@microsoft/api-extractor/refs_2022-08-12-00-18.json @@ -3,7 +3,7 @@ { "packageName": "@microsoft/api-extractor", "comment": "Fix incorrect declaration references for symbols not exported from the package's entry point.", - "type": "patch" + "type": "minor" } ], "packageName": "@microsoft/api-extractor"