From 56bd21de6f62893140675c874c556fd1b95232d6 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 28 Jun 2021 19:40:33 +0200 Subject: [PATCH] feat(dev-infra): introduce shared tool for validating API signature (#42688) For the last years the Angular repositories relied on `ts-api-guardian` for testing the public API signature. This project worked well in general but its another inconvenience to maintain if we could rely on Microsoft's `api-extractor` tool. Especially since with TypeScript 4.3 issues with export aliases appeared that would require us to extend TS API guardian to support such exports. This is not as straightforward as it sounds, given it requires rewriting of declarations to show-case the proper name in the API golden. Microsoft's API extractor has integrated support for this. As of TypeScript 4.3, we want to start using the new `override` keyword. We are not able to use that keyword currently because an old version of API extractor is used in the `ng_module` rule to flatten the types into a single file. To fix this, we need to update `api-extractor`, but this unveils the issue with TS API guardian because the most recent version of api-extractor uses alias exports to avoid potential conflicts with globals available through the TypeScript default libraries (e.g. `dom.d.ts`). PR Close #42688 --- dev-infra/bazel/BUILD.bazel | 1 + dev-infra/bazel/api-golden/BUILD.bazel | 32 +++++ .../bazel/api-golden/find_entry_points.ts | 62 ++++++++ dev-infra/bazel/api-golden/index.bzl | 83 +++++++++++ dev-infra/bazel/api-golden/index.ts | 45 ++++++ .../bazel/api-golden/index_npm_packages.ts | 71 ++++++++++ dev-infra/bazel/api-golden/test_api_report.ts | 134 ++++++++++++++++++ dev-infra/tmpl-package.json | 22 +-- dev-infra/tsconfig.json | 3 +- 9 files changed, 442 insertions(+), 11 deletions(-) create mode 100644 dev-infra/bazel/api-golden/BUILD.bazel create mode 100644 dev-infra/bazel/api-golden/find_entry_points.ts create mode 100644 dev-infra/bazel/api-golden/index.bzl create mode 100644 dev-infra/bazel/api-golden/index.ts create mode 100644 dev-infra/bazel/api-golden/index_npm_packages.ts create mode 100644 dev-infra/bazel/api-golden/test_api_report.ts diff --git a/dev-infra/bazel/BUILD.bazel b/dev-infra/bazel/BUILD.bazel index 6fdb989c2b..1b8806153c 100644 --- a/dev-infra/bazel/BUILD.bazel +++ b/dev-infra/bazel/BUILD.bazel @@ -5,6 +5,7 @@ filegroup( srcs = [ "BUILD.bazel", "expand_template.bzl", + "//dev-infra/bazel/api-golden:files", "//dev-infra/bazel/browsers:files", "//dev-infra/bazel/remote-execution:files", ], diff --git a/dev-infra/bazel/api-golden/BUILD.bazel b/dev-infra/bazel/api-golden/BUILD.bazel new file mode 100644 index 0000000000..0ab29df547 --- /dev/null +++ b/dev-infra/bazel/api-golden/BUILD.bazel @@ -0,0 +1,32 @@ +load("@npm//@bazel/typescript:index.bzl", "ts_library") + +package(default_visibility = ["//visibility:public"]) + +exports_files([ + "index.ts", + "index_npm_packages.ts", +]) + +ts_library( + name = "api-golden", + srcs = [ + "find_entry_points.ts", + "index.ts", + "index_npm_packages.ts", + "test_api_report.ts", + ], + deps = [ + "@npm//@bazel/runfiles", + "@npm//@microsoft/api-extractor", + "@npm//@types/node", + "@npm//chalk", + "@npm//tslib", + "@npm//typescript", + ], +) + +# Expose the sources in the dev-infra NPM package. +filegroup( + name = "files", + srcs = glob(["*"]), +) diff --git a/dev-infra/bazel/api-golden/find_entry_points.ts b/dev-infra/bazel/api-golden/find_entry_points.ts new file mode 100644 index 0000000000..3343639fec --- /dev/null +++ b/dev-infra/bazel/api-golden/find_entry_points.ts @@ -0,0 +1,62 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {lstatSync, readdirSync, readFileSync} from 'fs'; +import {dirname, join} from 'path'; + +/** Interface describing a resolved NPM package entry point. */ +export interface PackageEntryPoint { + typesEntryPointPath: string; + packageJsonPath: string; +} + +/** Interface describing contents of a `package.json`. */ +interface PackageJson { + types?: string; + typings?: string; +} + +/** Finds all entry points within a given NPM package directory. */ +export function findEntryPointsWithinNpmPackage(dirPath: string): PackageEntryPoint[] { + const entryPoints: PackageEntryPoint[] = []; + + for (const packageJsonFilePath of findPackageJsonFilesInDirectory(dirPath)) { + const packageJson = JSON.parse(readFileSync(packageJsonFilePath, 'utf8')) as PackageJson; + const typesFile = packageJson.types || packageJson.typings; + + if (typesFile) { + entryPoints.push({ + packageJsonPath: packageJsonFilePath, + typesEntryPointPath: join(dirname(packageJsonFilePath), typesFile), + }); + } + } + + return entryPoints; +} + +/** Determine if the provided path is a directory. */ +function isDirectory(dirPath: string) { + try { + return lstatSync(dirPath).isDirectory(); + } catch { + return false; + } +} + +/** Finds all `package.json` files within a directory. */ +function* findPackageJsonFilesInDirectory(directoryPath: string): IterableIterator { + for (const fileName of readdirSync(directoryPath)) { + const fullPath = join(directoryPath, fileName); + if (isDirectory(fullPath)) { + yield* findPackageJsonFilesInDirectory(fullPath); + } else if (fileName === 'package.json') { + yield fullPath; + } + } +} diff --git a/dev-infra/bazel/api-golden/index.bzl b/dev-infra/bazel/api-golden/index.bzl new file mode 100644 index 0000000000..fd8548f66e --- /dev/null +++ b/dev-infra/bazel/api-golden/index.bzl @@ -0,0 +1,83 @@ +load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "nodejs_test") + +nodejs_test_args = [ + # Needed so that node doesn't walk back to the source directory. + # From there, the relative imports would point to .ts files. + "--node_options=--preserve-symlinks", + # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver + # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324 + "--bazel_patch_module_resolver", +] + +default_strip_export_pattern = "^ɵ(?!ɵdefineInjectable|ɵinject|ɵInjectableDef)" + +"""Escapes a Regular expression so that it can be passed as process argument.""" + +def _escape_regex_for_arg(value): + return "\"%s\"" % value + +""" + Builds an API report for the specified entry-point and compares it against the + specified golden +""" + +def api_golden_test( + name, + golden, + entry_point, + data = [], + strip_export_pattern = default_strip_export_pattern, + **kwargs): + quoted_export_pattern = _escape_regex_for_arg(strip_export_pattern) + + kwargs["tags"] = kwargs.get("tags", []) + ["api_guard"] + + nodejs_test( + name = name, + data = ["//dev-infra/bazel/api-golden", "//:package.json"] + data, + entry_point = "//dev-infra/bazel/api-golden:index.ts", + templated_args = nodejs_test_args + [golden, entry_point, "false", quoted_export_pattern], + **kwargs + ) + + nodejs_binary( + name = name + ".accept", + testonly = True, + data = ["//dev-infra/bazel/api-golden", "//:package.json"] + data, + entry_point = "//dev-infra/bazel/api-golden:index.ts", + templated_args = nodejs_test_args + [golden, entry_point, "true", quoted_export_pattern], + **kwargs + ) + +""" + Builds an API report for all entrypoints within the given NPM package and compares it + against goldens within the specified directory. +""" + +def api_golden_test_npm_package( + name, + golden_dir, + npm_package, + data = [], + strip_export_pattern = default_strip_export_pattern, + **kwargs): + quoted_export_pattern = _escape_regex_for_arg(strip_export_pattern) + + kwargs["tags"] = kwargs.get("tags", []) + ["api_guard"] + + nodejs_test( + name = name, + data = ["//dev-infra/bazel/api-golden"] + data, + entry_point = "//dev-infra/bazel/api-golden:index_npm_packages.ts", + templated_args = nodejs_test_args + [golden_dir, npm_package, "false", quoted_export_pattern], + **kwargs + ) + + nodejs_binary( + name = name + ".accept", + testonly = True, + data = ["//dev-infra/bazel/api-golden"] + data, + entry_point = "//dev-infra/bazel/api-golden:index_npm_packages.ts", + templated_args = nodejs_test_args + [golden_dir, npm_package, "true", quoted_export_pattern], + **kwargs + ) diff --git a/dev-infra/bazel/api-golden/index.ts b/dev-infra/bazel/api-golden/index.ts new file mode 100644 index 0000000000..59ad79ac02 --- /dev/null +++ b/dev-infra/bazel/api-golden/index.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {runfiles} from '@bazel/runfiles'; +import * as chalk from 'chalk'; + +import {testApiGolden} from './test_api_report'; + +/** + * Entry point for the `api_golden_test` Bazel rule. This function builds an API report for + * the specified entry point file and compares it against the specified golden file. + */ +async function main( + goldenFilePath: string, entryPointFilePath: string, approveGolden: boolean, + stripExportPattern: RegExp) { + const {succeeded, apiReportChanged} = + await testApiGolden(goldenFilePath, entryPointFilePath, approveGolden, stripExportPattern); + + if (!succeeded && apiReportChanged) { + console.error(chalk.red(`The API signature has changed and the golden file is outdated.`)); + console.info(chalk.yellow( + `Golden can be updated by running: yarn bazel run ${process.env.TEST_TARGET}.accept`)); + } + + // Bazel expects `3` as exit code for failing tests. + process.exitCode = succeeded ? 0 : 3; +} + +if (require.main === module) { + const args = process.argv.slice(2); + const goldenFilePath = runfiles.resolve(args[0]); + const entryPointFilePath = runfiles.resolve(args[1]); + const approveGolden = args[2] === 'true'; + const stripExportPattern = new RegExp(args[3]); + + main(goldenFilePath, entryPointFilePath, approveGolden, stripExportPattern).catch(e => { + console.error(e); + process.exit(1); + }); +} diff --git a/dev-infra/bazel/api-golden/index_npm_packages.ts b/dev-infra/bazel/api-golden/index_npm_packages.ts new file mode 100644 index 0000000000..74f7d9d64a --- /dev/null +++ b/dev-infra/bazel/api-golden/index_npm_packages.ts @@ -0,0 +1,71 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {runfiles} from '@bazel/runfiles'; +import * as chalk from 'chalk'; +import {join, relative} from 'path'; + +import {findEntryPointsWithinNpmPackage} from './find_entry_points'; +import {testApiGolden} from './test_api_report'; + +/** + * Entry point for the `api_golden_test_npm_package` Bazel rule. This function determines + * all types within the specified NPM package and builds API reports that will be compared + * against golden files within the given golden directory. + */ +async function main( + goldenDir: string, npmPackageDir: string, approveGolden: boolean, stripExportPattern: RegExp) { + const entryPoints = findEntryPointsWithinNpmPackage(npmPackageDir); + const outdatedGoldens: string[] = []; + let allTestsSucceeding = true; + + for (const {packageJsonPath, typesEntryPointPath} of entryPoints) { + const pkgRelativeName = relative(npmPackageDir, typesEntryPointPath); + // API extractor generates API reports as markdown files. For each types + // entry-point we maintain a separate golden file. These golden files are + // based on the name of the entry-point `.d.ts` file in the NPM package, + // but with the proper `.md` file extension. + // See: https://api-extractor.com/pages/overview/demo_api_report/. + const goldenName = pkgRelativeName.replace(/\.d\.ts$/, '.md'); + const goldenFilePath = join(goldenDir, goldenName); + + const {succeeded, apiReportChanged} = await testApiGolden( + goldenFilePath, typesEntryPointPath, approveGolden, stripExportPattern, packageJsonPath); + + // Keep track of outdated goldens. + if (!succeeded && apiReportChanged) { + outdatedGoldens.push(goldenName); + } + + allTestsSucceeding = allTestsSucceeding && succeeded; + } + + if (outdatedGoldens.length) { + console.error(chalk.red(`The following goldens are outdated:`)); + outdatedGoldens.forEach(name => console.info(`- ${name}`)); + console.info(); + console.info(chalk.yellow( + `The goldens can be updated by running: yarn bazel run ${process.env.TEST_TARGET}.accept`)); + } + + // Bazel expects `3` as exit code for failing tests. + process.exitCode = allTestsSucceeding ? 0 : 3; +} + +if (require.main === module) { + const args = process.argv.slice(2); + const goldenDir = runfiles.resolve(args[0]); + const npmPackageDir = runfiles.resolve(args[1]); + const approveGolden = args[2] === 'true'; + const stripExportPattern = new RegExp(args[3]); + + main(goldenDir, npmPackageDir, approveGolden, stripExportPattern).catch(e => { + console.error(e); + process.exit(1); + }); +} diff --git a/dev-infra/bazel/api-golden/test_api_report.ts b/dev-infra/bazel/api-golden/test_api_report.ts new file mode 100644 index 0000000000..6261ce6739 --- /dev/null +++ b/dev-infra/bazel/api-golden/test_api_report.ts @@ -0,0 +1,134 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {runfiles} from '@bazel/runfiles'; +import {ConsoleMessageId, Extractor, ExtractorConfig, ExtractorLogLevel, ExtractorMessage, ExtractorMessageId, ExtractorResult, IConfigFile} from '@microsoft/api-extractor'; +import {AstModule} from '@microsoft/api-extractor/lib/analyzer/AstModule'; +import {ExportAnalyzer} from '@microsoft/api-extractor/lib/analyzer/ExportAnalyzer'; +import {basename, dirname} from 'path'; + +/** + * Original definition of the `ExportAnalyzer#fetchAstModuleExportInfo` method. + * We store the original function since we monkey-patch it later to account for + * specified strip export patterns. + * */ +const _origFetchAstModuleExportInfo = ExportAnalyzer.prototype.fetchAstModuleExportInfo; + +/** + * Builds an API report for the given entry-point file and compares + * it against a golden file. + * + * @param goldenFilePath Path to an API report file that is used as golden + * @param indexFilePath Entry point file that is analyzed to build the API report. + * @param approveGolden Whether the golden file should be updated. + * @param stripExportPattern Regular Expression that can be used to filter out exports + * from the API report. + * @param packageJsonPath Optional path to a `package.json` file that contains the entry + * point. Note that the `package.json` is currently only used by `api-extractor` to determine + * the package name displayed within the API golden. + */ +export async function testApiGolden( + goldenFilePath: string, indexFilePath: string, approveGolden: boolean, + stripExportPattern: RegExp, + packageJsonPath = resolveWorkspacePackageJsonPath()): Promise { + // If no `TEST_TMPDIR` is defined, then this script runs using `bazel run`. We use + // the runfile directory as temporary directory for API extractor. + const tempDir = process.env.TEST_TMPDIR ?? process.cwd(); + + const configObject: IConfigFile = { + compiler: {overrideTsconfig: {files: [indexFilePath]}}, + projectFolder: dirname(packageJsonPath), + mainEntryPointFilePath: indexFilePath, + dtsRollup: {enabled: false}, + docModel: {enabled: false}, + apiReport: { + enabled: true, + reportFolder: dirname(goldenFilePath), + reportTempFolder: tempDir, + reportFileName: basename(goldenFilePath), + }, + tsdocMetadata: {enabled: false}, + newlineKind: 'lf', + messages: { + extractorMessageReporting: { + // If an export does not have a release tag (like `@public`), API extractor maps + // considers it still as `Public`. We hide the message for now given the Angular + // repositories do not follow the TSDoc standard. https://tsdoc.org/. + // TODO: Make this an error once TSDoc standard is followed in all projects. + [ExtractorMessageId.MissingReleaseTag]: {logLevel: ExtractorLogLevel.None}, + }, + }, + }; + + // We read the specified `package.json` manually and build a package name that is + // compatible with the API extractor. This is a workaround for a bug in api-extractor. + // TODO remove once https://github.com/microsoft/rushstack/issues/2774 is resolved. + const packageJson = require(packageJsonPath); + const packageNameSegments = packageJson.name.split('/'); + const packageName = packageNameSegments.length === 1 ? + packageNameSegments[0] : + `${packageNameSegments[0]}/${packageNameSegments.slice(1).join('_')}`; + + const extractorConfig = ExtractorConfig.prepare({ + configObject, + // TODO: Remove workaround once https://github.com/microsoft/rushstack/issues/2774 is fixed. + packageJson: {name: packageName}, + packageJsonFullPath: packageJsonPath, + configObjectFullPath: undefined, + }); + + // This patches the `ExportAnalyzer` of `api-extractor` so that we can filter out + // exports that match a specified pattern. Ideally this would not be needed as the + // TSDoc JSDoc annotations could be used to filter out exports from the API report, + // but there are cases in Angular where exports cannot be `@internal` but at the same + // time are denoted as unstable. Such exports are allowed to change frequently and should + // not be captured in the API report (as this would be unnecessarily inconvenient). + ExportAnalyzer.prototype.fetchAstModuleExportInfo = function(module: AstModule) { + const info = _origFetchAstModuleExportInfo.apply(this, [module]); + + info.exportedLocalEntities.forEach((entity, exportName) => { + if (stripExportPattern.test(exportName)) { + info.exportedLocalEntities.delete(exportName); + } + }); + + return info; + }; + + return Extractor.invoke(extractorConfig, { + // If the golden should be approved, then `localBuild: true` instructs + // API extractor to update the file. + localBuild: approveGolden, + // Process messages from the API extractor (and modify log levels if needed). + messageCallback: msg => processExtractorMessage(msg, approveGolden), + }); +} + +/** + * Process an API extractor message. Microsoft's API extractor allows developers to + * handle messages before API extractor prints them. This allows us to adjust log level + * for certain messages, or to fully prevent messages from being printed out. + * */ +async function processExtractorMessage(message: ExtractorMessage, isApprove: boolean) { + // If the golden does not match, we hide the error as API extractor prints + // a warning asking the user to manually copy the new API report. We print + // a custom warning below asking the developer to run the `.accept` Bazel target. + // TODO: Simplify once https://github.com/microsoft/rushstack/issues/2773 is resolved. + if (message.messageId === ConsoleMessageId.ApiReportNotCopied) { + // Mark the message as handled so that API-extractor does not print it. We print + // a message manually after extraction. + message.handled = true; + message.logLevel = isApprove ? ExtractorLogLevel.None : ExtractorLogLevel.Error; + } +} + +/** Resolves the `package.json` of the workspace executing this action. */ +function resolveWorkspacePackageJsonPath(): string { + const workspaceName = process.env.BAZEL_WORKSPACE!; + return runfiles.resolve(`${workspaceName}/package.json`); +} diff --git a/dev-infra/tmpl-package.json b/dev-infra/tmpl-package.json index bef95947e0..3fb79079ff 100644 --- a/dev-infra/tmpl-package.json +++ b/dev-infra/tmpl-package.json @@ -10,11 +10,15 @@ }, "dependencies": { "@angular/benchpress": "0.2.1", + "@bazel/buildifier": "", + "@bazel/runfiles": "", + "@microsoft/api-extractor": "", "@octokit/graphql": "", - "@octokit/types": "", "@octokit/rest": "", + "@octokit/types": "", "brotli": "", "chalk": "", + "clang-format": "", "cli-progress": "", "conventional-commits-parser": "", "ejs": "", @@ -26,18 +30,16 @@ "node-fetch": "", "node-uuid": "", "ora": "", - "semver": "", - "shelljs": "", - "tslib": "", - "typed-graphqlify": "", - "yaml": "", - "yargs": "", - "@bazel/buildifier": "", - "clang-format": "", "protractor": "", "selenium-webdriver": "", + "semver": "", + "shelljs": "", "ts-node": "", - "typescript": "" + "tslib": "", + "typed-graphqlify": "", + "typescript": "", + "yaml": "", + "yargs": "" }, "peerDependenciesMeta": { "@bazel/buildifier": { diff --git a/dev-infra/tsconfig.json b/dev-infra/tsconfig.json index aee0ec940f..a107894e33 100644 --- a/dev-infra/tsconfig.json +++ b/dev-infra/tsconfig.json @@ -1,5 +1,6 @@ { "compilerOptions": { - "strict": true + "strict": true, + "downlevelIteration": true } }