From 14c0ec97d860fb3e3f205d0c25d49e3dcf50aae4 Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Fri, 8 May 2020 14:51:29 -0700 Subject: [PATCH] feat(dev-infra): introduce validators for ng-dev config loading (#37049) Introduces infrastructure to validate configuration of the ng-dev command at run time. Allowing for errors to be returned to the user running the command. PR Close #37049 --- dev-infra/commit-message/config.ts | 18 ++++++ dev-infra/commit-message/validate.spec.ts | 6 +- dev-infra/commit-message/validate.ts | 5 +- dev-infra/format/config.ts | 46 ++++++++++++-- dev-infra/format/formatters/index.ts | 9 +-- dev-infra/utils/BUILD.bazel | 5 +- dev-infra/utils/config.ts | 76 ++++++++++++++++------- 7 files changed, 119 insertions(+), 46 deletions(-) diff --git a/dev-infra/commit-message/config.ts b/dev-infra/commit-message/config.ts index 4085e809b1..a0d484bd3a 100644 --- a/dev-infra/commit-message/config.ts +++ b/dev-infra/commit-message/config.ts @@ -5,9 +5,27 @@ * 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 {assertNoErrors, getConfig, NgDevConfig} from '../utils/config'; + export interface CommitMessageConfig { maxLineLength: number; minBodyLength: number; types: string[]; scopes: string[]; } + +/** Retrieve and validate the config as `CommitMessageConfig`. */ +export function getCommitMessageConfig() { + // List of errors encountered validating the config. + const errors: string[] = []; + // The unvalidated config object. + const config: Partial> = getConfig(); + + if (config.commitMessage === undefined) { + errors.push(`No configuration defined for "commitMessage"`); + } + + assertNoErrors(errors); + return config as Required; +} diff --git a/dev-infra/commit-message/validate.spec.ts b/dev-infra/commit-message/validate.spec.ts index 5017ea15f6..9027b1d797 100644 --- a/dev-infra/commit-message/validate.spec.ts +++ b/dev-infra/commit-message/validate.spec.ts @@ -7,11 +7,9 @@ */ // Imports -import * as utilConfig from '../utils/config'; - +import * as validateConfig from './config'; import {validateCommitMessage} from './validate'; - // Constants const config = { 'commitMessage': { @@ -46,7 +44,7 @@ describe('validate-commit-message.js', () => { lastError = ''; spyOn(console, 'error').and.callFake((msg: string) => lastError = msg); - spyOn(utilConfig, 'getAngularDevConfig').and.returnValue(config); + spyOn(validateConfig, 'getCommitMessageConfig').and.returnValue(config); }); describe('validateMessage()', () => { diff --git a/dev-infra/commit-message/validate.ts b/dev-infra/commit-message/validate.ts index f8a62073f6..fb9a5be664 100644 --- a/dev-infra/commit-message/validate.ts +++ b/dev-infra/commit-message/validate.ts @@ -5,8 +5,7 @@ * 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 {getAngularDevConfig} from '../utils/config'; -import {CommitMessageConfig} from './config'; +import {getCommitMessageConfig} from './config'; /** Options for commit message validation. */ export interface ValidateCommitMessageOptions { @@ -76,7 +75,7 @@ export function validateCommitMessage( `(): \n\n`); } - const config = getAngularDevConfig<'commitMessage', CommitMessageConfig>().commitMessage; + const config = getCommitMessageConfig().commitMessage; const commit = parseCommitMessage(commitMsg); //////////////////////////////////// diff --git a/dev-infra/format/config.ts b/dev-infra/format/config.ts index 864f26aae2..cd5f3beba2 100644 --- a/dev-infra/format/config.ts +++ b/dev-infra/format/config.ts @@ -6,8 +6,46 @@ * found in the LICENSE file at https://angular.io/license */ -export interface FormatConfig { - [keyof: string]: boolean|{ - matchers: string[]; - }; +import {assertNoErrors, getConfig, NgDevConfig} from '../utils/config'; + +interface Formatter { + matchers: string[]; +} + +export interface FormatConfig { + [keyof: string]: boolean|Formatter; +} + +/** Retrieve and validate the config as `FormatConfig`. */ +export function getFormatConfig() { + // List of errors encountered validating the config. + const errors: string[] = []; + // The unvalidated config object. + const config: Partial> = getConfig(); + + if (config.format === undefined) { + errors.push(`No configuration defined for "format"`); + } + + for (const [key, value] of Object.entries(config.format!)) { + switch (typeof value) { + case 'boolean': + break; + case 'object': + checkFormatterConfig(key, value, errors); + break; + default: + errors.push(`"format.${key}" is not a boolean or Formatter object`); + } + } + + assertNoErrors(errors); + return config as Required; +} + +/** Validate an individual Formatter config. */ +function checkFormatterConfig(key: string, config: Partial, errors: string[]) { + if (config.matchers === undefined) { + errors.push(`Missing "format.${key}.matchers" value`); + } } diff --git a/dev-infra/format/formatters/index.ts b/dev-infra/format/formatters/index.ts index fe82cc5f07..6b207d10f3 100644 --- a/dev-infra/format/formatters/index.ts +++ b/dev-infra/format/formatters/index.ts @@ -6,8 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {getAngularDevConfig} from '../../utils/config'; -import {FormatConfig} from '../config'; +import {getFormatConfig} from '../config'; import {Buildifier} from './buildifier'; import {ClangFormat} from './clang-format'; @@ -16,11 +15,7 @@ import {ClangFormat} from './clang-format'; * Get all defined formatters which are active based on the current loaded config. */ export function getActiveFormatters() { - let config = {}; - try { - config = getAngularDevConfig<'format', FormatConfig>().format || {}; - } catch { - } + const config = getFormatConfig().format; return [new Buildifier(config), new ClangFormat(config)].filter( formatter => formatter.isEnabled()); } diff --git a/dev-infra/utils/BUILD.bazel b/dev-infra/utils/BUILD.bazel index 812c20ce99..0610a5469f 100644 --- a/dev-infra/utils/BUILD.bazel +++ b/dev-infra/utils/BUILD.bazel @@ -2,10 +2,7 @@ load("@npm_bazel_typescript//:index.bzl", "ts_library") ts_library( name = "utils", - srcs = [ - "config.ts", - "repo-files.ts", - ], + srcs = glob(["*.ts"]), module_name = "@angular/dev-infra-private/utils", visibility = ["//dev-infra:__subpackages__"], deps = [ diff --git a/dev-infra/utils/config.ts b/dev-infra/utils/config.ts index b7e0079082..f6e70e7649 100644 --- a/dev-infra/utils/config.ts +++ b/dev-infra/utils/config.ts @@ -9,13 +9,64 @@ import {join} from 'path'; import {exec} from 'shelljs'; +/** The common configuration for ng-dev. */ +type CommonConfig = { + // TODO: add common configuration +}; + +/** + * The configuration for the specific ng-dev command, providing both the common + * ng-dev config as well as the specific config of a subcommand. + */ +export type NgDevConfig = CommonConfig&T; + // The filename expected for creating the ng-dev config, without the file // extension to allow either a typescript or javascript file to be used. const CONFIG_FILE_NAME = '.ng-dev-config'; +/** The configuration for ng-dev. */ +let CONFIG: {}|null = null; + /** - * Gets the path of the directory for the repository base. + * Get the configuration from the file system, returning the already loaded copy if it + * is defined. */ +export function getConfig(): NgDevConfig { + // If the global config is not defined, load it from the file system. + if (CONFIG === null) { + // The full path to the configuration file. + const configPath = join(getRepoBaseDir(), CONFIG_FILE_NAME); + // Set the global config object to a clone of the configuration loaded through default exports + // from the config file. + CONFIG = {...require(configPath)}; + } + // Return a clone of the global config to ensure that a new instance of the config is returned + // each time, preventing unexpected effects of modifications to the config object. + return validateCommonConfig({...CONFIG}); +} + +/** Validate the common configuration has been met for the ng-dev command. */ +function validateCommonConfig(config: NgDevConfig) { + // TODO: add validation for the common configuration + return config; +} + +/** + * Asserts the provided array of error messages is empty. If any errors are in the array, + * logs the errors and exit the process as a failure. + */ +export function assertNoErrors(errors: string[]) { + if (errors.length == 0) { + return; + } + console.error(`Errors discovered while loading configuration file:`); + for (const error of errors) { + console.error(` - ${error}`); + } + process.exit(1); +} + +/** Gets the path of the directory for the repository base. */ export function getRepoBaseDir() { const baseRepoDir = exec(`git rev-parse --show-toplevel`, {silent: true}); if (baseRepoDir.code) { @@ -26,26 +77,3 @@ export function getRepoBaseDir() { } return baseRepoDir.trim(); } - -/** - * Retrieve the configuration from the .ng-dev-config.js file. - */ -export function getAngularDevConfig(supressError = false): DevInfraConfig { - const configPath = join(getRepoBaseDir(), CONFIG_FILE_NAME); - try { - return require(configPath) as DevInfraConfig; - } catch (err) { - if (!supressError) { - throw Error(`Unable to load config file at:\n ${configPath}`); - } - } - return {} as DevInfraConfig; -} - -/** - * Interface exressing the expected structure of the DevInfraConfig. - * Allows for providing a typing for a part of the config to read. - */ -export interface DevInfraConfig { - [K: string]: T; -}