Merge pull request #1169 from Microsoft/octogonz/ae-review-exports

[api-extractor] Improve the API review files to include import and export signatures
microsoft · Mar 20, 2019 · e444346 · e444346
2 parents 0fccc1a + 83fba87
commit e444346
Show file tree

Hide file tree

Showing 55 changed files with 472 additions and 280 deletions.
diff --git a/apps/api-extractor/build-tests.cmd b/apps/api-extractor/build-tests.cmd
@@ -1,3 +1,3 @@
 @ECHO OFF
 @SETLOCAL
-rush build -t api-extractor-test-01 -t api-extractor-test-02 -t api-extractor-test-03 -t api-extractor-test-04 -t api-extractor-test-05 -t api-extractor-test-06
+rush build -t api-extractor-lib1-test -t api-extractor-lib2-test -t api-extractor-lib3-test -t api-extractor-scenarios -t api-extractor-test-01 -t api-extractor-test-02 -t api-extractor-test-03 -t api-extractor-test-04 -t api-documenter-test
diff --git a/apps/api-extractor/src/generators/DtsEmitHelpers.ts b/apps/api-extractor/src/generators/DtsEmitHelpers.ts
@@ -0,0 +1,57 @@
+// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
+// See LICENSE in the project root for license information.
+
+import * as ts from 'typescript';
+
+import { InternalError } from '@microsoft/node-core-library';
+import { CollectorEntity } from '../collector/CollectorEntity';
+import { AstImport, AstImportKind } from '../analyzer/AstImport';
+import { StringWriter } from './StringWriter';
+import { Collector } from '../collector/Collector';
+
+/**
+ * Some common code shared between DtsRollupGenerator and ReviewFileGenerator.
+ */
+export class DtsEmitHelpers {
+  public static emitImport(stringWriter: StringWriter, collectorEntity: CollectorEntity, astImport: AstImport): void {
+    switch (astImport.importKind) {
+      case AstImportKind.NamedImport:
+        if (collectorEntity.nameForEmit !== astImport.exportName) {
+          stringWriter.write(`import { ${astImport.exportName} as ${collectorEntity.nameForEmit} }`);
+        } else {
+          stringWriter.write(`import { ${astImport.exportName} }`);
+        }
+        stringWriter.writeLine(` from '${astImport.modulePath}';`);
+        break;
+      case AstImportKind.StarImport:
+        stringWriter.writeLine(`import * as ${collectorEntity.nameForEmit} from '${astImport.modulePath}';`);
+        break;
+      case AstImportKind.EqualsImport:
+        stringWriter.writeLine(`import ${collectorEntity.nameForEmit} = require('${astImport.modulePath}');`);
+        break;
+      default:
+        throw new InternalError('Unimplemented AstImportKind');
+    }
+  }
+
+  public static emitNamedExport(stringWriter: StringWriter, exportName: string,
+    collectorEntity: CollectorEntity): void {
+
+    if (exportName === ts.InternalSymbolName.Default) {
+      stringWriter.writeLine(`export default ${collectorEntity.nameForEmit};`);
+    } else if (collectorEntity.nameForEmit !== exportName) {
+      stringWriter.writeLine(`export { ${collectorEntity.nameForEmit} as ${exportName} }`);
+    } else {
+      stringWriter.writeLine(`export { ${exportName} }`);
+    }
+  }
+
+  public static emitStarExports(stringWriter: StringWriter, collector: Collector): void {
+    if (collector.starExportedExternalModulePaths.length > 0) {
+      stringWriter.writeLine();
+      for (const starExportedExternalModulePath of collector.starExportedExternalModulePaths) {
+        stringWriter.writeLine(`export * from "${starExportedExternalModulePath}";`);
+      }
+    }
+  }
+}
diff --git a/apps/api-extractor/src/generators/DtsRollupGenerator.ts b/apps/api-extractor/src/generators/DtsRollupGenerator.ts
@@ -10,13 +10,14 @@ import { ReleaseTag } from '@microsoft/api-extractor-model';
 import { Collector } from '../collector/Collector';
 import { TypeScriptHelpers } from '../analyzer/TypeScriptHelpers';
 import { Span, SpanModification } from '../analyzer/Span';
-import { AstImport, AstImportKind } from '../analyzer/AstImport';
+import { AstImport } from '../analyzer/AstImport';
 import { CollectorEntity } from '../collector/CollectorEntity';
 import { AstDeclaration } from '../analyzer/AstDeclaration';
 import { DeclarationMetadata } from '../collector/DeclarationMetadata';
 import { AstSymbol } from '../analyzer/AstSymbol';
 import { SymbolMetadata } from '../collector/SymbolMetadata';
 import { StringWriter } from './StringWriter';
+import { DtsEmitHelpers } from './DtsEmitHelpers';
 
 /**
  * Used with DtsRollupGenerator.writeTypingsFile()
@@ -90,25 +91,7 @@ export class DtsRollupGenerator {
         const releaseTag: ReleaseTag = symbolMetadata ? symbolMetadata.releaseTag : ReleaseTag.None;
 
         if (this._shouldIncludeReleaseTag(releaseTag, dtsKind)) {
-          switch (astImport.importKind) {
-            case AstImportKind.NamedImport:
-              if (entity.nameForEmit !== astImport.exportName) {
-                stringWriter.write(`import { ${astImport.exportName} as ${entity.nameForEmit} }`);
-              } else {
-                stringWriter.write(`import { ${astImport.exportName} }`);
-              }
-              stringWriter.writeLine(` from '${astImport.modulePath}';`);
-              break;
-            case AstImportKind.StarImport:
-              stringWriter.writeLine(`import * as ${entity.nameForEmit} from '${astImport.modulePath}';`);
-              break;
-            case AstImportKind.EqualsImport:
-              stringWriter.writeLine(`import ${entity.nameForEmit} = require('${astImport.modulePath}');`);
-              break;
-            default:
-              throw new InternalError('Unimplemented AstImportKind');
-          }
-
+          DtsEmitHelpers.emitImport(stringWriter, entity, astImport);
         }
       }
     }
@@ -139,23 +122,12 @@ export class DtsRollupGenerator {
 
       if (!entity.shouldInlineExport) {
         for (const exportName of entity.exportNames) {
-          if (exportName === ts.InternalSymbolName.Default) {
-            stringWriter.writeLine(`export default ${entity.nameForEmit};`);
-          } else if (entity.nameForEmit !== exportName) {
-            stringWriter.writeLine(`export { ${entity.nameForEmit} as ${exportName} }`);
-          } else {
-            stringWriter.writeLine(`export { ${exportName} }`);
-          }
+          DtsEmitHelpers.emitNamedExport(stringWriter, exportName, entity);
         }
       }
     }
 
-    if (collector.starExportedExternalModulePaths.length > 0) {
-      stringWriter.writeLine();
-      for (const starExportedExternalModulePath of collector.starExportedExternalModulePaths) {
-        stringWriter.writeLine(`export * from "${starExportedExternalModulePath}";`);
-      }
-    }
+    DtsEmitHelpers.emitStarExports(stringWriter, collector);
 
     // Emit "export { }" which is a special directive that prevents consumers from importing declarations
     // that don't have an explicit "export" modifier.

diff --git a/apps/api-extractor/src/generators/ReviewFileGenerator.ts b/apps/api-extractor/src/generators/ReviewFileGenerator.ts
@@ -16,6 +16,7 @@ import { AstImport } from '../analyzer/AstImport';
 import { AstSymbol } from '../analyzer/AstSymbol';
 import { ExtractorMessage } from '../api/ExtractorMessage';
 import { StringWriter } from './StringWriter';
+import { DtsEmitHelpers } from './DtsEmitHelpers';
 
 export class ReviewFileGenerator {
   /**
@@ -45,6 +46,20 @@ export class ReviewFileGenerator {
     // Write the opening delimiter for the Markdown code fence
     stringWriter.writeLine('```ts\n');
 
+    // Emit the imports
+    let importsEmitted: boolean = false;
+    for (const entity of collector.entities) {
+      if (entity.astEntity instanceof AstImport) {
+        DtsEmitHelpers.emitImport(stringWriter, entity, entity.astEntity);
+        importsEmitted = true;
+      }
+    }
+
+    if (importsEmitted) {
+      stringWriter.writeLine();
+    }
+
+    // Emit the regular declarations
     for (const entity of collector.entities) {
       if (entity.exported) {
         if (entity.astEntity instanceof AstSymbol) {
@@ -58,30 +73,18 @@ export class ReviewFileGenerator {
             span.writeModifiedText(stringWriter.stringBuilder);
             stringWriter.writeLine('\n');
           }
-        } else {
-          // This definition is reexported from another package, so write it as an "export" line
-          // In general, we don't report on external packages; if that's important we assume API Extractor
-          // would be enabled for the upstream project.  But see GitHub issue #896 for a possible exception.
-          const astImport: AstImport = entity.astEntity;
-
-          if (astImport.exportName === '*') {
-            stringWriter.write(`export * as ${entity.nameForEmit}`);
-          } else if (entity.nameForEmit !== astImport.exportName) {
-            stringWriter.write(`export { ${astImport.exportName} as ${entity.nameForEmit} }`);
-          } else {
-            stringWriter.write(`export { ${astImport.exportName} }`);
+        }
+
+        for (const exportName of entity.exportNames) {
+          if (!entity.shouldInlineExport) {
+            DtsEmitHelpers.emitNamedExport(stringWriter, exportName, entity);
+            stringWriter.writeLine();
           }
-          stringWriter.writeLine(` from '${astImport.modulePath}';`);
         }
       }
     }
 
-    if (collector.starExportedExternalModulePaths.length > 0) {
-      stringWriter.writeLine();
-      for (const starExportedExternalModulePath of collector.starExportedExternalModulePaths) {
-        stringWriter.writeLine(`export * from "${starExportedExternalModulePath}";`);
-      }
-    }
+    DtsEmitHelpers.emitStarExports(stringWriter, collector);
 
     // Write the unassociated warnings at the bottom of the file
     const unassociatedMessages: ExtractorMessage[] = collector.messageRouter
@@ -120,6 +123,8 @@ export class ReviewFileGenerator {
       return;
     }
 
+    const previousSpan: Span | undefined = span.previousSibling;
+
     let recurseChildren: boolean = true;
     let sortChildren: boolean = false;
 
@@ -132,9 +137,35 @@ export class ReviewFileGenerator {
 
       case ts.SyntaxKind.ExportKeyword:
       case ts.SyntaxKind.DefaultKeyword:
+      case ts.SyntaxKind.DeclareKeyword:
+        // Delete any explicit "export" or "declare" keywords -- we will re-add them below
         span.modification.skipAll();
         break;
 
+      case ts.SyntaxKind.InterfaceKeyword:
+      case ts.SyntaxKind.ClassKeyword:
+      case ts.SyntaxKind.EnumKeyword:
+      case ts.SyntaxKind.NamespaceKeyword:
+      case ts.SyntaxKind.ModuleKeyword:
+      case ts.SyntaxKind.TypeKeyword:
+      case ts.SyntaxKind.FunctionKeyword:
+        // Replace the stuff we possibly deleted above
+        let replacedModifiers: string = '';
+
+        if (entity.shouldInlineExport) {
+          replacedModifiers = 'export ' + replacedModifiers;
+        }
+
+        if (previousSpan && previousSpan.kind === ts.SyntaxKind.SyntaxList) {
+          // If there is a previous span of type SyntaxList, then apply it before any other modifiers
+          // (e.g. "abstract") that appear there.
+          previousSpan.modification.prefix = replacedModifiers + previousSpan.modification.prefix;
+        } else {
+          // Otherwise just stick it in front of this span
+          span.modification.prefix = replacedModifiers + span.modification.prefix;
+        }
+        break;
+
       case ts.SyntaxKind.SyntaxList:
         if (span.parent) {
           if (AstDeclaration.isSupportedSyntaxKind(span.parent.kind)) {
@@ -165,9 +196,12 @@ export class ReviewFileGenerator {
           }
           const listPrefix: string = list.getSourceFile().text
             .substring(list.getStart(), list.declarations[0].getStart());
-          span.modification.prefix = 'declare ' + listPrefix + span.modification.prefix;
-
+          span.modification.prefix = listPrefix + span.modification.prefix;
           span.modification.suffix = ';';
+
+          if (entity.shouldInlineExport) {
+            span.modification.prefix = 'export ' + span.modification.prefix;
+          }
         }
         break;
 

diff --git a/build-tests/api-documenter-test/etc/api-documenter-test.api.md b/build-tests/api-documenter-test/etc/api-documenter-test.api.md
@@ -5,14 +5,14 @@
 ```ts
 
 // @public
-declare const constVariable: number;
+export const constVariable: number;
 
 // @public
-declare class DocBaseClass {
+export class DocBaseClass {
 }
 
 // @public
-declare class DocClass1 extends DocBaseClass implements IDocInterface1, IDocInterface2 {
+export class DocClass1 extends DocBaseClass implements IDocInterface1, IDocInterface2 {
     constructor(name: string);
     // @deprecated (undocumented)
     deprecatedExample(): void;
@@ -29,46 +29,46 @@ declare class DocClass1 extends DocBaseClass implements IDocInterface1, IDocInte
 }
 
 // @public
-declare enum DocEnum {
+export enum DocEnum {
     One = 1,
     Two = 2,
     Zero = 0
 }
 
 // @public
-declare type ExampleTypeAlias = Promise<boolean>;
+export type ExampleTypeAlias = Promise<boolean>;
 
 // @public
-declare function globalFunction(x: number): number;
+export function globalFunction(x: number): number;
 
 // @public (undocumented)
-interface IDocInterface1 {
+export interface IDocInterface1 {
     regularProperty: SystemEvent;
 }
 
 // @public (undocumented)
-interface IDocInterface2 extends IDocInterface1 {
+export interface IDocInterface2 extends IDocInterface1 {
     // @deprecated (undocumented)
     deprecatedExample(): void;
 }
 
 // @public
-interface IDocInterface3 {
+export interface IDocInterface3 {
     (x: number): number;
     [x: string]: string;
     new (): IDocInterface1;
 }
 
 // @public
-declare namespace OuterNamespace {
-    namespace InnerNamespace {
-        function nestedFunction(x: number): number;
+export namespace OuterNamespace {
+    export namespace InnerNamespace {
+        export function nestedFunction(x: number): number;
     }
     let nestedVariable: boolean;
 }
 
 // @public
-declare class SystemEvent {
+export class SystemEvent {
     addHandler(handler: () => void): void;
 }
 

diff --git a/build-tests/api-documenter-test/tsconfig.json b/build-tests/api-documenter-test/tsconfig.json
@@ -5,6 +5,7 @@
     "module": "commonjs",
     "declaration": true,
     "sourceMap": true,
+    "declarationMap": true,
     "experimentalDecorators": true,
     "strictNullChecks": true,
     "types": [

diff --git a/build-tests/api-extractor-lib1-test/etc/api-extractor-lib1-test.api.md b/build-tests/api-extractor-lib1-test/etc/api-extractor-lib1-test.api.md
@@ -7,11 +7,11 @@
 // Warning: (ae-forgotten-export) The symbol "Lib1ForgottenExport" needs to be exported by the entry point index.d.ts
 // 
 // @public (undocumented)
-declare class Lib1Class extends Lib1ForgottenExport {
+export class Lib1Class extends Lib1ForgottenExport {
 }
 
 // @alpha (undocumented)
-interface Lib1Interface {
+export interface Lib1Interface {
 }
 
 

diff --git a/build-tests/api-extractor-lib1-test/tsconfig.json b/build-tests/api-extractor-lib1-test/tsconfig.json
@@ -5,6 +5,7 @@
     "module": "commonjs",
     "declaration": true,
     "sourceMap": true,
+    "declarationMap": true,
     "experimentalDecorators": true,
     "strictNullChecks": true,
     "types": [

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
@@ -5,11 +5,11 @@
 ```ts
 
 // @public (undocumented)
-declare class Lib2Class {
+export class Lib2Class {
 }
 
 // @alpha (undocumented)
-interface Lib2Interface {
+export interface Lib2Interface {
 }
 
 

diff --git a/build-tests/api-extractor-lib2-test/tsconfig.json b/build-tests/api-extractor-lib2-test/tsconfig.json
@@ -5,6 +5,7 @@
     "module": "commonjs",
     "declaration": true,
     "sourceMap": true,
+    "declarationMap": true,
     "experimentalDecorators": true,
     "strictNullChecks": true,
     "types": [

diff --git a/build-tests/api-extractor-lib3-test/etc/api-extractor-lib3-test.api.md b/build-tests/api-extractor-lib3-test/etc/api-extractor-lib3-test.api.md
@@ -4,6 +4,9 @@
 
 ```ts
 
-export { Lib1Class } from 'api-extractor-lib1-test';
+import { Lib1Class } from 'api-extractor-lib1-test';
+
+export { Lib1Class }
+
 
 ```