feat(rules): add item index to rule error (fixes #116)

src/rule/SchemaRule.ts

@@ -56,7 +56,7 @@ export class SchemaRule implements Rule, RuleData, Visitor {
     if (filter(node)) {
       ctx.logger.debug({ item: node }, 'checking item');
       if (!check(node) && hasItems(check.errors)) {
-        errors.push(...check.errors.map((err) => friendlyError(ctx, err, this)));
+        errors.push(...check.errors.map((err) => friendlyError(ctx, err)));
       }
     } else {
       ctx.logger.debug({ errors: filter.errors, item: node }, 'skipping item');
@@ -74,21 +74,26 @@ export class SchemaRule implements Rule, RuleData, Visitor {
   }
 }
 
-export function friendlyError(ctx: VisitorContext, err: ErrorObject, rule: SchemaRule): VisitorError {
+export function friendlyError(ctx: VisitorContext, err: ErrorObject): VisitorError {
   return {
     data: {
       err,
-      rule,
     },
     level: 'error',
-    msg: friendlyErrorMessage(err, rule),
+    msg: friendlyErrorMessage(ctx, err),
   };
 }
 
-export function friendlyErrorMessage(err: ErrorObject, rule: SchemaRule): string {
+export function friendlyErrorMessage(ctx: VisitorContext, err: ErrorObject): string {
+  const msg = [err.dataPath];
   if (isNil(err.message)) {
-    return `${err.dataPath} ${err.keyword} at ${rule.select} for ${rule.name}`;
+    msg.push(err.keyword);
   } else {
-    return `${err.dataPath} ${err.message} at ${rule.select} for ${rule.name}`;
+    msg.push(err.message);
   }
+  msg.push('at', 'item', ctx.visitData.itemIndex.toString());
+  msg.push('of', ctx.visitData.rule.select);
+  msg.push('for', ctx.visitData.rule.name);
+
+  return msg.join(' ');
 }

src/rule/index.ts

@@ -202,13 +202,18 @@ export async function resolveRules(rules: Array<Rule>, selector: RuleSelector):
 export async function visitRules(ctx: VisitorContext, rules: Array<Rule>, data: any): Promise<VisitorContext> {
   for (const rule of rules) {
     const items = await rule.pick(ctx, data);
+    let itemIndex = 0;
     for (const item of items) {
+      ctx.visitData = {
+          itemIndex,
+          rule,
+      };
       const itemResult = cloneDeep(item);
       const ruleResult = await rule.visit(ctx, itemResult);
 
       if (hasItems(ruleResult.errors)) {
         ctx.logger.warn({ count: ruleResult.errors.length, rule }, 'rule failed');
-        ctx.mergeResult(ruleResult);
+        ctx.mergeResult(ruleResult, ctx.visitData);
         continue;
       }
 
@@ -226,6 +231,8 @@ export async function visitRules(ctx: VisitorContext, rules: Array<Rule>, data:
       } else {
         ctx.logger.info({ rule: rule.name }, 'rule passed');
       }
+
+      itemIndex += 1;
     }
   }
 

src/visitor/VisitorContext.ts

@@ -25,6 +25,7 @@ export class VisitorContext implements VisitorContextOptions, VisitorResult {
   protected readonly ajv: Ajv.Ajv;
   protected readonly changeBuffer: Array<any>;
   protected readonly errorBuffer: Array<VisitorError>;
+  protected data: any;
 
   public get changes(): ReadonlyArray<any> {
     return this.changeBuffer;
@@ -64,9 +65,17 @@ export class VisitorContext implements VisitorContextOptions, VisitorResult {
     return this.ajv.compile(schema);
   }
 
-  public mergeResult(other: VisitorResult): this {
+  public mergeResult(other: VisitorResult, data: any = {}): this {
     this.changeBuffer.push(...other.changes);
-    this.errorBuffer.push(...other.errors);
+    this.errorBuffer.push(...other.errors.map((err) => {
+      return {
+        ...err,
+        data: {
+          ...err.data,
+          ...data,
+        },
+      };
+    }));
     return this;
   }
 
@@ -87,4 +96,17 @@ export class VisitorContext implements VisitorContextOptions, VisitorResult {
       return [];
     }
   }
+
+  /**
+   * store some flash data. this is very much not the right way to do it.
+   *
+   * @TODO: fix this
+   */
+  public get visitData(): any {
+    return this.data;
+  }
+
+  public set visitData(value: any) {
+    this.data = value;
+  }
 }

src/visitor/index.ts

@@ -1,3 +1,4 @@
+import { Diff } from 'deep-diff';
 import { LogLevel } from 'noicejs';
 
 import { VisitorContext } from './VisitorContext';
@@ -12,8 +13,8 @@ export interface VisitorError {
 }
 
 export interface VisitorResult {
-  changes: ReadonlyArray<any>;
-  errors: ReadonlyArray<any>;
+  changes: ReadonlyArray<Diff<any, any>>;
+  errors: ReadonlyArray<VisitorError>;
 }
 
 export interface Visitor<TResult extends VisitorResult = VisitorResult> {

test/examples/kubernetes-resources-multi.yml

@@ -0,0 +1,27 @@
+# test rules kubernetes
+# test tags kubernetes
+# test exit-status 1
+
+metadata:
+  name: example
+  labels: {}
+spec:
+  template:
+    spec:
+      containers:
+        - name: test
+          resources:
+            limits:
+              cpu: 4000m
+              memory: 5Gi
+            requests:
+              cpu: 4000m
+              memory: 5Gi
+
+        - name: other
+          resources:
+            limits:
+              cpu: 2000m
+            requests:
+              cpu: 2000m
+

test/rule/TestRule.ts

@@ -227,6 +227,7 @@ describeLeaks('rule visitor', async () => {
       changes: [],
       errors: [{
         data: {},
+        level: 'error',
         msg: 'kaboom!',
       }],
     }));

test/rule/TestSchemaRule.ts

@@ -160,26 +160,64 @@ describeLeaks('schema rule', async () => {
 
 describe('friendly errors', () => {
   it('should have a message', () => {
-    const err = friendlyError(new VisitorContext({
-      innerOptions: {
-        coerce: false,
-        defaults: false,
-        mutate: false,
-      },
-      logger: NullLogger.global,
-    }), {
-      dataPath: 'test-path',
-      keyword: TEST_NAME,
-      params: { /* ? */ },
-      schemaPath: 'test-path',
-    }, new SchemaRule({
+    const rule = new SchemaRule({
       check: {},
       desc: TEST_NAME,
       level: 'info',
       name: TEST_NAME,
       select: '',
       tags: [TEST_NAME],
-    }));
-    expect(err.msg).to.not.equal('');
+    });
+    const ctx = new VisitorContext({
+      innerOptions: {
+        coerce: false,
+        defaults: false,
+        mutate: false,
+      },
+      logger: NullLogger.global,
+    });
+    ctx.visitData = {
+      itemIndex: 0,
+      rule,
+    };
+    const err = friendlyError(ctx, {
+      dataPath: 'test-path',
+      keyword: TEST_NAME,
+      params: { /* ? */ },
+      schemaPath: 'test-path',
+    });
+    expect(err.msg).to.include(TEST_NAME);
+  });
+
+  it('should handle errors with an existing message', () => {
+    const TEST_MESSAGE = 'test-message';
+    const rule = new SchemaRule({
+      check: {},
+      desc: TEST_NAME,
+      level: 'info',
+      name: TEST_NAME,
+      select: '',
+      tags: [TEST_NAME],
+    });
+    const ctx = new VisitorContext({
+      innerOptions: {
+        coerce: false,
+        defaults: false,
+        mutate: false,
+      },
+      logger: NullLogger.global,
+    });
+    ctx.visitData = {
+      itemIndex: 0,
+      rule,
+    };
+    const err = friendlyError(ctx, {
+      dataPath: 'test-path',
+      keyword: TEST_NAME,
+      message: TEST_MESSAGE,
+      params: { /* ? */ },
+      schemaPath: 'test-path',
+    });
+    expect(err.msg).to.include(TEST_MESSAGE);
   });
 });

test/visitor/TestContext.ts

@@ -15,8 +15,17 @@ describe('visitor context', () => {
     });
 
     const nextCtx = firstCtx.mergeResult({
-      changes: [{bar: 3}],
-      errors: [{foo: 2}],
+      changes: [{
+        kind: 'N',
+        rhs: {},
+      }],
+      errors: [{
+        data: {
+          foo: 2,
+        },
+        level: 'info',
+        msg: 'uh oh',
+      }],
     });
 
     expect(nextCtx).to.equal(firstCtx);