From 0ba63822531c21113ecc273d936eed282588b206 Mon Sep 17 00:00:00 2001 From: ssube Date: Thu, 14 Nov 2019 07:56:49 -0600 Subject: [PATCH] introduce formal rule visitor --- src/app.ts | 11 +++++-- src/rule/RuleVisitor.ts | 65 +++++++++++++++++++++++++++++++++++++++++ src/rule/index.ts | 45 ++-------------------------- test/rule/TestRule.ts | 23 +++++++++++---- 4 files changed, 93 insertions(+), 51 deletions(-) create mode 100644 src/rule/RuleVisitor.ts diff --git a/src/app.ts b/src/app.ts index ad7ac7b..d4d7306 100644 --- a/src/app.ts +++ b/src/app.ts @@ -4,7 +4,8 @@ import { showCompletionScript } from 'yargs'; import { loadConfig } from './config'; import { CONFIG_ARGS_NAME, CONFIG_ARGS_PATH, MODE, parseArgs } from './config/args'; import { YamlParser } from './parser/YamlParser'; -import { createRuleSelector, createRuleSources, loadRules, resolveRules, visitRules } from './rule'; +import { createRuleSelector, createRuleSources, loadRules, resolveRules } from './rule'; +import { RuleVisitor } from './rule/RuleVisitor'; import { readSource, writeSource } from './source'; import { VERSION_INFO } from './version'; import { VisitorContext } from './visitor/VisitorContext'; @@ -59,8 +60,12 @@ export async function main(argv: Array): Promise { const source = await readSource(args.source); const docs = parser.parse(source); - for (const data of docs) { - await visitRules(ctx, activeRules, data); + const visitor = new RuleVisitor({ + rules: activeRules, + }); + + for (const root of docs) { + await visitor.visit(ctx, root); } if (ctx.errors.length === 0) { diff --git a/src/rule/RuleVisitor.ts b/src/rule/RuleVisitor.ts new file mode 100644 index 0000000..d5781b5 --- /dev/null +++ b/src/rule/RuleVisitor.ts @@ -0,0 +1,65 @@ +import { applyDiff, diff } from 'deep-diff'; +import { cloneDeep } from 'lodash'; + +import { Rule } from '.'; +import { hasItems } from '../utils'; +import { Visitor } from '../visitor'; +import { VisitorContext } from '../visitor/VisitorContext'; + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +export interface RuleVisitorOptions { + rules: ReadonlyArray; +} + +export class RuleVisitor implements RuleVisitorOptions, Visitor { + public readonly rules: ReadonlyArray; + + constructor(options: RuleVisitorOptions) { + this.rules = Array.from(options.rules); + } + + public async pick(ctx: VisitorContext, root: any): Promise> { + return []; // why is this part of visitor rather than rule? + } + + public async visit(ctx: VisitorContext, root: any): Promise { + for (const rule of this.rules) { + const items = await rule.pick(ctx, root); + 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.visitData); + continue; + } + + const itemDiff = diff(item, itemResult); + if (hasItems(itemDiff)) { + ctx.logger.info({ + diff: itemDiff, + item, + rule: rule.name, + }, 'rule passed with modifications'); + + if (ctx.schemaOptions.mutate) { + applyDiff(item, itemResult); + } + } else { + ctx.logger.info({ rule: rule.name }, 'rule passed'); + } + + itemIndex += 1; + } + } + + return ctx; + } +} diff --git a/src/rule/index.ts b/src/rule/index.ts index 4a9ccc3..d24bac1 100644 --- a/src/rule/index.ts +++ b/src/rule/index.ts @@ -1,13 +1,12 @@ import { ValidateFunction } from 'ajv'; -import { applyDiff, diff } from 'deep-diff'; -import { cloneDeep, Dictionary, intersection, isNil } from 'lodash'; +import { Dictionary, intersection, isNil } from 'lodash'; import { Minimatch } from 'minimatch'; import { LogLevel } from 'noicejs'; import recursive from 'recursive-readdir'; import { YamlParser } from '../parser/YamlParser'; import { readFile } from '../source'; -import { ensureArray, hasItems } from '../utils'; +import { ensureArray } from '../utils'; import { VisitorResult } from '../visitor'; import { VisitorContext } from '../visitor/VisitorContext'; import { SchemaRule } from './SchemaRule'; @@ -204,43 +203,3 @@ export async function resolveRules(rules: Array, selector: RuleSelector): return Array.from(activeRules); } - -export async function visitRules(ctx: VisitorContext, rules: Array, data: any): Promise { - 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.visitData); - continue; - } - - const itemDiff = diff(item, itemResult); - if (hasItems(itemDiff)) { - ctx.logger.info({ - diff: itemDiff, - item, - rule: rule.name, - }, 'rule passed with modifications'); - - if (ctx.schemaOptions.mutate) { - applyDiff(item, itemResult); - } - } else { - ctx.logger.info({ rule: rule.name }, 'rule passed'); - } - - itemIndex += 1; - } - } - - return ctx; -} diff --git a/test/rule/TestRule.ts b/test/rule/TestRule.ts index ad431a6..53a12e9 100644 --- a/test/rule/TestRule.ts +++ b/test/rule/TestRule.ts @@ -2,7 +2,8 @@ import { expect } from 'chai'; import { LogLevel, NullLogger } from 'noicejs'; import { mock, spy, stub } from 'sinon'; -import { createRuleSelector, createRuleSources, resolveRules, visitRules } from '../../src/rule'; +import { createRuleSelector, createRuleSources, resolveRules } from '../../src/rule'; +import { RuleVisitor } from '../../src/rule/RuleVisitor'; import { SchemaRule } from '../../src/rule/SchemaRule'; import { VisitorContext } from '../../src/visitor/VisitorContext'; import { describeLeaks, itLeaks } from '../helpers/async'; @@ -128,7 +129,10 @@ describeLeaks('rule visitor', async () => { pickStub.onFirstCall().returns(Promise.resolve([])); pickStub.throws(); - await visitRules(ctx, [rule], {}); + const visitor = new RuleVisitor({ + rules: [rule], + }); + await visitor.visit(ctx, {}); mockRule.verify(); expect(ctx.errors.length).to.equal(0); @@ -163,7 +167,10 @@ describeLeaks('rule visitor', async () => { visitStub.onFirstCall().returns(Promise.resolve(ctx)); visitStub.throws(); - await visitRules(ctx, [rule], {}); + const visitor = new RuleVisitor({ + rules: [rule], + }); + await visitor.visit(ctx, {}); mockRule.verify(); expect(ctx.errors.length).to.equal(0); @@ -196,7 +203,10 @@ describeLeaks('rule visitor', async () => { errors: [], })); - await visitRules(ctx, [rule], data); + const visitor = new RuleVisitor({ + rules: [rule], + }); + await visitor.visit(ctx, data); expect(pickSpy).to.have.callCount(1).and.to.have.been.calledWithExactly(ctx, data); expect(visitStub).to.have.callCount(3); @@ -232,7 +242,10 @@ describeLeaks('rule visitor', async () => { }], })); - await visitRules(ctx, [rule], data); + const visitor = new RuleVisitor({ + rules: [rule], + }); + await visitor.visit(ctx, data); expect(visitStub).to.have.callCount(3); expect(ctx.errors.length).to.equal(3);