[api-extractor] Allow separate release tags for overloaded functions and methods

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

@@ -47,6 +47,11 @@ export const enum ExtractorMessageId {
    */
   InternalMissingUnderscore = 'ae-internal-missing-underscore',
 
+  /**
+   * "Mixed release tags are not allowed for overload ___ when one is marked as `@internal`."
+   */
+  InternalMixedReleaseTag = 'ae-internal-mixed-release-tag',
+
   /**
    * "The `@preapproved` tag cannot be applied to ___ because it is not a supported declaration type."
    */
@@ -91,6 +96,7 @@ export const allExtractorMessageIds: Set<string> = new Set<string>([
   'ae-misplaced-package-tag',
   'ae-forgotten-export',
   'ae-internal-missing-underscore',
+  'ae-internal-mixed-release-tag',
   'ae-preapproved-unsupported-type',
   'ae-preapproved-bad-release-tag',
   'ae-unresolved-inheritdoc-reference',

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

@@ -511,70 +511,42 @@ export class Collector {
       this._calculateMetadataForDeclaration(astDeclaration);
     }
 
-    // We know we solved parentAstSymbol.metadata above
-    const parentSymbolMetadata: SymbolMetadata | undefined = astSymbol.parentAstSymbol
-      ? astSymbol.parentAstSymbol.metadata as SymbolMetadata : undefined;
-
-    const symbolMetadata: SymbolMetadata = new SymbolMetadata();
-
     // Do any of the declarations have a release tag?
-    let effectiveReleaseTag: ReleaseTag = ReleaseTag.None;
+    let maxEffectiveReleaseTag: ReleaseTag = ReleaseTag.None;
 
     for (const astDeclaration of astSymbol.astDeclarations) {
       // We know we solved this above
       const declarationMetadata: DeclarationMetadata = astDeclaration.metadata as DeclarationMetadata;
+      const effectiveReleaseTag: ReleaseTag = declarationMetadata.effectiveReleaseTag;
 
-      const declaredReleaseTag: ReleaseTag = declarationMetadata.declaredReleaseTag;
-
-      if (declaredReleaseTag !== ReleaseTag.None) {
-        if (effectiveReleaseTag !== ReleaseTag.None && effectiveReleaseTag !== declaredReleaseTag) {
+      if (effectiveReleaseTag !== ReleaseTag.None) {
+        if (maxEffectiveReleaseTag !== ReleaseTag.None && maxEffectiveReleaseTag !== effectiveReleaseTag) {
           if (!astSymbol.isExternal) { // for now, don't report errors for external code
-            this.messageRouter.addAnalyzerIssue(
-              ExtractorMessageId.DifferentReleaseTags,
-              'This symbol has another declaration with a different release tag',
-              astDeclaration
-            );
+            switch (astDeclaration.declaration.kind) {
+              case ts.SyntaxKind.FunctionDeclaration:
+              case ts.SyntaxKind.MethodDeclaration:
+                // For function and method overloads, take the highest release from multiple declarations
+                // TODO: Expand this out to more kinds
+                if (maxEffectiveReleaseTag < effectiveReleaseTag) {
+                  maxEffectiveReleaseTag = effectiveReleaseTag;
+                }
+                break;
+              default:
+                this.messageRouter.addAnalyzerIssue(
+                  ExtractorMessageId.DifferentReleaseTags,
+                  'This symbol has another declaration with a different release tag',
+                  astDeclaration
+                );
+            }
           }
         } else {
-          effectiveReleaseTag = declaredReleaseTag;
-        }
-      }
-    }
-
-    // If this declaration doesn't have a release tag, then inherit it from the parent
-    if (effectiveReleaseTag === ReleaseTag.None && astSymbol.parentAstSymbol) {
-      if (parentSymbolMetadata) {
-        effectiveReleaseTag = parentSymbolMetadata.releaseTag;
-      }
-    }
-
-    if (effectiveReleaseTag === ReleaseTag.None) {
-      if (!astSymbol.isExternal) { // for now, don't report errors for external code
-        // Don't report missing release tags for forgotten exports
-        const entity: CollectorEntity | undefined = this._entitiesByAstEntity.get(astSymbol.rootAstSymbol);
-        if (entity && entity.exported) {
-          // We also don't report errors for the default export of an entry point, since its doc comment
-          // isn't easy to obtain from the .d.ts file
-          if (astSymbol.rootAstSymbol.localName !== '_default') {
-
-            this.messageRouter.addAnalyzerIssue(
-              ExtractorMessageId.MissingReleaseTag,
-              `"${entity.astEntity.localName}" is exported by the package, but it is missing `
-              + `a release tag (@alpha, @beta, @public, or @internal)`,
-              astSymbol
-            );
-          }
+          maxEffectiveReleaseTag = effectiveReleaseTag;
         }
       }
-
-      effectiveReleaseTag = ReleaseTag.Public;
     }
 
-    symbolMetadata.releaseTag = effectiveReleaseTag;
-    symbolMetadata.releaseTagSameAsParent = false;
-    if (parentSymbolMetadata) {
-      symbolMetadata.releaseTagSameAsParent = symbolMetadata.releaseTag === parentSymbolMetadata.releaseTag;
-    }
+    const symbolMetadata: SymbolMetadata = new SymbolMetadata();
+    symbolMetadata.maxEffectiveReleaseTag = maxEffectiveReleaseTag;
 
     // Update this last when we're sure no exceptions were thrown
     astSymbol.metadata = symbolMetadata;
@@ -664,6 +636,41 @@ export class Collector {
         }
       }
     }
+
+    // This needs to be set regardless of whether or not a parserContext exists
+    if (astDeclaration.parent) {
+      const parentDeclarationMetadata: DeclarationMetadata = this.fetchMetadata(astDeclaration.parent);
+      declarationMetadata.effectiveReleaseTag = declarationMetadata.declaredReleaseTag === ReleaseTag.None
+        ? parentDeclarationMetadata.effectiveReleaseTag
+        : declarationMetadata.declaredReleaseTag;
+
+      declarationMetadata.releaseTagSameAsParent =
+        parentDeclarationMetadata.effectiveReleaseTag === declarationMetadata.effectiveReleaseTag;
+    } else {
+      declarationMetadata.effectiveReleaseTag = declarationMetadata.declaredReleaseTag;
+    }
+
+    if (declarationMetadata.effectiveReleaseTag === ReleaseTag.None) {
+      if (!astDeclaration.astSymbol.isExternal) { // for now, don't report errors for external code
+        // Don't report missing release tags for forgotten exports
+        const astSymbol: AstSymbol = astDeclaration.astSymbol;
+        const entity: CollectorEntity | undefined = this._entitiesByAstEntity.get(astSymbol.rootAstSymbol);
+        if (entity && entity.exported) {
+          // We also don't report errors for the default export of an entry point, since its doc comment
+          // isn't easy to obtain from the .d.ts file
+          if (astSymbol.rootAstSymbol.localName !== '_default') {
+            this.messageRouter.addAnalyzerIssue(
+              ExtractorMessageId.MissingReleaseTag,
+              `"${entity.astEntity.localName}" is exported by the package, but it is missing `
+              + `a release tag (@alpha, @beta, @public, or @internal)`,
+              astSymbol
+            );
+          }
+        }
+      }
+
+      declarationMetadata.effectiveReleaseTag = ReleaseTag.Public;
+    }
   }
 
   private _parseTsdocForAstDeclaration(astDeclaration: AstDeclaration): tsdoc.ParserContext | undefined {

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

@@ -20,11 +20,17 @@ export class DeclarationMetadata {
 
   /**
    * This is the release tag that was explicitly specified in the original doc comment, if any.
-   * Compare with SymbolMetadata.releaseTag, which is the effective release tag, possibly inherited from
-   * a parent.
    */
   public declaredReleaseTag: ReleaseTag = ReleaseTag.None;
 
+  /**
+   * The "effective" release tag is a normalized value that is based on `declaredReleaseTag`,
+   * but may be inherited from a parent, or corrected if the declared value was somehow invalid.
+   * When actually trimming .d.ts files or generating docs, API Extractor uses the "effective" value
+   * instead of the "declared" value.
+   */
+  public effectiveReleaseTag: ReleaseTag = ReleaseTag.None;
+
   // NOTE: In the future, the Collector may infer or error-correct some of these states.
   // Generators should rely on these instead of tsdocComment.modifierTagSet.
   public isEventProperty: boolean = false;
@@ -37,5 +43,8 @@ export class DeclarationMetadata {
   // Assigned by DocCommentEnhancer
   public needsDocumentation: boolean = true;
 
+  // If true, then it would be redundant to show this release tag
+  public releaseTagSameAsParent: boolean = false;
+
   public docCommentEnhancerVisitorState: VisitorState = VisitorState.Unvisited;
 }

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

@@ -4,8 +4,5 @@
 import { ReleaseTag } from '@microsoft/api-extractor-model';
 
 export class SymbolMetadata {
-  public releaseTag: ReleaseTag = ReleaseTag.None;
-
-  // If true, then it would be redundant to show this release tag
-  public releaseTagSameAsParent: boolean = false;
+  public maxEffectiveReleaseTag: ReleaseTag = ReleaseTag.None;
 }

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

@@ -12,7 +12,6 @@ import { AedocDefinitions, ReleaseTag } from '@microsoft/api-extractor-model';
 import { ExtractorMessageId } from '../api/ExtractorMessageId';
 import { VisitorState } from '../collector/VisitorState';
 import { ResolverFailure } from '../analyzer/AstReferenceResolver';
-import { SymbolMetadata } from '../collector/SymbolMetadata';
 
 export class DocCommentEnhancer {
   private readonly _collector: Collector;
@@ -93,9 +92,8 @@ export class DocCommentEnhancer {
         ]);
       }
 
-      const symbolMetadata: SymbolMetadata = this._collector.fetchMetadata(astDeclaration.astSymbol);
-
-      if (symbolMetadata.releaseTag === ReleaseTag.Internal) {
+      const declarationMetadata: DeclarationMetadata = this._collector.fetchMetadata(astDeclaration);
+      if (declarationMetadata.effectiveReleaseTag === ReleaseTag.Internal) {
         // If the constructor is marked as internal, then add a boilerplate notice for the containing class
         const classMetadata: DeclarationMetadata = this._collector.fetchMetadata(classDeclaration);
 

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

@@ -7,6 +7,7 @@ import * as ts from 'typescript';
 import { Collector } from '../collector/Collector';
 import { AstSymbol } from '../analyzer/AstSymbol';
 import { AstDeclaration } from '../analyzer/AstDeclaration';
+import { DeclarationMetadata } from '../collector/DeclarationMetadata';
 import { SymbolMetadata } from '../collector/SymbolMetadata';
 import { CollectorEntity } from '../collector/CollectorEntity';
 import { ExtractorMessageId } from '../api/ExtractorMessageId';
@@ -30,32 +31,56 @@ export class ValidationEnhancer {
     }
   }
 
-  private static _checkForInternalUnderscore(collector: Collector, collectorEntity: CollectorEntity,
-    astSymbol: AstSymbol): void {
-
-    const astSymbolMetadata: SymbolMetadata = collector.fetchMetadata(astSymbol);
-
-    if (astSymbolMetadata.releaseTag === ReleaseTag.Internal && !astSymbolMetadata.releaseTagSameAsParent) {
-      for (const exportName of collectorEntity.exportNames) {
-        if (exportName[0] !== '_') {
-          collector.messageRouter.addAnalyzerIssue(
-            ExtractorMessageId.InternalMissingUnderscore,
-            `The name "${exportName}" should be prefixed with an underscore`
-            + ` because the declaration is marked as @internal`,
-            astSymbol,
-            { exportName }
-          );
+  private static _checkForInternalUnderscore(
+    collector: Collector,
+    collectorEntity: CollectorEntity,
+    astSymbol: AstSymbol
+  ): void {
+    let containsInternal: boolean = false;
+    for (let i: number = 0; i < astSymbol.astDeclarations.length; i++) {
+      const astDeclaration: AstDeclaration = astSymbol.astDeclarations[i];
+      const declarationMetadata: DeclarationMetadata = collector.fetchMetadata(astDeclaration);
+
+      if (
+        (containsInternal && declarationMetadata.effectiveReleaseTag !== ReleaseTag.Internal) ||
+        (!containsInternal && declarationMetadata.effectiveReleaseTag === ReleaseTag.Internal && i > 0)
+      ) {
+        const exportName: string = astDeclaration.astSymbol.localName;
+        collector.messageRouter.addAnalyzerIssue(
+          ExtractorMessageId.InternalMixedReleaseTag,
+          `Mixed release tags are not allowed for overload "${exportName}" when one is marked as @internal`,
+          astDeclaration
+        );
+      } else if (declarationMetadata.effectiveReleaseTag === ReleaseTag.Internal) {
+        containsInternal = true;
+      }
+
+      if (
+        declarationMetadata.effectiveReleaseTag === ReleaseTag.Internal &&
+        !declarationMetadata.releaseTagSameAsParent
+      ) {
+        for (const exportName of collectorEntity.exportNames) {
+          if (exportName[0] !== '_') {
+            collector.messageRouter.addAnalyzerIssue(
+              ExtractorMessageId.InternalMissingUnderscore,
+              `The name "${exportName}" should be prefixed with an underscore`
+              + ` because the declaration is marked as @internal`,
+              astSymbol,
+              { exportName }
+            );
+          }
         }
       }
     }
-
   }
 
-  private static _checkReferences(collector: Collector, astDeclaration: AstDeclaration,
-    alreadyWarnedSymbols: Set<AstSymbol>): void {
-
-    const astSymbolMetadata: SymbolMetadata = collector.fetchMetadata(astDeclaration.astSymbol);
-    const astSymbolReleaseTag: ReleaseTag = astSymbolMetadata.releaseTag;
+  private static _checkReferences(
+    collector: Collector,
+    astDeclaration: AstDeclaration,
+    alreadyWarnedSymbols: Set<AstSymbol>
+  ): void {
+    const declarationMetadata: DeclarationMetadata = collector.fetchMetadata(astDeclaration);
+    const declarationReleaseTag: ReleaseTag = declarationMetadata.effectiveReleaseTag;
 
     for (const referencedEntity of astDeclaration.referencedAstEntities) {
 
@@ -71,12 +96,12 @@ export class ValidationEnhancer {
 
           if (collectorEntity && collectorEntity.exported) {
             const referencedMetadata: SymbolMetadata = collector.fetchMetadata(referencedEntity);
-            const referencedReleaseTag: ReleaseTag = referencedMetadata.releaseTag;
+            const referencedReleaseTag: ReleaseTag = referencedMetadata.maxEffectiveReleaseTag;
 
-            if (ReleaseTag.compare(astSymbolReleaseTag, referencedReleaseTag) > 0) {
+            if (ReleaseTag.compare(declarationReleaseTag, referencedReleaseTag) > 0) {
               collector.messageRouter.addAnalyzerIssue(ExtractorMessageId.IncompatibleReleaseTags,
                 `The symbol "${astDeclaration.astSymbol.localName}"`
-                + ` is marked as ${ReleaseTag.getTagName(astSymbolReleaseTag)},`
+                + ` is marked as ${ReleaseTag.getTagName(declarationReleaseTag)},`
                 + ` but its signature references "${referencedEntity.localName}"`
                 + ` which is marked as ${ReleaseTag.getTagName(referencedReleaseTag)}`,
                 astDeclaration);