From fcd4740eee65d2fe1a4161dbfaabe1bc0b4be898 Mon Sep 17 00:00:00 2001
From: ssube <seansube@gmail.com>
Date: Sat, 2 Nov 2019 14:58:49 -0500
Subject: [PATCH] fix(visitor): include rule name and selector in error
 messages

---
 README.md                            | 26 ++++++++++++++++++++++++--
 src/config/args.ts                   |  2 +-
 src/rule/SchemaRule.ts               |  2 +-
 src/utils/ajv/index.ts               | 13 ++++++++-----
 test/utils/ajv/TestFriendlyErrors.ts | 25 ++++++++++++++++++++++---
 5 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/README.md b/README.md
index 3b40cca..9aff5fe 100644
--- a/README.md
+++ b/README.md
@@ -197,8 +197,30 @@ the easiest to read, and can be pretty-printed by redirecting `stderr` through `
 }
 ```
 
-Using `jq` allows for additional filtering, for example `>(jq 'select(.level > 30)')` will only print warnings and
-errors (log level is also part of the configuration file).
+Using `jq` allows for additional filtering and formatting. For example, `>(jq 'select(.level > 30)')` will only print
+warnings and errors (log level is also part of the configuration file).
+
+To print the last line's message and error messages: `>(tail -1 | jq '[.msg, ((.errors // [])[] | .msg)]')`
+
+```shell
+> cat test/examples/kubernetes-resources-high.yml | salty-dog \
+    --rules rules/kubernetes.yml \
+    --tag kubernetes 2> >(tail -1 | jq '[.msg, ((.errors // [])[] | .msg)]')
+
+[
+  "all rules passed"
+]
+
+> cat test/examples/kubernetes-resources-some.yml | salty-dog \
+    --rules rules/kubernetes.yml \
+    --tag kubernetes 2> >(tail -1 | jq '[.msg, ((.errors // [])[] | .msg)]')
+
+[
+  "some rules failed",
+  ".resources.limits should have required property 'memory' at $.spec.template.spec.containers[*] for kubernetes-resources",
+  ".metadata should have required property 'labels' at $ for kubernetes-labels"
+]
+```
 
 ### Modes
 
diff --git a/src/config/args.ts b/src/config/args.ts
index 4131f18..18797bc 100644
--- a/src/config/args.ts
+++ b/src/config/args.ts
@@ -156,7 +156,7 @@ export async function parseArgs(argv: Array<string>): Promise<ParseResults> {
     .version(VERSION_INFO.package.version)
     .alias('version', 'v');
 
-  // @TODO: this should not need a cast but the parser's type only has the last option (include-tag)
+  // @TODO: this should not need a cast but the parser's type omits command options and doesn't expose camelCase
   // tslint:disable-next-line:no-any
   const args = parser.argv as any;
   return {
diff --git a/src/rule/SchemaRule.ts b/src/rule/SchemaRule.ts
index 5209ca4..64b82cf 100644
--- a/src/rule/SchemaRule.ts
+++ b/src/rule/SchemaRule.ts
@@ -57,7 +57,7 @@ export class SchemaRule implements RuleData, Visitor {
     if (filter(node)) {
       ctx.logger.debug({ item: node }, 'checking item');
       if (!check(node) && hasItems(check.errors)) {
-        errors.push(...check.errors.map(friendlyError));
+        errors.push(...check.errors.map((err) => friendlyError(ctx, err, this)));
       }
     } else {
       ctx.logger.debug({ errors: filter.errors, item: node }, 'skipping item');
diff --git a/src/utils/ajv/index.ts b/src/utils/ajv/index.ts
index ff02b66..80d6b58 100644
--- a/src/utils/ajv/index.ts
+++ b/src/utils/ajv/index.ts
@@ -1,22 +1,25 @@
 import { ErrorObject } from 'ajv';
 import { isNil } from 'lodash';
 
+import { SchemaRule } from '../../rule/SchemaRule';
+import { VisitorContext } from '../../visitor/VisitorContext';
 import { VisitorError } from '../../visitor/VisitorError';
 
-export function friendlyError(err: ErrorObject): VisitorError {
+export function friendlyError(ctx: VisitorContext, err: ErrorObject, rule: SchemaRule): VisitorError {
   return {
     data: {
       err,
+      rule,
     },
     level: 'error',
-    msg: friendlyErrorMessage(err),
+    msg: friendlyErrorMessage(err, rule),
   };
 }
 
-export function friendlyErrorMessage(err: ErrorObject): string {
+export function friendlyErrorMessage(err: ErrorObject, rule: SchemaRule): string {
   if (isNil(err.message)) {
-    return `${err.dataPath} ${err.keyword}`;
+    return `${err.dataPath} ${err.keyword} at ${rule.select} for ${rule.name}`;
   } else {
-    return `${err.dataPath} ${err.message}`;
+    return `${err.dataPath} ${err.message} at ${rule.select} for ${rule.name}`;
   }
 }
diff --git a/test/utils/ajv/TestFriendlyErrors.ts b/test/utils/ajv/TestFriendlyErrors.ts
index 481c0b8..47ad058 100644
--- a/test/utils/ajv/TestFriendlyErrors.ts
+++ b/test/utils/ajv/TestFriendlyErrors.ts
@@ -1,15 +1,34 @@
 import { expect } from 'chai';
+import { NullLogger } from 'noicejs';
 
+import { SchemaRule } from '../../../src/rule/SchemaRule';
 import { friendlyError } from '../../../src/utils/ajv';
+import { VisitorContext } from '../../../src/visitor/VisitorContext';
+
+const TEST_NAME = 'test';
 
 describe('friendly errors', () => {
   it('should have a message', () => {
-    const err = friendlyError({
+    const err = friendlyError(new VisitorContext({
+      innerOptions: {
+        coerce: false,
+        defaults: false,
+        mutate: false,
+      },
+      logger: NullLogger.global,
+    }), {
       dataPath: 'test-path',
-      keyword: 'test',
+      keyword: TEST_NAME,
       params: { /* ? */ },
       schemaPath: 'test-path',
-    });
+    }, new SchemaRule({
+      check: {},
+      desc: TEST_NAME,
+      level: 'info',
+      name: TEST_NAME,
+      select: '',
+      tags: [TEST_NAME],
+    }));
     expect(err.msg).to.not.equal('');
   });
 });