Support warnings and VSO output

change/@boll-cli-2020-12-14-12-59-47-warnings-enabled.json

@@ -0,0 +1,8 @@
+{
+  "type": "minor",
+  "comment": "CLI supports warnings and VSO output",
+  "packageName": "@boll/cli",
+  "email": "jdh@microsoft.com",
+  "dependentChangeType": "patch",
+  "date": "2020-12-14T20:59:47.543Z"
+}

change/@boll-core-2020-12-14-12-22-32-warnings-enabled.json

@@ -0,0 +1,8 @@
+{
+  "type": "minor",
+  "comment": "Configurable warnings",
+  "packageName": "@boll/core",
+  "email": "jdh@microsoft.com",
+  "dependentChangeType": "patch",
+  "date": "2020-12-14T20:22:32.767Z"
+}

packages/cli/src/bin/run.ts

@@ -1,11 +1,10 @@
 import { Cli, Status } from "../cli";
 import { DefaultLogger } from "@boll/core";
 
-const cli = new Cli(DefaultLogger);
 async function doStuff() {
+  const cli = new Cli(DefaultLogger);
   const result = await cli.run(process.argv.slice(2));
-  if (result !== Status.Ok) {
-    console.error("@boll/cli detected lint errors");
+  if (result === Status.Error) {
     process.exit(1);
   }
 }

packages/cli/src/cli.ts

@@ -1,12 +1,16 @@
 import * as fs from "fs";
 import { ConfigGenerator } from "./config-generator";
-import { ArgumentParser } from "argparse";
+import { ArgumentDefaultsHelpFormatter, ArgumentParser } from "argparse";
 import { Config, configFileName, ConfigRegistryInstance, Logger, RuleRegistryInstance, Suite } from "@boll/core";
 import { promisify } from "util";
 import { resolve } from "path";
+import { Formatter } from "./lib/formatter";
+import { DefaultFormatter } from "./lib/default-formatter";
+import { VsoFormatter } from "./lib/vso-formatter";
 const fileExistsAsync = promisify(fs.exists);
 
 const parser = new ArgumentParser({ description: "@boll/cli" });
+parser.addArgument("--azure-devops", { help: "Enable Azure DevOps pipeline output formatter.", action: "storeTrue" });
 const subParser = parser.addSubparsers({
   description: "commands",
   dest: "command"
@@ -15,12 +19,14 @@ subParser.addParser("run");
 subParser.addParser("init");
 
 type ParsedCommand = {
+  azure_devops: boolean;
   command: "run" | "init";
 };
 
 export enum Status {
   Ok,
-  Error
+  Error,
+  Warn
 }
 
 export class Cli {
@@ -28,15 +34,24 @@ export class Cli {
 
   async run(args: string[]): Promise<Status> {
     const parsedCommand: ParsedCommand = parser.parseArgs(args);
+    const formatter: Formatter = parsedCommand.azure_devops ? new VsoFormatter() : new DefaultFormatter();
     if (parsedCommand.command === "run") {
       const suite = await this.buildSuite();
       const result = await suite.run(this.logger);
       result.errors.forEach(e => {
-        this.logger.error(e.formattedMessage);
+        this.logger.error(formatter.error(e.formattedMessage));
+      });
+      result.warnings.forEach(e => {
+        this.logger.warn(formatter.warn(e.formattedMessage));
       });
       if (result.hasErrors) {
+        this.logger.error(formatter.finishWithErrors());
         return Status.Error;
       }
+      if (result.hasWarnings) {
+        this.logger.warn(formatter.finishWithWarnings());
+        return Status.Warn;
+      }
       return Status.Ok;
     }
     if (parsedCommand.command === "init") {

packages/cli/src/lib/default-formatter.ts

@@ -0,0 +1,19 @@
+import { Formatter } from "./formatter";
+
+export class DefaultFormatter implements Formatter {
+  finishWithErrors(): string {
+    return "";
+  }
+  finishWithWarnings(): string {
+    return "";
+  }
+  warn(str: string): string {
+    return str;
+  }
+  info(str: string): string {
+    return str;
+  }
+  error(str: string): string {
+    return str;
+  }
+}

packages/cli/src/lib/formatter.ts

@@ -0,0 +1,7 @@
+export interface Formatter {
+  finishWithErrors(): string;
+  finishWithWarnings(): string;
+  warn(str: string): string;
+  info(str: string): string;
+  error(str: string): string;
+}

packages/cli/src/lib/vso-formatter.ts

@@ -0,0 +1,19 @@
+import { Formatter } from "./formatter";
+
+export class VsoFormatter implements Formatter {
+  finishWithErrors(): string {
+    return "##vso[task.complete result=Failed;]There were failures";
+  }
+  finishWithWarnings(): string {
+    return "##vso[task.complete result=SucceededWithIssues;]There were warnings";
+  }
+  warn(str: string): string {
+    return `##[warning]${str}`;
+  }
+  info(str: string): string {
+    return str;
+  }
+  error(str: string): string {
+    return `##[error]${str}`;
+  }
+}

packages/core/src/config.ts

@@ -2,7 +2,7 @@ import { ConfigDefinition, FileGlob, PackageRule } from "./types";
 import { ConfigRegistry } from "./config-registry";
 import { Logger } from "./logger";
 import { RuleRegistry } from "./rule-registry";
-import { RuleSet } from "./rule-set";
+import { InstantiatedPackageRule, RuleSet } from "./rule-set";
 import { Suite } from "./suite";
 import { IgnoredFiles } from "./ignore";
 import { getRepoRoot } from "./git-utils";
@@ -39,7 +39,8 @@ export class Config {
         const optionsFromConfig =
           (config.configuration && config.configuration.rules && (config.configuration.rules as any)[check.rule]) || {};
         const options = { ...check.options, ...optionsFromConfig };
-        return this.ruleRegistry.get(check.rule)(this.logger, options);
+        const rule = this.ruleRegistry.get(check.rule)(this.logger, options);
+        return new InstantiatedPackageRule(rule.name, check.severity || "error", rule);
       });
       return new RuleSet(glob, checks);
     });

packages/core/src/result-set.ts

@@ -33,16 +33,29 @@ export class Failure implements Result {
 
 export class ResultSet {
   errors: Result[] = [];
+  warnings: Result[] = [];
 
   get hasErrors(): boolean {
     return this.errors.length > 0;
   }
 
-  add(results: Result[]) {
+  get hasWarnings(): boolean {
+    return this.warnings.length > 0;
+  }
+
+  addErrors(results: Result[]) {
     results.forEach(result => {
       if (result.status === ResultStatus.failure) {
         this.errors.push(result);
       }
     });
   }
+
+  addWarnings(results: Result[]) {
+    results.forEach(result => {
+      if (result.status === ResultStatus.failure) {
+        this.warnings.push(result);
+      }
+    });
+  }
 }

packages/core/src/rule-set.ts

@@ -1,5 +1,15 @@
-import { FileGlob, PackageRule } from "./types";
+import { FileContext } from "./file-context";
+import { Result } from "./result-set";
+import { CheckSeverity, FileGlob, PackageRule } from "./types";
+
+export class InstantiatedPackageRule {
+  constructor(public name: string, public severity: CheckSeverity, public rule: PackageRule) {}
+
+  check(file: FileContext): Promise<Result[]> {
+    return this.rule.check(file);
+  }
+}
 
 export class RuleSet {
-  constructor(public fileGlob: FileGlob, public checks: PackageRule[]) {}
+  constructor(public fileGlob: FileGlob, public checks: InstantiatedPackageRule[]) {}
 }

packages/core/src/suite.ts

@@ -39,8 +39,14 @@ export class Suite {
       sourceFiles.forEach(async s => {
         if (s.shouldSkip(r)) return;
         const results = await r.check(s);
-        const filterResults = await this.filterIgnoredChecksByLine(results, s);
-        resultSet.add(filterResults);
+        const filteredResults = await this.filterIgnoredChecksByLine(results, s);
+        if (r.severity === "error") {
+          resultSet.addErrors(filteredResults);
+        } else if (r.severity === "warn") {
+          resultSet.addWarnings(filteredResults);
+        } else {
+          throw new Error("Unknown severity! (This is likely a boll bug)");
+        }
       });
     });
     return true;

packages/core/src/tests/all.test.ts

@@ -6,6 +6,7 @@ import { test as GitUtilsTest } from "./git-utils.test";
 import { test as GlobTest } from "./glob.test";
 import { test as IgnoreTest } from "./ignore.test";
 import { test as PragmaTest } from "./pragma.test";
+import { test as ResultTest } from "./result.test";
 import { test as SuiteTest } from "./suite.test";
 
-suite(ConfigTest, FormatTest, GitUtilsTest, GlobTest, IgnoreTest, PragmaTest, SuiteTest);
+suite(ConfigTest, FormatTest, GitUtilsTest, GlobTest, IgnoreTest, PragmaTest, SuiteTest, ResultTest);

packages/core/src/tests/config.test.ts

@@ -93,7 +93,7 @@ test("downstream rules configuration applies to rules", async () => {
   };
   config.load(myConfig);
   const suite = await config.buildSuite();
-  const fakeRuleInstance = suite.ruleSets[0].checks[0] as FakeRule;
+  const fakeRuleInstance = suite.ruleSets[0].checks[0].rule as FakeRule;
   assert.deepStrictEqual(fakeRuleInstance.options, { bar: "baz", some: "rule" });
 });
 

packages/core/src/tests/result.test.ts

@@ -0,0 +1,78 @@
+import * as assert from "assert";
+import baretest from "baretest";
+import { Config } from "../config";
+import { ConfigRegistry } from "../config-registry";
+import { CheckConfiguration, PackageRule } from "../types";
+import { NullLogger } from "../logger";
+import { Result } from "../result-set";
+import { RuleRegistry } from "../rule-registry";
+import { Failure } from "../result-set";
+import { asBollLineNumber } from "../boll-line-number";
+import { inFixtureDir } from "@boll/test-internal";
+import { TypescriptSourceGlob } from "../glob";
+import { FileContext } from "../file-context";
+
+export const test: any = baretest("Suite result");
+
+class FakeFile {}
+
+class FakeRule implements PackageRule {
+  name: string = "fakerule";
+
+  constructor(public options: {} = {}) {}
+
+  async check(file: FileContext): Promise<Result[]> {
+    return [new Failure(this.name, file.filename, asBollLineNumber(0), "Something went wrong.")];
+  }
+}
+
+test("should log a failure as an error by default", async () => {
+  await inFixtureDir("project-a", __dirname, async () => {
+    const ruleRegistry = new RuleRegistry();
+    ruleRegistry.register("foo", (l: any, options: any) => {
+      return new FakeRule(options);
+    });
+    const config = new Config(new ConfigRegistry(), ruleRegistry, NullLogger);
+    const myConfig = {
+      ruleSets: [{ fileLocator: new TypescriptSourceGlob(), checks: [{ rule: "foo", options: { bar: "baz" } }] }],
+      configuration: {
+        rules: {
+          foo: { some: "rule" }
+        }
+      }
+    };
+    config.load(myConfig);
+    const suite = await config.buildSuite();
+    const results = await suite.run(NullLogger);
+    assert.strictEqual(results.warnings.length, 0);
+    assert.strictEqual(results.errors.length, 2);
+  });
+});
+
+test("should log a failure as a warning if configured in the rule", async () => {
+  await inFixtureDir("project-a", __dirname, async () => {
+    const ruleRegistry = new RuleRegistry();
+    ruleRegistry.register("foo", (l: any, options: any) => {
+      return new FakeRule(options);
+    });
+    const config = new Config(new ConfigRegistry(), ruleRegistry, NullLogger);
+    const myConfig = {
+      ruleSets: [
+        {
+          fileLocator: new TypescriptSourceGlob(),
+          checks: [{ severity: "warn", rule: "foo", options: { bar: "baz" } } as CheckConfiguration]
+        }
+      ],
+      configuration: {
+        rules: {
+          foo: { some: "rule" }
+        }
+      }
+    };
+    config.load(myConfig);
+    const suite = await config.buildSuite();
+    const results = await suite.run(NullLogger);
+    assert.strictEqual(results.errors.length, 0);
+    assert.strictEqual(results.warnings.length, 2);
+  });
+});

packages/core/src/types.ts

@@ -4,6 +4,7 @@ import { Result } from "./result-set";
 
 export interface CheckConfiguration {
   rule: string;
+  severity?: "warn" | "error";
   options?: {};
 }
 
@@ -52,3 +53,5 @@ export interface ImportPathAndLineNumber {
   path: string;
   lineNumber: number;
 }
+
+export type CheckSeverity = "warn" | "error";