diff --git a/apps/api-extractor/src/analyzer/AstSymbolTable.ts b/apps/api-extractor/src/analyzer/AstSymbolTable.ts index db450e0b986..310b15130e0 100644 --- a/apps/api-extractor/src/analyzer/AstSymbolTable.ts +++ b/apps/api-extractor/src/analyzer/AstSymbolTable.ts @@ -9,9 +9,9 @@ import { TypeScriptHelpers } from './TypeScriptHelpers'; import { AstSymbol } from './AstSymbol'; import { AstModule, AstModuleExportInfo } from './AstModule'; import { PackageMetadataManager } from './PackageMetadataManager'; -import { ILogger } from '../api/ILogger'; import { ExportAnalyzer } from './ExportAnalyzer'; import { AstImport } from './AstImport'; +import { MessageRouter } from '../collector/MessageRouter'; export type AstEntity = AstSymbol | AstImport; @@ -83,11 +83,11 @@ export class AstSymbolTable { = new Map(); public constructor(program: ts.Program, typeChecker: ts.TypeChecker, packageJsonLookup: PackageJsonLookup, - logger: ILogger) { + messageRouter: MessageRouter) { this._program = program; this._typeChecker = typeChecker; - this._packageMetadataManager = new PackageMetadataManager(packageJsonLookup, logger); + this._packageMetadataManager = new PackageMetadataManager(packageJsonLookup, messageRouter); this._exportAnalyzer = new ExportAnalyzer( this._program, diff --git a/apps/api-extractor/src/analyzer/PackageMetadataManager.ts b/apps/api-extractor/src/analyzer/PackageMetadataManager.ts index 2d73d598fcd..995eaf92906 100644 --- a/apps/api-extractor/src/analyzer/PackageMetadataManager.ts +++ b/apps/api-extractor/src/analyzer/PackageMetadataManager.ts @@ -11,7 +11,8 @@ import { INodePackageJson } from '@microsoft/node-core-library'; import { Extractor } from '../api/Extractor'; -import { ILogger } from '../api/ILogger'; +import { MessageRouter } from '../collector/MessageRouter'; +import { ConsoleMessageId } from '../api/ConsoleMessageId'; /** * Represents analyzed information for a package.json file. @@ -57,7 +58,7 @@ export class PackageMetadataManager { public static tsdocMetadataFilename: string = 'tsdoc-metadata.json'; private readonly _packageJsonLookup: PackageJsonLookup; - private readonly _logger: ILogger; + private readonly _messageRouter: MessageRouter; private readonly _packageMetadataByPackageJsonPath: Map = new Map(); @@ -147,9 +148,9 @@ export class PackageMetadataManager { }); } - public constructor(packageJsonLookup: PackageJsonLookup, logger: ILogger) { + public constructor(packageJsonLookup: PackageJsonLookup, messageRouter: MessageRouter) { this._packageJsonLookup = packageJsonLookup; - this._logger = logger; + this._messageRouter = messageRouter; } /** @@ -179,7 +180,7 @@ export class PackageMetadataManager { ); if (FileSystem.exists(tsdocMetadataPath)) { - this._logger.logVerbose('Found metadata in ' + tsdocMetadataPath); + this._messageRouter.logVerbose(ConsoleMessageId.FoundTSDocMetadata, 'Found metadata in ' + tsdocMetadataPath); // If the file exists at all, assume it was written by API Extractor aedocSupported = true; } diff --git a/apps/api-extractor/src/api/ConsoleMessageId.ts b/apps/api-extractor/src/api/ConsoleMessageId.ts new file mode 100644 index 00000000000..b8e145fd617 --- /dev/null +++ b/apps/api-extractor/src/api/ConsoleMessageId.ts @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +/** + * Unique identifiers for console messages reported by API Extractor. + * + * @remarks + * + * These strings are possible values for the {@link ExtractorMessage.messageId} property + * when the `ExtractorMessage.category` is {@link ExtractorMessageCategory.Console}. + * + * @public + */ +export const enum ConsoleMessageId { + /** + * "'Found metadata in ___" + */ + FoundTSDocMetadata = 'console-found-tsdoc-metadata', + + /** + * "Writing: ___" + */ + WritingDocModelFile = 'console-writing-doc-model-file', + + /** + * "Writing package typings: ___" + */ + WritingDtsRollup = 'console-writing-dts-rollup', + + /** + * "You have changed the public API signature for this project. Please overwrite ___ with a + * copy of ___ and then request an API review. See the Git repository README.md for more info." + */ + ApiReportNotCopied = 'console-api-report-not-copied', + + /** + * "You have changed the public API signature for this project. Updating ___" + */ + ApiReportCopied = 'console-api-report-copied', + + /** + * "The API signature is up to date: ___" + */ + ApiReportUnchanged = 'console-api-report-unchanged', + + /** + * "The API review file has not been set up. Do this by copying ___ to ___ and committing it." + */ + ApiReportMissing = 'console-api-report-missing' +} diff --git a/apps/api-extractor/src/api/Extractor.ts b/apps/api-extractor/src/api/Extractor.ts index 0f727811921..5c1572b4bbf 100644 --- a/apps/api-extractor/src/api/Extractor.ts +++ b/apps/api-extractor/src/api/Extractor.ts @@ -1,9 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import lodash = require('lodash'); -import colors = require('colors'); - import { FileSystem, NewlineKind, @@ -11,10 +8,8 @@ import { IPackageJson } from '@microsoft/node-core-library'; import { ExtractorConfig } from './ExtractorConfig'; -import { ILogger } from './ILogger'; import { Collector } from '../collector/Collector'; import { DtsRollupGenerator, DtsRollupKind } from '../generators/DtsRollupGenerator'; -import { MonitoredLogger } from './MonitoredLogger'; import { ApiModelGenerator } from '../generators/ApiModelGenerator'; import { ApiPackage } from '@microsoft/api-extractor-model'; import { ReviewFileGenerator } from '../generators/ReviewFileGenerator'; @@ -22,6 +17,9 @@ import { PackageMetadataManager } from '../analyzer/PackageMetadataManager'; import { ValidationEnhancer } from '../enhancers/ValidationEnhancer'; import { DocCommentEnhancer } from '../enhancers/DocCommentEnhancer'; import { CompilerState } from './CompilerState'; +import { ExtractorMessage } from './ExtractorMessage'; +import { MessageRouter } from '../collector/MessageRouter'; +import { ConsoleMessageId } from './ConsoleMessageId'; /** * Runtime options for Extractor. @@ -35,12 +33,6 @@ export interface IExtractorInvokeOptions { */ compilerState?: CompilerState; - /** - * Allows the caller to customize how API Extractor's errors, warnings, and informational logging is processed. - * If omitted, the output will be printed to the console. - */ - customLogger?: Partial; - /** * Indicates that API Extractor is running as part of a local build, e.g. on developer's * machine. This disables certain validation that would normally be performed @@ -51,12 +43,27 @@ export interface IExtractorInvokeOptions { */ localBuild?: boolean; + /** + * If true, API Extractor will include {@link ExtractorLogLevel.Verbose} messages in its output. + */ + showVerboseMessages?: boolean; + /** * By default API Extractor uses its own TypeScript compiler version to analyze your project. * This can often cause compiler errors due to incompatibilities between different TS versions. * Use this option to specify the folder path for your compiler version. */ typescriptCompilerFolder?: string; + + /** + * An optional callback function that will be called for each `ExtractorMessage` before it is displayed by + * API Extractor. The callback can customize the message, handle it, or discard it. + * + * @remarks + * If a `messageCallback` is not provided, then by default API Extractor will print the messages to + * the STDERR/STDOUT console. + */ + messageCallback?: (message: ExtractorMessage) => void; } /** @@ -88,12 +95,23 @@ export class ExtractorResult { public readonly succeeded: boolean; /** - * Reports the number of times that {@link ILogger.logError} was called. + * Returns true if the API report was found to have changed. + */ + public readonly apiReportChanged: boolean; + + /** + * Reports the number of errors encountered during analysis. + * + * @remarks + * This does not count exceptions, where unexpected issues prematurely abort the operation. */ public readonly errorCount: number; /** - * Reports the number of times that {@link ILogger.logWarning} was called. + * Reports the number of warnings encountered during analysis. + * + * @remarks + * This does not count warnings that are emitted in the API report file. */ public readonly warningCount: number; @@ -102,6 +120,7 @@ export class ExtractorResult { this.compilerState = properties.compilerState; this.extractorConfig = properties.extractorConfig; this.succeeded = properties.succeeded; + this.apiReportChanged = properties.apiReportChanged; this.errorCount = properties.errorCount; this.warningCount = properties.warningCount; } @@ -130,13 +149,6 @@ export class Extractor { return PackageJsonLookup.loadOwnPackageJson(__dirname); } - private static _defaultLogger: ILogger = { - logVerbose: (message: string) => console.log('(Verbose) ' + message), - logInfo: (message: string) => console.log(message), - logWarning: (message: string) => console.warn(colors.yellow(message)), - logError: (message: string) => console.error(colors.red(message)) - }; - /** * Load the api-extractor.json config file from the specified path, and then invoke API Extractor. */ @@ -155,18 +167,8 @@ export class Extractor { options = { }; } - let mergedLogger: ILogger; - if (options && options.customLogger) { - mergedLogger = lodash.merge(lodash.clone(Extractor._defaultLogger), options.customLogger); - } else { - mergedLogger = Extractor._defaultLogger; - } - const monitoredLogger: MonitoredLogger = new MonitoredLogger(mergedLogger); - const localBuild: boolean = options.localBuild || false; - monitoredLogger.resetCounters(); - let compilerState: CompilerState | undefined; if (options.compilerState) { compilerState = options.compilerState; @@ -176,10 +178,13 @@ export class Extractor { const collector: Collector = new Collector({ program: compilerState.program, - logger: monitoredLogger, + messageCallback: options.messageCallback, extractorConfig: extractorConfig }); + const messageRouter: MessageRouter = collector.messageRouter; + messageRouter.showVerboseMessages = !!options.showVerboseMessages; + collector.analyze(); DocCommentEnhancer.analyze(collector); @@ -189,7 +194,7 @@ export class Extractor { const apiPackage: ApiPackage = modelBuilder.buildApiPackage(); if (extractorConfig.docModelEnabled) { - monitoredLogger.logVerbose('Writing: ' + extractorConfig.apiJsonFilePath); + messageRouter.logVerbose(ConsoleMessageId.WritingDocModelFile, 'Writing: ' + extractorConfig.apiJsonFilePath); apiPackage.saveToJsonFile(extractorConfig.apiJsonFilePath, { toolPackage: Extractor.packageName, toolVersion: Extractor.version, @@ -200,6 +205,8 @@ export class Extractor { }); } + let apiReportChanged: boolean = false; + if (extractorConfig.apiReportEnabled) { const actualApiReportPath: string = extractorConfig.reportTempFilePath; const actualApiReviewShortPath: string = extractorConfig._getShortFilePath(extractorConfig.reportTempFilePath); @@ -222,7 +229,8 @@ export class Extractor { if (!ReviewFileGenerator.areEquivalentApiFileContents(actualApiReviewContent, expectedApiReviewContent)) { if (!localBuild) { // For production, issue a warning that will break the CI build. - monitoredLogger.logWarning('You have changed the public API signature for this project.' + messageRouter.logWarning(ConsoleMessageId.ApiReportNotCopied, + 'You have changed the public API signature for this project.' // @microsoft/gulp-core-build seems to run JSON.stringify() on the error messages for some reason, // so try to avoid escaped characters: + ` Please overwrite ${expectedApiReviewShortPath} with a` @@ -230,7 +238,8 @@ export class Extractor { + ' and then request an API review. See the Git repository README.md for more info.'); } else { // For a local build, just copy the file automatically. - monitoredLogger.logWarning('You have changed the public API signature for this project.' + messageRouter.logWarning(ConsoleMessageId.ApiReportCopied, + 'You have changed the public API signature for this project.' + ` Updating ${expectedApiReviewShortPath}`); FileSystem.writeFile(expectedApiReviewPath, actualApiReviewContent, { @@ -238,31 +247,26 @@ export class Extractor { convertLineEndings: NewlineKind.CrLf }); } + + apiReportChanged = true; } else { - monitoredLogger.logVerbose(`The API signature is up to date: ${actualApiReviewShortPath}`); + messageRouter.logVerbose(ConsoleMessageId.ApiReportUnchanged, + `The API signature is up to date: ${actualApiReviewShortPath}`); } } else { // NOTE: This warning seems like a nuisance, but it has caught genuine mistakes. // For example, when projects were moved into category folders, the relative path for // the API review files ended up in the wrong place. - monitoredLogger.logError(`The API review file has not been set up.` + messageRouter.logError(ConsoleMessageId.ApiReportMissing, `The API review file has not been set up.` + ` Do this by copying ${actualApiReviewShortPath}` + ` to ${expectedApiReviewShortPath} and committing it.`); } } if (extractorConfig.rollupEnabled) { - Extractor._generateRollupDtsFile(collector, monitoredLogger, - extractorConfig.publicTrimmedFilePath, - DtsRollupKind.PublicRelease); - - Extractor._generateRollupDtsFile(collector, monitoredLogger, - extractorConfig.betaTrimmedFilePath, - DtsRollupKind.BetaRelease); - - Extractor._generateRollupDtsFile(collector, monitoredLogger, - extractorConfig.untrimmedFilePath, - DtsRollupKind.InternalRelease); + Extractor._generateRollupDtsFile(collector, extractorConfig.publicTrimmedFilePath, DtsRollupKind.PublicRelease); + Extractor._generateRollupDtsFile(collector, extractorConfig.betaTrimmedFilePath, DtsRollupKind.BetaRelease); + Extractor._generateRollupDtsFile(collector, extractorConfig.untrimmedFilePath, DtsRollupKind.InternalRelease); } if (extractorConfig.tsdocMetadataEnabled) { @@ -270,33 +274,32 @@ export class Extractor { PackageMetadataManager.writeTsdocMetadataFile(extractorConfig.tsdocMetadataFilePath); } - // Show out all the messages that we collected during analysis - collector.messageRouter.reportMessagesToLogger(monitoredLogger, collector.workingPackage.packageFolder); + // Show all the messages that we collected during analysis + messageRouter.handleRemainingNonConsoleMessages(); // Determine success let succeeded: boolean; if (localBuild) { // For a local build, fail if there were errors (but ignore warnings) - succeeded = monitoredLogger.errorCount === 0; + succeeded = messageRouter.errorCount === 0; } else { // For a production build, fail if there were any errors or warnings - succeeded = monitoredLogger.errorCount + monitoredLogger.warningCount === 0; + succeeded = messageRouter.errorCount + messageRouter.warningCount === 0; } return new ExtractorResult({ compilerState, extractorConfig, succeeded, - errorCount: monitoredLogger.errorCount, - warningCount: monitoredLogger.warningCount + apiReportChanged, + errorCount: messageRouter.errorCount, + warningCount: messageRouter.warningCount }); } - private static _generateRollupDtsFile(collector: Collector, monitoredLogger: MonitoredLogger, - outputPath: string, dtsKind: DtsRollupKind): void { - + private static _generateRollupDtsFile(collector: Collector, outputPath: string, dtsKind: DtsRollupKind): void { if (outputPath !== '') { - monitoredLogger.logVerbose(`Writing package typings: ${outputPath}`); + collector.messageRouter.logVerbose(ConsoleMessageId.WritingDtsRollup, `Writing package typings: ${outputPath}`); DtsRollupGenerator.writeTypingsFile(collector, outputPath, dtsKind); } } diff --git a/apps/api-extractor/src/api/ExtractorLogLevel.ts b/apps/api-extractor/src/api/ExtractorLogLevel.ts new file mode 100644 index 00000000000..40a687aa3a0 --- /dev/null +++ b/apps/api-extractor/src/api/ExtractorLogLevel.ts @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +/** + * Used with {@link IConfigMessageReportingRule.logLevel} and {@link IExtractorInvokeOptions.messageCallback}. + * + * @remarks + * This is part of the {@link IConfigFile} structure. + * + * @public + */ +export const enum ExtractorLogLevel { + /** + * The message will be displayed as an error. + * + * @remarks + * Errors typically cause the build to fail and return a nonzero exit code. + */ + Error = 'error', + + /** + * The message will be displayed as an warning. + * + * @remarks + * Warnings typically cause a production build fail and return a nonzero exit code. For a non-production build + * (e.g. using the `--local` option with `api-extractor run`), the warning is displayed but the build will not fail. + */ + Warning = 'warning', + + /** + * The message will be displayed as an informational message. + * + * @remarks + * Informational messages may contain newlines to ensure nice formatting of the output, + * however word-wrapping is the responsibility of the message handler. + */ + Info = 'info', + + /** + * The message will be displayed only when "verbose" output is requested, e.g. using the `--verbose` + * command line option. + */ + Verbose = 'verbose', + + /** + * The message will be discarded entirely. + */ + None = 'none' +} diff --git a/apps/api-extractor/src/api/ExtractorMessage.ts b/apps/api-extractor/src/api/ExtractorMessage.ts index c92a5227557..d9a2bad88ef 100644 --- a/apps/api-extractor/src/api/ExtractorMessage.ts +++ b/apps/api-extractor/src/api/ExtractorMessage.ts @@ -5,6 +5,8 @@ import * as tsdoc from '@microsoft/tsdoc'; import * as path from 'path'; import { ExtractorMessageId } from './ExtractorMessageId'; import { Path, Text } from '@microsoft/node-core-library'; +import { ExtractorLogLevel } from './ExtractorLogLevel'; +import { ConsoleMessageId } from './ConsoleMessageId'; /** * Used by {@link ExtractorMessage.properties}. @@ -54,7 +56,17 @@ export const enum ExtractorMessageCategory { * These strings begin with the prefix "ae-". * Example: `ae-extra-release-tag` */ - Extractor = 'Extractor' + Extractor = 'Extractor', + + /** + * Console messages communicate the progress of the overall operation. They may include newlines to ensure + * nice formatting. They are output in real time, and cannot be routed to the API Report file. + * + * @remarks + * These strings begin with the prefix "console-". + * Example: `console-writing-typings-file` + */ + Console = 'console' } /** @@ -62,12 +74,13 @@ export const enum ExtractorMessageCategory { */ export interface IExtractorMessageOptions { category: ExtractorMessageCategory; - messageId: tsdoc.TSDocMessageId | ExtractorMessageId | string; + messageId: tsdoc.TSDocMessageId | ExtractorMessageId | ConsoleMessageId | string; text: string; sourceFilePath?: string; sourceFileLine?: number; sourceFileColumn?: number; properties?: IExtractorMessageProperties; + logLevel?: ExtractorLogLevel; } /** @@ -76,6 +89,9 @@ export interface IExtractorMessageOptions { * @public */ export class ExtractorMessage { + private _handled: boolean; + private _logLevel: ExtractorLogLevel; + /** * The category of issue. */ @@ -85,7 +101,7 @@ export class ExtractorMessage { * A text string that uniquely identifies the issue type. This identifier can be used to suppress * or configure the reporting of issues, and also to search for help about an issue. */ - public readonly messageId: tsdoc.TSDocMessageId | ExtractorMessageId | string; + public readonly messageId: tsdoc.TSDocMessageId | ExtractorMessageId | ConsoleMessageId | string; /** * The text description of this issue. @@ -124,6 +140,62 @@ export class ExtractorMessage { this.sourceFileLine = options.sourceFileLine; this.sourceFileColumn = options.sourceFileColumn; this.properties = options.properties || { }; + + this._handled = false; + this._logLevel = options.logLevel || ExtractorLogLevel.None; + } + + /** + * If the {@link IExtractorInvokeOptions.messageCallback} sets this property to true, it will prevent the message + * from being displayed by API Extractor. + * + * @remarks + * If the `messageCallback` routes the message to a custom handler (e.g. a toolchain logger), it should + * assign `handled = true` to prevent API Extractor from displaying it. Assigning `handled = true` for all messages + * would effectively disable all console output from the `Extractor` API. + * + * If `handled` is set to true, the message will still be included in the count of errors/warnings; + * to discard a message entirely, instead assign `logLevel = none`. + */ + public get handled(): boolean { + return this._handled; + } + + public set handled(value: boolean) { + if (this._handled && !value) { + throw new Error('One a message has been marked as handled, the "handled" property cannot be set to false'); + } + this._handled = value; + } + + /** + * Specifies how the message should be reported. + * + * @remarks + * If the {@link IExtractorInvokeOptions.messageCallback} handles the message (i.e. sets `handled = true`), + * it can use the `logLevel` to determine how to display the message. + * + * Alternatively, if API Extractor is handling the message, the `messageCallback` could assign `logLevel` to change + * how it will be processed. However, in general the recommended practice is to configure message routing + * using the `messages` section in api-extractor.json. + * + * To discard a message entirely, assign `logLevel = none`. + */ + public get logLevel(): ExtractorLogLevel { + return this._logLevel; + } + + public set logLevel(value: ExtractorLogLevel) { + switch (value) { + case ExtractorLogLevel.Error: + case ExtractorLogLevel.Info: + case ExtractorLogLevel.None: + case ExtractorLogLevel.Verbose: + case ExtractorLogLevel.Warning: + break; + default: + throw new Error('Invalid log level'); + } } /** diff --git a/apps/api-extractor/src/api/IConfigFile.ts b/apps/api-extractor/src/api/IConfigFile.ts index 5a5ac3f7312..1a0b5ae510e 100644 --- a/apps/api-extractor/src/api/IConfigFile.ts +++ b/apps/api-extractor/src/api/IConfigFile.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import { ExtractorLogLevel } from './ExtractorLogLevel'; + /** * Determines how the TypeScript compiler engine will be invoked by API Extractor. * @@ -198,38 +200,6 @@ export interface IConfigTsdocMetadata { tsdocMetadataFilePath?: string; } -/** - * Used with {@link IConfigMessageReportingRule.logLevel}. - * - * @remarks - * This is part of the {@link IConfigFile} structure. - * - * @public - */ -export const enum ExtractorMessageLogLevel { - /** - * The message will be written to the output log as an error. - * - * @remarks - * Errors cause the build to fail and return a nonzero exit code. - */ - Error = 'error', - - /** - * The message will be written to the build output as an warning. - * - * @remarks - * Warnings cause a production build fail and return a nonzero exit code. For a non-production build - * (e.g. using the `--local` option with `api-extractor run`), the warning is displayed but the build will not fail. - */ - Warning = 'warning', - - /** - * The message will not be reported to the output log. - */ - None = 'none' -} - /** * Configures reporting for a given message identifier. * @@ -245,7 +215,7 @@ export interface IConfigMessageReportingRule { * @remarks * Note that the `addToApiReviewFile` property may supersede this option. */ - logLevel: ExtractorMessageLogLevel; + logLevel: ExtractorLogLevel; /** * If API Extractor is configured to write an API review file (.api.md), then the message will be written diff --git a/apps/api-extractor/src/api/ILogger.ts b/apps/api-extractor/src/api/ILogger.ts deleted file mode 100644 index b01c5dddf3e..00000000000 --- a/apps/api-extractor/src/api/ILogger.ts +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -/** - * Provides a custom logging service to API Extractor. - * @public - */ -export interface ILogger { - /** - * Log a message that will only be shown in a "verbose" logging mode. - */ - logVerbose(message: string): void; - - /** - * Log a normal message. - */ - logInfo(message: string): void; - - /** - * Log a warning message. Typically it is shown in yellow and will break a production build. - */ - logWarning(message: string): void; - - /** - * Log an error message. Typically it is shown in red and will break the build, even if it - * is not a production build. - */ - logError(message: string): void; -} diff --git a/apps/api-extractor/src/api/MonitoredLogger.ts b/apps/api-extractor/src/api/MonitoredLogger.ts deleted file mode 100644 index 8175bddeb9c..00000000000 --- a/apps/api-extractor/src/api/MonitoredLogger.ts +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -import { ILogger } from './ILogger'; - -/** - * Used to collect statistics for an ILogger implementation. - */ -export class MonitoredLogger implements ILogger { - /** - * Number of calls to logError() - */ - public errorCount: number; - - /** - * Number of calls to logWarning() - */ - public warningCount: number; - - /** - * Number of calls to any logging method. - */ - public messageCount: number; - - private _innerLogger: ILogger; - - constructor(logger: ILogger) { - this._innerLogger = logger; - this.resetCounters(); - } - - public logVerbose(message: string): void { - ++this.messageCount; - this._innerLogger.logVerbose(message); - } - - public logInfo(message: string): void { - ++this.messageCount; - this._innerLogger.logInfo(message); - } - - public logWarning(message: string): void { - ++this.messageCount; - ++this.warningCount; - this._innerLogger.logWarning(message); - } - - public logError(message: string): void { - ++this.messageCount; - ++this.errorCount; - this._innerLogger.logError(message); - } - - public resetCounters(): void { - this.errorCount = 0; - this.warningCount = 0; - this.messageCount = 0; - } -} diff --git a/apps/api-extractor/src/cli/RunAction.ts b/apps/api-extractor/src/cli/RunAction.ts index d5ab32f3b9e..b17f27a5a5d 100644 --- a/apps/api-extractor/src/cli/RunAction.ts +++ b/apps/api-extractor/src/cli/RunAction.ts @@ -25,6 +25,7 @@ import { ExtractorConfig } from '../api/ExtractorConfig'; export class RunAction extends CommandLineAction { private _configFileParameter: CommandLineStringParameter; private _localParameter: CommandLineFlagParameter; + private _verboseParameter: CommandLineFlagParameter; private _typescriptCompilerFolder: CommandLineStringParameter; constructor(parser: ApiExtractorCommandLine) { @@ -52,6 +53,12 @@ export class RunAction extends CommandLineAction { + ' review file is automatically copied in a local build.' }); + this._verboseParameter = this.defineFlagParameter({ + parameterLongName: '--verbose', + parameterShortName: '-v', + description: 'Show additional diagnostic messages in the output.' + }); + this._typescriptCompilerFolder = this.defineStringParameter({ parameterLongName: '--typescript-compiler-folder', argumentName: 'PATH', @@ -119,7 +126,7 @@ export class RunAction extends CommandLineAction { } } - console.log(`Using configuration from ${configFilename}` + os.EOL + os.EOL); + console.log(`Using configuration from ${configFilename}` + os.EOL); } const configObjectFullPath: string = path.resolve(configFilename); @@ -134,11 +141,14 @@ export class RunAction extends CommandLineAction { const extractorResult: ExtractorResult = Extractor.invoke(extractorConfig, { localBuild: this._localParameter.value, + showVerboseMessages: this._verboseParameter.value, typescriptCompilerFolder: typescriptCompilerFolder } ); - if (!extractorResult.succeeded) { + if (extractorResult.succeeded) { + console.log(os.EOL + 'API Extractor completed successfully'); + } else { process.exitCode = 1; if (extractorResult.errorCount > 0) { diff --git a/apps/api-extractor/src/collector/Collector.ts b/apps/api-extractor/src/collector/Collector.ts index eda18b52423..60e3de13c18 100644 --- a/apps/api-extractor/src/collector/Collector.ts +++ b/apps/api-extractor/src/collector/Collector.ts @@ -13,7 +13,6 @@ import { AedocDefinitions } from '@microsoft/api-extractor-model'; -import { ILogger } from '../api/ILogger'; import { ExtractorMessageId } from '../api/ExtractorMessageId'; import { CollectorEntity } from './CollectorEntity'; @@ -30,6 +29,7 @@ import { TypeScriptInternals } from '../analyzer/TypeScriptInternals'; import { MessageRouter } from './MessageRouter'; import { AstReferenceResolver } from '../analyzer/AstReferenceResolver'; import { ExtractorConfig } from '../api/ExtractorConfig'; +import { ExtractorMessage } from '../api/ExtractorMessage'; /** * Options for Collector constructor. @@ -45,7 +45,7 @@ export interface ICollectorOptions { */ program: ts.Program; - logger: ILogger; + messageCallback: ((message: ExtractorMessage) => void) | undefined; extractorConfig: ExtractorConfig; } @@ -65,8 +65,6 @@ export class Collector { public readonly packageJsonLookup: PackageJsonLookup; public readonly messageRouter: MessageRouter; - public readonly logger: ILogger; - public readonly workingPackage: WorkingPackage; private readonly _program: ts.Program; @@ -85,9 +83,7 @@ export class Collector { constructor(options: ICollectorOptions) { this.packageJsonLookup = new PackageJsonLookup(); - this.messageRouter = new MessageRouter(options.extractorConfig.messages || { }); - this.logger = options.logger; this._program = options.program; const extractorConfig: ExtractorConfig = options.extractorConfig; @@ -111,11 +107,17 @@ export class Collector { entryPointSourceFile }); + this.messageRouter = new MessageRouter( + this.workingPackage.packageFolder, + options.messageCallback, + options.extractorConfig.messages || { }); + this.program = options.program; this.typeChecker = options.program.getTypeChecker(); this._tsdocParser = new tsdoc.TSDocParser(AedocDefinitions.tsdocConfiguration); - this.astSymbolTable = new AstSymbolTable(this.program, this.typeChecker, this.packageJsonLookup, this.logger); + this.astSymbolTable = new AstSymbolTable(this.program, this.typeChecker, this.packageJsonLookup, + this.messageRouter); this.astReferenceResolver = new AstReferenceResolver(this.astSymbolTable, this.workingPackage); } diff --git a/apps/api-extractor/src/collector/MessageRouter.ts b/apps/api-extractor/src/collector/MessageRouter.ts index 081d5576717..bdfaba671c3 100644 --- a/apps/api-extractor/src/collector/MessageRouter.ts +++ b/apps/api-extractor/src/collector/MessageRouter.ts @@ -1,9 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import * as colors from 'colors'; import * as ts from 'typescript'; import * as tsdoc from '@microsoft/tsdoc'; -import { Sort } from '@microsoft/node-core-library'; +import { Sort, InternalError } from '@microsoft/node-core-library'; import { AedocDefinitions } from '@microsoft/api-extractor-model'; import { TypeScriptMessageFormatter } from '../analyzer/TypeScriptMessageFormatter'; @@ -18,42 +19,51 @@ import { import { ExtractorMessageId, allExtractorMessageIds } from '../api/ExtractorMessageId'; import { IExtractorMessagesConfig, - ExtractorMessageLogLevel, IConfigMessageReportingRule } from '../api/IConfigFile'; -import { ILogger } from '../api/ILogger'; import { SourceMapper } from './SourceMapper'; +import { ExtractorLogLevel } from '../api/ExtractorLogLevel'; +import { ConsoleMessageId } from '../api/ConsoleMessageId'; interface IReportingRule { - logLevel: ExtractorMessageLogLevel; + logLevel: ExtractorLogLevel; addToApiReviewFile: boolean; } export class MessageRouter { + private readonly _workingPackageFolder: string; + private readonly _messageCallback: ((message: ExtractorMessage) => void) | undefined; + // All messages private readonly _messages: ExtractorMessage[]; // For each AstDeclaration, the messages associated with it. This is used when addToApiReviewFile=true private readonly _associatedMessagesForAstDeclaration: Map; - // Messages that got written to the API review file - private readonly _messagesAddedToApiReviewFile: Set; - private readonly _sourceMapper: SourceMapper; // Normalized representation of the routing rules from api-extractor.json private _reportingRuleByMessageId: Map = new Map(); - private _compilerDefaultRule: IReportingRule = { logLevel: ExtractorMessageLogLevel.None, + private _compilerDefaultRule: IReportingRule = { logLevel: ExtractorLogLevel.None, addToApiReviewFile: false }; - private _extractorDefaultRule: IReportingRule = { logLevel: ExtractorMessageLogLevel.None, + private _extractorDefaultRule: IReportingRule = { logLevel: ExtractorLogLevel.None, addToApiReviewFile: false }; - private _tsdocDefaultRule: IReportingRule = { logLevel: ExtractorMessageLogLevel.None, + private _tsdocDefaultRule: IReportingRule = { logLevel: ExtractorLogLevel.None, addToApiReviewFile: false }; - public constructor(messagesConfig: IExtractorMessagesConfig) { + public errorCount: number = 0; + public warningCount: number = 0; + public showVerboseMessages: boolean = false; + + public constructor(workingPackageFolder: string, + messageCallback: ((message: ExtractorMessage) => void) | undefined, + messagesConfig: IExtractorMessagesConfig) { + + this._workingPackageFolder = workingPackageFolder; + this._messageCallback = messageCallback; + this._messages = []; this._associatedMessagesForAstDeclaration = new Map(); - this._messagesAddedToApiReviewFile = new Set(); this._sourceMapper = new SourceMapper(); this._applyMessagesConfig(messagesConfig); @@ -266,7 +276,7 @@ export class MessageRouter { for (const associatedMessage of associatedMessages) { // Make sure we didn't already report this message for some reason - if (!this._messagesAddedToApiReviewFile.has(associatedMessage)) { + if (!associatedMessage.handled) { // Is this message type configured to go in the API review file? const reportingRule: IReportingRule = this._getRuleForMessage(associatedMessage); @@ -274,7 +284,7 @@ export class MessageRouter { // Include it in the result, and record that it went to the API review file messagesForApiReviewFile.push(associatedMessage); - this._messagesAddedToApiReviewFile.add(associatedMessage); + associatedMessage.handled = true; } } @@ -294,7 +304,7 @@ export class MessageRouter { for (const unassociatedMessage of this.messages) { // Make sure we didn't already report this message for some reason - if (!this._messagesAddedToApiReviewFile.has(unassociatedMessage)) { + if (!unassociatedMessage.handled) { // Is this message type configured to go in the API review file? const reportingRule: IReportingRule = this._getRuleForMessage(unassociatedMessage); @@ -302,7 +312,7 @@ export class MessageRouter { // Include it in the result, and record that it went to the API review file messagesForApiReviewFile.push(unassociatedMessage); - this._messagesAddedToApiReviewFile.add(unassociatedMessage); + unassociatedMessage.handled = true; } } @@ -317,13 +327,12 @@ export class MessageRouter { * `fetchAssociatedMessagesForReviewFile()` or `fetchUnassociatedMessagesForReviewFile()`. * These messages will be shown on the console. */ - public reportMessagesToLogger(logger: ILogger, workingPackageFolderPath: string): ExtractorMessage[] { + public handleRemainingNonConsoleMessages(): void { const messagesForLogger: ExtractorMessage[] = []; for (const message of this.messages) { - // Make sure we didn't already report this message - if (!this._messagesAddedToApiReviewFile.has(message)) { + if (!message.handled) { messagesForLogger.push(message); } } @@ -331,23 +340,99 @@ export class MessageRouter { this._sortMessagesForOutput(messagesForLogger); for (const message of messagesForLogger) { - // Is this message type configured to go to the console? - const reportingRule: IReportingRule = this._getRuleForMessage(message); - switch (reportingRule.logLevel) { - case ExtractorMessageLogLevel.Error: - logger.logError('Error: ' + message.formatMessageWithLocation(workingPackageFolderPath)); - break; - case ExtractorMessageLogLevel.Warning: - logger.logWarning('Warning: ' + message.formatMessageWithLocation(workingPackageFolderPath)); - break; - case ExtractorMessageLogLevel.None: - break; - default: - throw new Error(`Invalid logLevel value: ${JSON.stringify(reportingRule.logLevel)}`); + this._handleMessage(message); + } + } + + public logError(messageId: ConsoleMessageId, message: string, properties?: IExtractorMessageProperties): void { + this._handleMessage(new ExtractorMessage({ + category: ExtractorMessageCategory.Console, + messageId, + text: message, + properties, + logLevel: ExtractorLogLevel.Error + })); + } + + public logWarning(messageId: ConsoleMessageId, message: string, properties?: IExtractorMessageProperties): void { + this._handleMessage(new ExtractorMessage({ + category: ExtractorMessageCategory.Console, + messageId, + text: message, + properties, + logLevel: ExtractorLogLevel.Warning + })); + } + + public logInfo(messageId: ConsoleMessageId, message: string, properties?: IExtractorMessageProperties): void { + this._handleMessage(new ExtractorMessage({ + category: ExtractorMessageCategory.Console, + messageId, + text: message, + properties, + logLevel: ExtractorLogLevel.Info + })); + } + + public logVerbose(messageId: ConsoleMessageId, message: string, properties?: IExtractorMessageProperties): void { + this._handleMessage(new ExtractorMessage({ + category: ExtractorMessageCategory.Console, + messageId, + text: message, + properties, + logLevel: ExtractorLogLevel.Verbose + })); + } + + /** + * Give the calling application a chance to handle the `ExtractorMessage`, and if not, display it on the console. + */ + private _handleMessage(message: ExtractorMessage): void { + if (message.handled) { + return; + } + + if (this._messageCallback) { + this._messageCallback(message); + + if (message.handled) { + return; } } - return messagesForLogger; + message.handled = true; + + if (message.logLevel === ExtractorLogLevel.None) { + return; + } + + let messageText: string; + if (message.category === ExtractorMessageCategory.Console) { + messageText = message.text; + } else { + messageText = message.formatMessageWithLocation(this._workingPackageFolder); + } + + switch (message.logLevel) { + case ExtractorLogLevel.Error: + ++this.errorCount; + console.error(colors.red('Error: ' + messageText)); + break; + case ExtractorLogLevel.Warning: + ++this.warningCount; + console.warn(colors.yellow('Warning: ' + messageText)); + break; + case ExtractorLogLevel.Info: + console.log(messageText); + break; + case ExtractorLogLevel.Verbose: + if (this.showVerboseMessages) { + console.log(colors.cyan(messageText)); + } + break; + default: + throw new Error(`Invalid logLevel value: ${JSON.stringify(message.logLevel)}`); + } } /** @@ -365,6 +450,8 @@ export class MessageRouter { return this._extractorDefaultRule; case ExtractorMessageCategory.TSDoc: return this._tsdocDefaultRule; + case ExtractorMessageCategory.Console: + throw new InternalError('ExtractorMessageCategory.Console is not supported with IReportingRule'); } } diff --git a/apps/api-extractor/src/index.ts b/apps/api-extractor/src/index.ts index af7be29f370..dd58da6ca71 100644 --- a/apps/api-extractor/src/index.ts +++ b/apps/api-extractor/src/index.ts @@ -9,6 +9,8 @@ * @packageDocumentation */ +export { ConsoleMessageId } from './api/ConsoleMessageId'; + export { CompilerState, ICompilerStateCreateOptions } from './api/CompilerState'; export { @@ -22,6 +24,8 @@ export { ExtractorConfig } from './api/ExtractorConfig'; +export { ExtractorLogLevel } from './api/ExtractorLogLevel'; + export { ExtractorMessage, IExtractorMessageProperties, @@ -36,11 +40,8 @@ export { IConfigDocModel, IConfigDtsRollup, IConfigTsdocMetadata, - ExtractorMessageLogLevel, IConfigMessageReportingRule, IConfigMessageReportingTable, IExtractorMessagesConfig, IConfigFile } from './api/IConfigFile'; - -export { ILogger } from './api/ILogger'; diff --git a/build-tests/api-extractor-scenarios/src/runScenarios.ts b/build-tests/api-extractor-scenarios/src/runScenarios.ts index affd8f5a7ac..bf273df8d7f 100644 --- a/build-tests/api-extractor-scenarios/src/runScenarios.ts +++ b/build-tests/api-extractor-scenarios/src/runScenarios.ts @@ -3,8 +3,8 @@ import * as path from 'path'; import * as colors from 'colors'; -import { JsonFile, Executable, FileSystem } from '@microsoft/node-core-library'; -import { Extractor, ExtractorConfig, CompilerState, ExtractorResult } from '@microsoft/api-extractor'; +import { JsonFile, FileSystem } from '@microsoft/node-core-library'; +import { Extractor, ExtractorConfig, CompilerState, ExtractorResult, ExtractorMessage, ConsoleMessageId } from '@microsoft/api-extractor'; export function runScenarios(buildConfigPath: string): void { const buildConfig = JsonFile.load(buildConfigPath); @@ -75,6 +75,8 @@ export function runScenarios(buildConfigPath: string): void { for (const scenarioFolderName of buildConfig.scenarioFolderNames) { const apiExtractorJsonPath: string = `./temp/configs/api-extractor-${scenarioFolderName}.json`; + console.log('Scenario: ' + scenarioFolderName); + // Run the API Extractor command-line const extractorConfig: ExtractorConfig = ExtractorConfig.loadFileAndPrepare(apiExtractorJsonPath); @@ -86,18 +88,12 @@ export function runScenarios(buildConfigPath: string): void { const extractorResult: ExtractorResult = Extractor.invoke(extractorConfig, { localBuild: true, - customLogger: { - logWarning: (message: string): void => { - // This will get fixed with https://github.com/Microsoft/web-build-tools/issues/1133 - if (message.indexOf('You have changed') >= 0) { - // ignore the "You have changed the public API signature for this project." - // warning for now. - } else { - console.warn(colors.yellow(message)); - } - }, - logVerbose: (message: string): void => { - // ignore verbose output + showVerboseMessages: true, + messageCallback: (message: ExtractorMessage) => { + if (message.messageId === ConsoleMessageId.ApiReportCopied) { + // ignore the "You have changed the public API signature for this project." + // warning for until the above issue #1018 is fixed. + message.handled = true; } }, compilerState diff --git a/common/changes/@microsoft/api-extractor/octogonz-ae-remove-logger_2019-04-05-16-09.json b/common/changes/@microsoft/api-extractor/octogonz-ae-remove-logger_2019-04-05-16-09.json new file mode 100644 index 00000000000..0179ddb2088 --- /dev/null +++ b/common/changes/@microsoft/api-extractor/octogonz-ae-remove-logger_2019-04-05-16-09.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@microsoft/api-extractor", + "comment": "(Breaking change) Removed the ILogger API and renamed ExtractorMessageLogLevel to ExtractorLogLevel", + "type": "patch" + } + ], + "packageName": "@microsoft/api-extractor", + "email": "4673363+octogonz@users.noreply.github.com" +} \ No newline at end of file diff --git a/common/changes/@microsoft/api-extractor/octogonz-ae-remove-logger_2019-04-05-16-11.json b/common/changes/@microsoft/api-extractor/octogonz-ae-remove-logger_2019-04-05-16-11.json new file mode 100644 index 00000000000..d8a35bd6729 --- /dev/null +++ b/common/changes/@microsoft/api-extractor/octogonz-ae-remove-logger_2019-04-05-16-11.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@microsoft/api-extractor", + "comment": "(Breaking change) Extractor console output is now modeled as ExtractorMessage objects and can be customized/filtered/handled by IExtractorInvokeOptions.messageCallback", + "type": "patch" + } + ], + "packageName": "@microsoft/api-extractor", + "email": "4673363+octogonz@users.noreply.github.com" +} \ No newline at end of file diff --git a/common/reviews/api/api-extractor.api.md b/common/reviews/api/api-extractor.api.md index af5c2c5ce7a..d2e8ad4783c 100644 --- a/common/reviews/api/api-extractor.api.md +++ b/common/reviews/api/api-extractor.api.md @@ -15,6 +15,17 @@ export class CompilerState { readonly program: ts.Program; } +// @public +export const enum ConsoleMessageId { + ApiReportCopied = "console-api-report-copied", + ApiReportMissing = "console-api-report-missing", + ApiReportNotCopied = "console-api-report-not-copied", + ApiReportUnchanged = "console-api-report-unchanged", + FoundTSDocMetadata = "console-found-tsdoc-metadata", + WritingDocModelFile = "console-writing-doc-model-file", + WritingDtsRollup = "console-writing-dts-rollup" +} + // @public export class Extractor { static invoke(extractorConfig: ExtractorConfig, options?: IExtractorInvokeOptions): ExtractorResult; @@ -54,6 +65,15 @@ export class ExtractorConfig { readonly untrimmedFilePath: string; } +// @public +export const enum ExtractorLogLevel { + Error = "error", + Info = "info", + None = "none", + Verbose = "verbose", + Warning = "warning" +} + // @public export class ExtractorMessage { // Warning: (ae-forgotten-export) The symbol "IExtractorMessageOptions" needs to be exported by the entry point index.d.ts @@ -64,7 +84,9 @@ export class ExtractorMessage { formatMessageWithLocation(workingPackageFolderPath: string): string; // (undocumented) formatMessageWithoutLocation(): string; - readonly messageId: tsdoc.TSDocMessageId | ExtractorMessageId | string; + handled: boolean; + logLevel: ExtractorLogLevel; + readonly messageId: tsdoc.TSDocMessageId | ExtractorMessageId | ConsoleMessageId | string; readonly properties: IExtractorMessageProperties; readonly sourceFileColumn: number | undefined; readonly sourceFileLine: number | undefined; @@ -75,6 +97,7 @@ export class ExtractorMessage { // @public export const enum ExtractorMessageCategory { Compiler = "Compiler", + Console = "console", Extractor = "Extractor", TSDoc = "TSDoc" } @@ -96,17 +119,11 @@ export const enum ExtractorMessageId { UnresolvedLink = "ae-unresolved-link" } -// @public -export const enum ExtractorMessageLogLevel { - Error = "error", - None = "none", - Warning = "warning" -} - // @public export class ExtractorResult { // @internal constructor(properties: ExtractorResult); + readonly apiReportChanged: boolean; readonly compilerState: CompilerState; readonly errorCount: number; readonly extractorConfig: ExtractorConfig; @@ -167,7 +184,7 @@ export interface IConfigFile { // @public export interface IConfigMessageReportingRule { addToApiReviewFile?: boolean; - logLevel: ExtractorMessageLogLevel; + logLevel: ExtractorLogLevel; } // @public @@ -192,8 +209,9 @@ export interface IExtractorConfigPrepareOptions { // @public export interface IExtractorInvokeOptions { compilerState?: CompilerState; - customLogger?: Partial; localBuild?: boolean; + messageCallback?: (message: ExtractorMessage) => void; + showVerboseMessages?: boolean; typescriptCompilerFolder?: string; } @@ -209,13 +227,5 @@ export interface IExtractorMessagesConfig { tsdocMessageReporting?: IConfigMessageReportingTable; } -// @public -export interface ILogger { - logError(message: string): void; - logInfo(message: string): void; - logVerbose(message: string): void; - logWarning(message: string): void; -} - ``` diff --git a/stack/rush-stack-compiler-shared/src/shared/ApiExtractorRunner.ts b/stack/rush-stack-compiler-shared/src/shared/ApiExtractorRunner.ts index 5f2e4f720f5..5ff36932370 100644 --- a/stack/rush-stack-compiler-shared/src/shared/ApiExtractorRunner.ts +++ b/stack/rush-stack-compiler-shared/src/shared/ApiExtractorRunner.ts @@ -39,11 +39,24 @@ export class ApiExtractorRunner extends RushStackCompilerBase { try { const extractorOptions: IExtractorInvokeOptions = { ...this._extractorOptions, - customLogger: { - logVerbose: this._terminal.writeVerboseLine.bind(this._terminal), - logInfo: this._terminal.writeLine.bind(this._terminal), - logWarning: this._terminal.writeWarningLine.bind(this._terminal), - logError: this._terminal.writeErrorLine.bind(this._terminal) + messageCallback: (message: ApiExtractor.ExtractorMessage) => { + switch (message.logLevel) { + case ApiExtractor.ExtractorLogLevel.Error: + this._terminal.writeErrorLine.bind(this._terminal); + break; + case ApiExtractor.ExtractorLogLevel.Warning: + this._terminal.writeWarningLine.bind(this._terminal); + break; + case ApiExtractor.ExtractorLogLevel.Info: + this._terminal.writeLine.bind(this._terminal); + break; + case ApiExtractor.ExtractorLogLevel.Verbose: + this._terminal.writeVerboseLine.bind(this._terminal); + break; + default: + return; + } + message.handled = true; }, typescriptCompilerFolder: ToolPaths.typescriptPackagePath };