From 3d5f67b65dc1234f5879819d4ae043c0dc01e573 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 10 Oct 2022 16:06:37 -0700 Subject: [PATCH 1/3] fix(policy-check): Make check policy continue after errors --- .../build-cli/src/commands/check/policy.ts | 85 +++++++++---------- 1 file changed, 41 insertions(+), 44 deletions(-) diff --git a/build-tools/packages/build-cli/src/commands/check/policy.ts b/build-tools/packages/build-cli/src/commands/check/policy.ts index 8883bc1d36a1..c98c8ae3d378 100644 --- a/build-tools/packages/build-cli/src/commands/check/policy.ts +++ b/build-tools/packages/build-cli/src/commands/check/policy.ts @@ -12,7 +12,7 @@ import { policyHandlers, readJsonAsync } from "@fluidframework/build-tools"; import { BaseCommand } from "../../base"; -const readStdin: () => Promise = async () => { +async function readStdin(): Promise { return new Promise((resolve) => { const stdin = process.openStdin(); stdin.setEncoding("utf-8"); @@ -94,19 +94,19 @@ export class CheckPolicy extends BaseCommand { : new RegExp(this.processedFlags.path, "i"); if (this.processedFlags.handler !== undefined) { - this.log(`Filtering handlers by regex: ${handlerRegex}`); + this.info(`Filtering handlers by regex: ${handlerRegex}`); } if (this.processedFlags.path !== undefined) { - this.log(`Filtering file paths by regex: ${pathRegex}`); + this.info(`Filtering file paths by regex: ${pathRegex}`); } if (this.processedFlags.fix) { - this.log("Resolving errors if possible."); + this.info("Resolving errors if possible."); } if (this.processedFlags.exclusions === undefined) { - this.error("ERROR: No exclusions file provided."); + this.error("No exclusions file provided."); } let exclusionsFile: string[]; @@ -174,49 +174,46 @@ export class CheckPolicy extends BaseCommand { // route files to their handlers by regex testing their full paths // synchronize output, exit code, and resolve decision for all handlers - routeToHandlers(file: string, handlerRegex: RegExp) { - const filteredHandlers = policyHandlers.filter( - (handler) => handler.match.test(file) && handlerRegex.test(handler.name), - ); - - for (const handler of filteredHandlers) { - const result = runWithPerf(handler.name, "handle", () => - handler.handler(file, CheckPolicy.pathToGitRoot), - ); - const resolver = handler.resolver; - - if (result === undefined) { - return; - } - - if (!this.processedFlags.fix || resolver === undefined) { - return this.exit(1); - } - - let output = `${newline}file failed policy check: ${file}${newline}${result}`; - - output += `${newline}attempting to resolve: ${file}`; - const resolveResult = runWithPerf(handler.name, "resolve", () => - resolver(file, CheckPolicy.pathToGitRoot), - ); - - // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions - if (resolveResult.message) { - output += newline + resolveResult.message; - } - - if (!resolveResult.resolved) { - return this.exit(1); - } + routeToHandlers(file: string, handlerRegex: RegExp): void { + policyHandlers + .filter((handler) => handler.match.test(file) && handlerRegex.test(handler.name)) + // eslint-disable-next-line unicorn/no-array-for-each + .forEach((handler) => { + const result = runWithPerf(handler.name, "handle", () => + handler.handler(file, CheckPolicy.pathToGitRoot), + ); + if (result !== undefined && result !== "") { + let output = `${newline}file failed policy check: ${file}${newline}${result}`; + const resolver = handler.resolver; + if (this.processedFlags.fix && resolver) { + output += `${newline}attempting to resolve: ${file}`; + const resolveResult = runWithPerf(handler.name, "resolve", () => + resolver(file, CheckPolicy.pathToGitRoot), + ); - this.log(output); - } + if (resolveResult?.message !== undefined) { + output += newline + resolveResult.message; + } + + if (!resolveResult.resolved) { + process.exitCode = 1; + } + } else { + process.exitCode = 1; + } + + if(process.exitCode === 1) { + this.warning(output); + } else { + this.info(output); + } + } + }); } logStats() { this.log( - `Statistics: ${CheckPolicy.processed} processed, ${ - CheckPolicy.count - CheckPolicy.processed + `Statistics: ${CheckPolicy.processed} processed, ${CheckPolicy.count - CheckPolicy.processed } excluded, ${CheckPolicy.count} total`, ); for (const [action, handlerPerf] of CheckPolicy.handlerActionPerf.entries()) { @@ -236,7 +233,7 @@ export class CheckPolicy extends BaseCommand { CheckPolicy.count++; if (!exclusions.every((value) => !value.test(line))) { - this.log(`Excluded: ${line}`); + this.verbose(`Excluded: ${line}`); return; } From 2eb53e09d87bf2d32e494ad05a63db46e0db97e4 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 10 Oct 2022 16:09:48 -0700 Subject: [PATCH 2/3] unneeded change --- build-tools/packages/build-cli/src/commands/check/policy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tools/packages/build-cli/src/commands/check/policy.ts b/build-tools/packages/build-cli/src/commands/check/policy.ts index c98c8ae3d378..5eb319f9bfe1 100644 --- a/build-tools/packages/build-cli/src/commands/check/policy.ts +++ b/build-tools/packages/build-cli/src/commands/check/policy.ts @@ -12,7 +12,7 @@ import { policyHandlers, readJsonAsync } from "@fluidframework/build-tools"; import { BaseCommand } from "../../base"; -async function readStdin(): Promise { +const readStdin: () => Promise = async () => { return new Promise((resolve) => { const stdin = process.openStdin(); stdin.setEncoding("utf-8"); From 24abed3b92c0b07ca76686c677434f6c2048ceb0 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 10 Oct 2022 16:26:06 -0700 Subject: [PATCH 3/3] formatting --- build-tools/packages/build-cli/src/commands/check/policy.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/build-tools/packages/build-cli/src/commands/check/policy.ts b/build-tools/packages/build-cli/src/commands/check/policy.ts index 5eb319f9bfe1..2a8ac42b5c04 100644 --- a/build-tools/packages/build-cli/src/commands/check/policy.ts +++ b/build-tools/packages/build-cli/src/commands/check/policy.ts @@ -202,7 +202,7 @@ export class CheckPolicy extends BaseCommand { process.exitCode = 1; } - if(process.exitCode === 1) { + if (process.exitCode === 1) { this.warning(output); } else { this.info(output); @@ -213,7 +213,8 @@ export class CheckPolicy extends BaseCommand { logStats() { this.log( - `Statistics: ${CheckPolicy.processed} processed, ${CheckPolicy.count - CheckPolicy.processed + `Statistics: ${CheckPolicy.processed} processed, ${ + CheckPolicy.count - CheckPolicy.processed } excluded, ${CheckPolicy.count} total`, ); for (const [action, handlerPerf] of CheckPolicy.handlerActionPerf.entries()) {