X Tutup
Skip to content

Commit cab1eab

Browse files
octogonzoctogonz
authored
Merge pull request microsoft#1544 from D4N14L/danade/FunctionAndMethodAccessability
[api-extractor] Allow separate release tags for overloaded functions and methods
2 parents b6a8c52 + ca2cf88 commit cab1eab

File tree

24 files changed

+948
-172
lines changed

24 files changed

+948
-172
lines changed

‎apps/api-extractor/src/api/ExtractorMessageId.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ export const enum ExtractorMessageId {
4747
*/
4848
InternalMissingUnderscore = 'ae-internal-missing-underscore',
4949

50+
/**
51+
* "Mixed release tags are not allowed for ___ because one of its declarations is marked as `@internal`."
52+
*/
53+
InternalMixedReleaseTag = 'ae-internal-mixed-release-tag',
54+
5055
/**
5156
* "The `@preapproved` tag cannot be applied to ___ because it is not a supported declaration type."
5257
*/
@@ -91,6 +96,7 @@ export const allExtractorMessageIds: Set<string> = new Set<string>([
9196
'ae-misplaced-package-tag',
9297
'ae-forgotten-export',
9398
'ae-internal-missing-underscore',
99+
'ae-internal-mixed-release-tag',
94100
'ae-preapproved-unsupported-type',
95101
'ae-preapproved-bad-release-tag',
96102
'ae-unresolved-inheritdoc-reference',

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

Lines changed: 42 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -511,70 +511,21 @@ export class Collector {
511511
this._calculateMetadataForDeclaration(astDeclaration);
512512
}
513513

514-
// We know we solved parentAstSymbol.metadata above
515-
const parentSymbolMetadata: SymbolMetadata | undefined = astSymbol.parentAstSymbol
516-
? astSymbol.parentAstSymbol.metadata as SymbolMetadata : undefined;
517-
518-
const symbolMetadata: SymbolMetadata = new SymbolMetadata();
519-
520-
// Do any of the declarations have a release tag?
521-
let effectiveReleaseTag: ReleaseTag = ReleaseTag.None;
514+
// The most public effectiveReleaseTag for all declarations
515+
let maxEffectiveReleaseTag: ReleaseTag = ReleaseTag.None;
522516

523517
for (const astDeclaration of astSymbol.astDeclarations) {
524518
// We know we solved this above
525519
const declarationMetadata: DeclarationMetadata = astDeclaration.metadata as DeclarationMetadata;
520+
const effectiveReleaseTag: ReleaseTag = declarationMetadata.effectiveReleaseTag;
526521

527-
const declaredReleaseTag: ReleaseTag = declarationMetadata.declaredReleaseTag;
528-
529-
if (declaredReleaseTag !== ReleaseTag.None) {
530-
if (effectiveReleaseTag !== ReleaseTag.None && effectiveReleaseTag !== declaredReleaseTag) {
531-
if (!astSymbol.isExternal) { // for now, don't report errors for external code
532-
this.messageRouter.addAnalyzerIssue(
533-
ExtractorMessageId.DifferentReleaseTags,
534-
'This symbol has another declaration with a different release tag',
535-
astDeclaration
536-
);
537-
}
538-
} else {
539-
effectiveReleaseTag = declaredReleaseTag;
540-
}
541-
}
542-
}
543-
544-
// If this declaration doesn't have a release tag, then inherit it from the parent
545-
if (effectiveReleaseTag === ReleaseTag.None && astSymbol.parentAstSymbol) {
546-
if (parentSymbolMetadata) {
547-
effectiveReleaseTag = parentSymbolMetadata.releaseTag;
548-
}
549-
}
550-
551-
if (effectiveReleaseTag === ReleaseTag.None) {
552-
if (!astSymbol.isExternal) { // for now, don't report errors for external code
553-
// Don't report missing release tags for forgotten exports
554-
const entity: CollectorEntity | undefined = this._entitiesByAstEntity.get(astSymbol.rootAstSymbol);
555-
if (entity && entity.exported) {
556-
// We also don't report errors for the default export of an entry point, since its doc comment
557-
// isn't easy to obtain from the .d.ts file
558-
if (astSymbol.rootAstSymbol.localName !== '_default') {
559-
560-
this.messageRouter.addAnalyzerIssue(
561-
ExtractorMessageId.MissingReleaseTag,
562-
`"${entity.astEntity.localName}" is exported by the package, but it is missing `
563-
+ `a release tag (@alpha, @beta, @public, or @internal)`,
564-
astSymbol
565-
);
566-
}
567-
}
522+
if (effectiveReleaseTag > maxEffectiveReleaseTag) {
523+
maxEffectiveReleaseTag = effectiveReleaseTag;
568524
}
569-
570-
effectiveReleaseTag = ReleaseTag.Public;
571525
}
572526

573-
symbolMetadata.releaseTag = effectiveReleaseTag;
574-
symbolMetadata.releaseTagSameAsParent = false;
575-
if (parentSymbolMetadata) {
576-
symbolMetadata.releaseTagSameAsParent = symbolMetadata.releaseTag === parentSymbolMetadata.releaseTag;
577-
}
527+
const symbolMetadata: SymbolMetadata = new SymbolMetadata();
528+
symbolMetadata.maxEffectiveReleaseTag = maxEffectiveReleaseTag;
578529

579530
// Update this last when we're sure no exceptions were thrown
580531
astSymbol.metadata = symbolMetadata;
@@ -664,6 +615,41 @@ export class Collector {
664615
}
665616
}
666617
}
618+
619+
// This needs to be set regardless of whether or not a parserContext exists
620+
if (astDeclaration.parent) {
621+
const parentDeclarationMetadata: DeclarationMetadata = this.fetchMetadata(astDeclaration.parent);
622+
declarationMetadata.effectiveReleaseTag = declarationMetadata.declaredReleaseTag === ReleaseTag.None
623+
? parentDeclarationMetadata.effectiveReleaseTag
624+
: declarationMetadata.declaredReleaseTag;
625+
626+
declarationMetadata.releaseTagSameAsParent =
627+
parentDeclarationMetadata.effectiveReleaseTag === declarationMetadata.effectiveReleaseTag;
628+
} else {
629+
declarationMetadata.effectiveReleaseTag = declarationMetadata.declaredReleaseTag;
630+
}
631+
632+
if (declarationMetadata.effectiveReleaseTag === ReleaseTag.None) {
633+
if (!astDeclaration.astSymbol.isExternal) { // for now, don't report errors for external code
634+
// Don't report missing release tags for forgotten exports
635+
const astSymbol: AstSymbol = astDeclaration.astSymbol;
636+
const entity: CollectorEntity | undefined = this._entitiesByAstEntity.get(astSymbol.rootAstSymbol);
637+
if (entity && entity.exported) {
638+
// We also don't report errors for the default export of an entry point, since its doc comment
639+
// isn't easy to obtain from the .d.ts file
640+
if (astSymbol.rootAstSymbol.localName !== '_default') {
641+
this.messageRouter.addAnalyzerIssue(
642+
ExtractorMessageId.MissingReleaseTag,
643+
`"${entity.astEntity.localName}" is exported by the package, but it is missing `
644+
+ `a release tag (@alpha, @beta, @public, or @internal)`,
645+
astSymbol
646+
);
647+
}
648+
}
649+
}
650+
651+
declarationMetadata.effectiveReleaseTag = ReleaseTag.Public;
652+
}
667653
}
668654

669655
private _parseTsdocForAstDeclaration(astDeclaration: AstDeclaration): tsdoc.ParserContext | undefined {

‎apps/api-extractor/src/collector/DeclarationMetadata.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,17 @@ export class DeclarationMetadata {
2020

2121
/**
2222
* This is the release tag that was explicitly specified in the original doc comment, if any.
23-
* Compare with SymbolMetadata.releaseTag, which is the effective release tag, possibly inherited from
24-
* a parent.
2523
*/
2624
public declaredReleaseTag: ReleaseTag = ReleaseTag.None;
2725

26+
/**
27+
* The "effective" release tag is a normalized value that is based on `declaredReleaseTag`,
28+
* but may be inherited from a parent, or corrected if the declared value was somehow invalid.
29+
* When actually trimming .d.ts files or generating docs, API Extractor uses the "effective" value
30+
* instead of the "declared" value.
31+
*/
32+
public effectiveReleaseTag: ReleaseTag = ReleaseTag.None;
33+
2834
// NOTE: In the future, the Collector may infer or error-correct some of these states.
2935
// Generators should rely on these instead of tsdocComment.modifierTagSet.
3036
public isEventProperty: boolean = false;
@@ -37,5 +43,8 @@ export class DeclarationMetadata {
3743
// Assigned by DocCommentEnhancer
3844
public needsDocumentation: boolean = true;
3945

46+
// If true, then it would be redundant to show this release tag
47+
public releaseTagSameAsParent: boolean = false;
48+
4049
public docCommentEnhancerVisitorState: VisitorState = VisitorState.Unvisited;
4150
}

‎apps/api-extractor/src/collector/SymbolMetadata.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
import { ReleaseTag } from '@microsoft/api-extractor-model';
55

66
export class SymbolMetadata {
7-
public releaseTag: ReleaseTag = ReleaseTag.None;
8-
9-
// If true, then it would be redundant to show this release tag
10-
public releaseTagSameAsParent: boolean = false;
7+
// For all declarations associated with this symbol, this is the
8+
// `DeclarationMetadata.effectiveReleaseTag` value that is most public.
9+
public maxEffectiveReleaseTag: ReleaseTag = ReleaseTag.None;
1110
}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import { AedocDefinitions, ReleaseTag } from '@microsoft/api-extractor-model';
1212
import { ExtractorMessageId } from '../api/ExtractorMessageId';
1313
import { VisitorState } from '../collector/VisitorState';
1414
import { ResolverFailure } from '../analyzer/AstReferenceResolver';
15-
import { SymbolMetadata } from '../collector/SymbolMetadata';
1615

1716
export class DocCommentEnhancer {
1817
private readonly _collector: Collector;
@@ -93,9 +92,8 @@ export class DocCommentEnhancer {
9392
]);
9493
}
9594

96-
const symbolMetadata: SymbolMetadata = this._collector.fetchMetadata(astDeclaration.astSymbol);
97-
98-
if (symbolMetadata.releaseTag === ReleaseTag.Internal) {
95+
const declarationMetadata: DeclarationMetadata = this._collector.fetchMetadata(astDeclaration);
96+
if (declarationMetadata.effectiveReleaseTag === ReleaseTag.Internal) {
9997
// If the constructor is marked as internal, then add a boilerplate notice for the containing class
10098
const classMetadata: DeclarationMetadata = this._collector.fetchMetadata(classDeclaration);
10199

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

Lines changed: 121 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as ts from 'typescript';
77
import { Collector } from '../collector/Collector';
88
import { AstSymbol } from '../analyzer/AstSymbol';
99
import { AstDeclaration } from '../analyzer/AstDeclaration';
10+
import { DeclarationMetadata } from '../collector/DeclarationMetadata';
1011
import { SymbolMetadata } from '../collector/SymbolMetadata';
1112
import { CollectorEntity } from '../collector/CollectorEntity';
1213
import { ExtractorMessageId } from '../api/ExtractorMessageId';
@@ -24,18 +25,59 @@ export class ValidationEnhancer {
2425
ValidationEnhancer._checkReferences(collector, astDeclaration, alreadyWarnedSymbols);
2526
});
2627

27-
ValidationEnhancer._checkForInternalUnderscore(collector, entity, entity.astEntity);
28+
const symbolMetadata: SymbolMetadata = collector.fetchMetadata(entity.astEntity);
29+
ValidationEnhancer._checkForInternalUnderscore(collector, entity, entity.astEntity, symbolMetadata);
30+
ValidationEnhancer._checkForInconsistentReleaseTags(collector, entity.astEntity, symbolMetadata);
2831
}
2932
}
3033
}
3134
}
3235

33-
private static _checkForInternalUnderscore(collector: Collector, collectorEntity: CollectorEntity,
34-
astSymbol: AstSymbol): void {
36+
private static _checkForInternalUnderscore(
37+
collector: Collector,
38+
collectorEntity: CollectorEntity,
39+
astSymbol: AstSymbol,
40+
symbolMetadata: SymbolMetadata
41+
): void {
3542

36-
const astSymbolMetadata: SymbolMetadata = collector.fetchMetadata(astSymbol);
43+
let needsUnderscore: boolean = false;
3744

38-
if (astSymbolMetadata.releaseTag === ReleaseTag.Internal && !astSymbolMetadata.releaseTagSameAsParent) {
45+
if (symbolMetadata.maxEffectiveReleaseTag === ReleaseTag.Internal) {
46+
if (!astSymbol.parentAstSymbol) {
47+
// If it's marked as @internal and has no parent, then it needs and underscore.
48+
// We use maxEffectiveReleaseTag because a merged declaration would NOT need an underscore in a case like this:
49+
//
50+
// /** @public */
51+
// export enum X { }
52+
//
53+
// /** @internal */
54+
// export namespace X { }
55+
//
56+
// (The above normally reports an error "ae-different-release-tags", but that may be suppressed.)
57+
needsUnderscore = true;
58+
} else {
59+
// If it's marked as @internal and the parent isn't obviously already @internal, then it needs an underscore.
60+
//
61+
// For example, we WOULD need an underscore for a merged declaration like this:
62+
//
63+
// /** @internal */
64+
// export namespace X {
65+
// export interface _Y { }
66+
// }
67+
//
68+
// /** @public */
69+
// export class X {
70+
// /** @internal */
71+
// public static _Y(): void { } // <==== different from parent
72+
// }
73+
const parentSymbolMetadata: SymbolMetadata = collector.fetchMetadata(astSymbol);
74+
if (parentSymbolMetadata.maxEffectiveReleaseTag > ReleaseTag.Internal) {
75+
needsUnderscore = true;
76+
}
77+
}
78+
}
79+
80+
if (needsUnderscore) {
3981
for (const exportName of collectorEntity.exportNames) {
4082
if (exportName[0] !== '_') {
4183
collector.messageRouter.addAnalyzerIssue(
@@ -48,14 +90,80 @@ export class ValidationEnhancer {
4890
}
4991
}
5092
}
51-
5293
}
5394

54-
private static _checkReferences(collector: Collector, astDeclaration: AstDeclaration,
55-
alreadyWarnedSymbols: Set<AstSymbol>): void {
95+
private static _checkForInconsistentReleaseTags(
96+
collector: Collector,
97+
astSymbol: AstSymbol,
98+
symbolMetadata: SymbolMetadata
99+
): void {
100+
if (astSymbol.isExternal) {
101+
// For now, don't report errors for external code. If the developer cares about it, they should run
102+
// API Extractor separately on the external project
103+
return;
104+
}
105+
106+
// Normally we will expect all release tags to be the same. Arbitrarily we choose the maxEffectiveReleaseTag
107+
// as the thing they should all match.
108+
const expectedEffectiveReleaseTag: ReleaseTag = symbolMetadata.maxEffectiveReleaseTag;
109+
110+
// This is set to true if we find a declaration whose release tag is different from expectedEffectiveReleaseTag
111+
let mixedReleaseTags: boolean = false;
112+
113+
// This is set to false if we find a declaration that is not a function/method overload
114+
let onlyFunctionOverloads: boolean = true;
115+
116+
// This is set to true if we find a declaration that is @internal
117+
let anyInternalReleaseTags: boolean = false;
118+
119+
for (const astDeclaration of astSymbol.astDeclarations) {
120+
const declarationMetadata: DeclarationMetadata = collector.fetchMetadata(astDeclaration);
121+
const effectiveReleaseTag: ReleaseTag = declarationMetadata.effectiveReleaseTag;
122+
123+
switch (astDeclaration.declaration.kind) {
124+
case ts.SyntaxKind.FunctionDeclaration:
125+
case ts.SyntaxKind.MethodDeclaration:
126+
break;
127+
default:
128+
onlyFunctionOverloads = false;
129+
}
130+
131+
if (effectiveReleaseTag !== expectedEffectiveReleaseTag) {
132+
mixedReleaseTags = true;
133+
}
134+
135+
if (effectiveReleaseTag === ReleaseTag.Internal) {
136+
anyInternalReleaseTags = true;
137+
}
138+
}
139+
140+
if (mixedReleaseTags) {
141+
if (!onlyFunctionOverloads) {
142+
collector.messageRouter.addAnalyzerIssue(
143+
ExtractorMessageId.DifferentReleaseTags,
144+
'This symbol has another declaration with a different release tag',
145+
astSymbol
146+
);
147+
}
148+
149+
if (anyInternalReleaseTags) {
150+
collector.messageRouter.addAnalyzerIssue(
151+
ExtractorMessageId.InternalMixedReleaseTag,
152+
`Mixed release tags are not allowed for "${astSymbol.localName}" because one of its declarations` +
153+
` is marked as @internal`,
154+
astSymbol
155+
);
156+
}
157+
}
158+
}
56159

57-
const astSymbolMetadata: SymbolMetadata = collector.fetchMetadata(astDeclaration.astSymbol);
58-
const astSymbolReleaseTag: ReleaseTag = astSymbolMetadata.releaseTag;
160+
private static _checkReferences(
161+
collector: Collector,
162+
astDeclaration: AstDeclaration,
163+
alreadyWarnedSymbols: Set<AstSymbol>
164+
): void {
165+
const declarationMetadata: DeclarationMetadata = collector.fetchMetadata(astDeclaration);
166+
const declarationReleaseTag: ReleaseTag = declarationMetadata.effectiveReleaseTag;
59167

60168
for (const referencedEntity of astDeclaration.referencedAstEntities) {
61169

@@ -71,12 +179,12 @@ export class ValidationEnhancer {
71179

72180
if (collectorEntity && collectorEntity.exported) {
73181
const referencedMetadata: SymbolMetadata = collector.fetchMetadata(referencedEntity);
74-
const referencedReleaseTag: ReleaseTag = referencedMetadata.releaseTag;
182+
const referencedReleaseTag: ReleaseTag = referencedMetadata.maxEffectiveReleaseTag;
75183

76-
if (ReleaseTag.compare(astSymbolReleaseTag, referencedReleaseTag) > 0) {
184+
if (ReleaseTag.compare(declarationReleaseTag, referencedReleaseTag) > 0) {
77185
collector.messageRouter.addAnalyzerIssue(ExtractorMessageId.IncompatibleReleaseTags,
78186
`The symbol "${astDeclaration.astSymbol.localName}"`
79-
+ ` is marked as ${ReleaseTag.getTagName(astSymbolReleaseTag)},`
187+
+ ` is marked as ${ReleaseTag.getTagName(declarationReleaseTag)},`
80188
+ ` but its signature references "${referencedEntity.localName}"`
81189
+ ` which is marked as ${ReleaseTag.getTagName(referencedReleaseTag)}`,
82190
astDeclaration);

0 commit comments

Comments
 (0)
X Tutup