Skip to content

Commit fe87df4

Browse files
pgonzalpgonzal
authored
Merge pull request microsoft#985 from Microsoft/pgonzal/ae7-pr-feedback
[api-extractor] Miscellaneous PR feedback
2 parents f5ced04 + 2d6fcdb commit fe87df4

55 files changed

Lines changed: 915 additions & 192 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎apps/api-documenter/src/documenters/MarkdownDocumenter.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -569,10 +569,8 @@ export class MarkdownDocumenter {
569569
);
570570

571571
if (apiFunctionLike instanceof ApiDocumentedItem) {
572-
if (apiFunctionLike.tsdocComment) {
573-
if (apiFunctionLike.tsdocComment.returnsBlock) {
574-
this._appendSection(output, apiFunctionLike.tsdocComment.returnsBlock.content);
575-
}
572+
if (apiFunctionLike.tsdocComment && apiFunctionLike.tsdocComment.returnsBlock) {
573+
this._appendSection(output, apiFunctionLike.tsdocComment.returnsBlock.content);
576574
}
577575
}
578576

@@ -596,6 +594,13 @@ export class MarkdownDocumenter {
596594
]);
597595
}
598596

597+
/**
598+
* This generates a DocTableCell for an ApiItem including the summary section and "(BETA)" annotation.
599+
*
600+
* @remarks
601+
* We mostly assume that the input is an ApiDocumentedItem, but it's easier to perform this as a runtime
602+
* check than to have each caller perform a type cast.
603+
*/
599604
private _createDescriptionCell(apiItem: ApiItem): DocTableCell {
600605
const configuration: TSDocConfiguration = this._tsdocConfiguration;
601606

@@ -659,6 +664,9 @@ export class MarkdownDocumenter {
659664
switch (hierarchyItem.kind) {
660665
case ApiItemKind.Model:
661666
case ApiItemKind.EntryPoint:
667+
// We don't show the model as part of the breadcrumb because it is the root-level container.
668+
// We don't show the entry point because today API Extractor doesn't support multiple entry points;
669+
// this may change in the future.
662670
break;
663671
default:
664672
output.appendNodesInParagraph([

‎apps/api-documenter/src/documenters/YamlDocumenter.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ import {
99
JsonSchema,
1010
PackageName,
1111
FileSystem,
12-
NewlineKind
12+
NewlineKind,
13+
InternalError
1314
} from '@microsoft/node-core-library';
1415
import { StringBuilder, DocSection, DocComment } from '@microsoft/tsdoc';
1516
import {
@@ -108,7 +109,7 @@ export class YamlDocumenter {
108109

109110
if (this._shouldEmbed(apiItem.kind)) {
110111
if (!parentYamlFile) {
111-
throw new Error('Missing file context'); // program bug
112+
throw new InternalError('Missing file context');
112113
}
113114
parentYamlFile.items.push(yamlItem);
114115
} else {

‎apps/api-documenter/src/markdown/MarkdownEmitter.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
DocErrorText
1919
} from '@microsoft/tsdoc';
2020
import { IndentedWriter } from '@microsoft/api-extractor';
21+
import { InternalError } from '@microsoft/node-core-library';
2122

2223
export interface IMarkdownEmitterOptions {
2324
}
@@ -172,7 +173,7 @@ export class MarkdownEmitter {
172173
protected writeLinkTagWithCodeDestination(docLinkTag: DocLinkTag, context: IMarkdownEmitterContext): void {
173174

174175
// The subclass needs to implement this to support code destinations
175-
throw new Error('writeLinkTagWithCodeDestination()');
176+
throw new InternalError('writeLinkTagWithCodeDestination()');
176177
}
177178

178179
/** @virtual */

‎apps/api-documenter/src/nodes/DocHeading.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export interface IDocHeadingParameters extends IDocNodeParameters {
1616
}
1717

1818
/**
19-
* Represents table, similar to an HTML `<h1>` or `<h2>` element.
19+
* Represents a section header similar to an HTML `<h1>` or `<h2>` element.
2020
*/
2121
export class DocHeading extends DocNode {
2222
public readonly title: string;

‎apps/api-extractor/src/analyzer/AstDeclaration.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import * as ts from 'typescript';
55
import { AstSymbol } from './AstSymbol';
66
import { Span } from './Span';
7+
import { InternalError } from '@microsoft/node-core-library';
78

89
/**
910
* Constructor options for AstDeclaration
@@ -103,11 +104,11 @@ export class AstDeclaration {
103104
*/
104105
public _notifyChildAttach(child: AstDeclaration): void {
105106
if (child.parent !== this) {
106-
throw new Error('Program Bug: Invalid call to notifyChildAttach()');
107+
throw new InternalError('Invalid call to notifyChildAttach()');
107108
}
108109

109110
if (this.astSymbol.analyzed) {
110-
throw new Error('Program Bug: _notifyChildAttach() called after analysis is already complete');
111+
throw new InternalError('_notifyChildAttach() called after analysis is already complete');
111112
}
112113

113114
this._analyzedChildren.push(child);
@@ -152,7 +153,7 @@ export class AstDeclaration {
152153
*/
153154
public _notifyReferencedAstSymbol(referencedAstSymbol: AstSymbol): void {
154155
if (this.astSymbol.analyzed) {
155-
throw new Error('Program Bug: notifyReferencedAstSymbol() called after analysis is already complete');
156+
throw new InternalError('notifyReferencedAstSymbol() called after analysis is already complete');
156157
}
157158

158159
for (let current: AstDeclaration | undefined = this; current; current = current.parent) {

‎apps/api-extractor/src/analyzer/AstSymbol.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import * as ts from 'typescript';
55
import { AstImport } from './AstImport';
66
import { AstDeclaration } from './AstDeclaration';
7+
import { InternalError } from '@microsoft/node-core-library';
78

89
/**
910
* Constructor options for AstSymbol
@@ -130,7 +131,7 @@ export class AstSymbol {
130131
*/
131132
public _notifyDeclarationAttach(astDeclaration: AstDeclaration): void {
132133
if (this.analyzed) {
133-
throw new Error('Program Bug: _notifyDeclarationAttach() called after analysis is already complete');
134+
throw new InternalError('_notifyDeclarationAttach() called after analysis is already complete');
134135
}
135136
this._astDeclarations.push(astDeclaration);
136137
}
@@ -142,7 +143,7 @@ export class AstSymbol {
142143
*/
143144
public _notifyAnalyzed(): void {
144145
if (this.parentAstSymbol) {
145-
throw new Error('Program Bug: _notifyAnalyzed() called for an AstSymbol which is not the root');
146+
throw new InternalError('_notifyAnalyzed() called for an AstSymbol which is not the root');
146147
}
147148
this._analyzed = true;
148149
}

‎apps/api-extractor/src/analyzer/AstSymbolTable.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
/* tslint:disable:no-bitwise */
55

66
import * as ts from 'typescript';
7-
import { PackageJsonLookup } from '@microsoft/node-core-library';
7+
import { PackageJsonLookup, InternalError } from '@microsoft/node-core-library';
88

99
import { AstDeclaration } from './AstDeclaration';
1010
import { SymbolAnalyzer, IFollowAliasesResult } from './SymbolAnalyzer';
@@ -170,7 +170,7 @@ export class AstSymbolTable {
170170
throw new Error('Child declaration not found for the specified node');
171171
}
172172
if (childAstDeclaration.parent !== parentAstDeclaration) {
173-
throw new Error('Program Bug: The found child is not attached to the parent AstDeclaration');
173+
throw new InternalError('The found child is not attached to the parent AstDeclaration');
174174
}
175175

176176
return childAstDeclaration;
@@ -230,7 +230,7 @@ export class AstSymbolTable {
230230
const astDeclaration: AstDeclaration | undefined
231231
= this._astDeclarationsByDeclaration.get(node);
232232
if (!astDeclaration) {
233-
throw new Error('Program Bug: Unable to find constructed AstDeclaration');
233+
throw new InternalError('Unable to find constructed AstDeclaration');
234234
}
235235

236236
return astDeclaration;
@@ -243,7 +243,7 @@ export class AstSymbolTable {
243243

244244
const symbol: ts.Symbol | undefined = TypeScriptHelpers.getSymbolForDeclaration(node as ts.Declaration);
245245
if (!symbol) {
246-
throw new Error('Program Bug: Unable to find symbol for node');
246+
throw new InternalError('Unable to find symbol for node');
247247
}
248248

249249
return this._fetchAstSymbol(symbol, true);
@@ -267,7 +267,7 @@ export class AstSymbolTable {
267267

268268
if (!astSymbol) {
269269
if (!followedSymbol.declarations || followedSymbol.declarations.length < 1) {
270-
throw new Error('Program Bug: Followed a symbol with no declarations');
270+
throw new InternalError('Followed a symbol with no declarations');
271271
}
272272

273273
const astImport: AstImport | undefined = followAliasesResult.astImport;
@@ -316,7 +316,7 @@ export class AstSymbolTable {
316316
if (!nominal) {
317317
for (const declaration of followedSymbol.declarations || []) {
318318
if (!SymbolAnalyzer.isAstDeclaration(declaration.kind)) {
319-
throw new Error(`Program Bug: The "${followedSymbol.name}" symbol uses the construct`
319+
throw new InternalError(`The "${followedSymbol.name}" symbol uses the construct`
320320
+ ` "${ts.SyntaxKind[declaration.kind]}" which may be an unimplemented language feature`);
321321
}
322322
}
@@ -342,7 +342,7 @@ export class AstSymbolTable {
342342

343343
parentAstSymbol = this._fetchAstSymbol(parentSymbol, addIfMissing);
344344
if (!parentAstSymbol) {
345-
throw new Error('Program bug: Unable to construct a parent AstSymbol for '
345+
throw new InternalError('Unable to construct a parent AstSymbol for '
346346
+ followedSymbol.name);
347347
}
348348
}
@@ -373,12 +373,12 @@ export class AstSymbolTable {
373373
const parentDeclaration: ts.Node | undefined = this._tryFindFirstAstDeclarationParent(declaration);
374374

375375
if (!parentDeclaration) {
376-
throw new Error('Program bug: Missing parent declaration');
376+
throw new InternalError('Missing parent declaration');
377377
}
378378

379379
parentAstDeclaration = this._astDeclarationsByDeclaration.get(parentDeclaration);
380380
if (!parentAstDeclaration) {
381-
throw new Error('Program bug: Missing parent AstDeclaration');
381+
throw new InternalError('Missing parent AstDeclaration');
382382
}
383383
}
384384

@@ -398,7 +398,7 @@ export class AstSymbolTable {
398398
//
399399
// This assumption might be violated if the caller did something unusual like feeding random
400400
// symbols to AstSymbolTable.analyze() in the middle of the analysis.
401-
throw new Error('Program Bug: The symbol ' + astSymbol.localName + ' is being imported'
401+
throw new InternalError('The symbol ' + astSymbol.localName + ' is being imported'
402402
+ ' after it was already registered as non-imported');
403403
}
404404

‎apps/api-extractor/src/api/mixins/ApiDeclarationMixin.ts‎

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import { ApiItem, IApiItemJson, IApiItemConstructor, IApiItemOptions } from '../
55
import { ApiDocumentedItem } from '../model/ApiDocumentedItem';
66
import { Excerpt, ExcerptToken, IExcerptTokenRange, IDeclarationExcerpt, ExcerptName } from './Excerpt';
77

8-
/** @public */
8+
/**
9+
* Constructor options for {@link (ApiDeclarationMixin:interface)}.
10+
* @public
11+
*/
912
export interface IApiDeclarationMixinOptions extends IApiItemOptions {
1013
declarationExcerpt: IDeclarationExcerpt;
1114
}
@@ -17,18 +20,51 @@ const _excerpt: unique symbol = Symbol('ApiDeclarationMixin._excerpt');
1720
const _excerptTokens: unique symbol = Symbol('ApiDeclarationMixin._excerptTokens');
1821
const _embeddedExcerptsByName: unique symbol = Symbol('ApiDeclarationMixin._embeddedExcerptsByName');
1922

20-
/** @public */
23+
/**
24+
* The mixin base class for API items that have an associated source code excerpt containing a
25+
* TypeScript declaration.
26+
*
27+
* @remarks
28+
*
29+
* This is part of the {@link ApiModel} hierarchy of classes, which are serializable representations of
30+
* API declarations. The non-abstract classes (e.g. `ApiClass`, `ApiEnum`, `ApiInterface`, etc.) use
31+
* TypeScript "mixin" functions (e.g. `ApiDeclarationMixin`, `ApiItemContainerMixin`, etc.) to add various
32+
* features that cannot be represented as a normal inheritance chain (since TypeScript does not allow a child class
33+
* to extend more than one base class). The "mixin" is a TypeScript merged declaration with three components:
34+
* the function that generates a subclass, an interface that describes the members of the subclass, and
35+
* a namespace containing static members of the class.
36+
*
37+
* Most `ApiItem` subclasses have declarations and thus extend `ApiDeclarationMixin`. Counterexamples include
38+
* `ApiModel` and `ApiPackage`, which do not have any corresponding TypeScript source code.
39+
*
40+
* @public
41+
*/
2142
// tslint:disable-next-line:interface-name
2243
export interface ApiDeclarationMixin extends ApiItem {
44+
/**
45+
* The source code excerpt where the API item is declared.
46+
*/
2347
readonly excerpt: Excerpt;
2448

49+
/**
50+
* The individual source code tokens that comprise the main excerpt.
51+
*/
2552
readonly excerptTokens: ReadonlyArray<ExcerptToken>;
2653

54+
/**
55+
* A collection of named embedded excerpts. For example, if `ApiDeclarationMixin.excerpt` is a property
56+
* declaration, then `embeddedExcerptsByName` might contain an embedded excerpt corresponding to the
57+
* type of the property.
58+
*/
2759
readonly embeddedExcerptsByName: ReadonlyMap<ExcerptName, Excerpt>;
2860

2961
/** @override */
3062
serializeInto(jsonObject: Partial<IApiItemJson>): void;
3163

64+
/**
65+
* Returns a member of the {@link (ApiDeclarationMixin:interface).embeddedExcerptsByName} map,
66+
* or throws an exception if was not found.
67+
*/
3268
getEmbeddedExcerpt(name: ExcerptName): Excerpt;
3369

3470
/**
@@ -38,7 +74,14 @@ export interface ApiDeclarationMixin extends ApiItem {
3874
getExcerptWithModifiers(): string;
3975
}
4076

41-
/** @public */
77+
/**
78+
* Mixin function for {@link (ApiDeclarationMixin:interface)}.
79+
*
80+
* @param baseClass - The base class to be extended
81+
* @returns A child class that extends baseClass, adding the {@link (ApiDeclarationMixin:interface)} functionality.
82+
*
83+
* @public
84+
*/
4285
export function ApiDeclarationMixin<TBaseClass extends IApiItemConstructor>(baseClass: TBaseClass):
4386
TBaseClass & (new (...args: any[]) => ApiDeclarationMixin) { // tslint:disable-line:no-any
4487

@@ -148,8 +191,20 @@ export function ApiDeclarationMixin<TBaseClass extends IApiItemConstructor>(base
148191
return MixedClass;
149192
}
150193

151-
/** @public */
194+
/**
195+
* Static members for {@link (ApiDeclarationMixin:interface)}.
196+
* @public
197+
*/
152198
export namespace ApiDeclarationMixin {
199+
/**
200+
* A type guard that tests whether the specified `ApiItem` subclass extends the `ApiDeclarationMixin` mixin.
201+
*
202+
* @remarks
203+
*
204+
* The JavaScript `instanceof` operator cannot be used to test for mixin inheritance, because each invocation of
205+
* the mixin function produces a different subclass. (This could be mitigated by `Symbol.hasInstance`, however
206+
* the TypeScript type system cannot invoke a runtime test.)
207+
*/
153208
export function isBaseClassOf(apiItem: ApiItem): apiItem is ApiDeclarationMixin {
154209
return apiItem.hasOwnProperty(_excerpt);
155210
}

0 commit comments

Comments
 (0)