Skip to content

Commit

Permalink
adds the rule violated to the output object (/ json)
Browse files Browse the repository at this point in the history
sverweij committed Nov 24, 2016
1 parent 40886cd commit 52c5a5c
Showing 10 changed files with 252 additions and 54 deletions.
13 changes: 13 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -121,6 +121,10 @@ src/extract/extractor.js: \
src/utl/index.js \
src/validate/validator.js

src/validate/validator.js: \
src/validate/ruleSetNormalizer.js \
src/validate/ruleSetValidator.js

src/extract/extractor-AMD.js: \
src/extract/extractor-commonJS.js

@@ -167,6 +171,8 @@ ALL_SRC=src/index.js \
src/report/htmlReporter.js \
src/report/jsonReporter.js \
src/utl/index.js \
src/validate/ruleSetNormalizer.js \
src/validate/ruleSetValidator.js \
src/validate/validator.js
# cjs dependencies
test/cli.index.spec.js: \
@@ -195,6 +201,10 @@ src/extract/extractor.js: \
src/utl/index.js \
src/validate/validator.js

src/validate/validator.js: \
src/validate/ruleSetNormalizer.js \
src/validate/ruleSetValidator.js

src/extract/extractor-AMD.js: \
src/extract/extractor-commonJS.js

@@ -236,6 +246,9 @@ test/report.dotReporter.spec.js: \
test/report.errReporter.spec.js: \
src/report/errReporter.js

test/validate.ruleSetNormalizer.js: \
src/validate/ruleSetNormalizer.js

test/validate.validator.spec.js: \
src/validate/validator.js

34 changes: 27 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
@@ -56,7 +56,7 @@ and analyze from there.

### dot
Supplying `dot` as output type will make dependency-cruiser write
a GraphViz dot format directed graph. Typical use is in concert
a GraphViz dot format directed graph. Typical use is in concert
with _GraphViz dot_:

```shell
@@ -124,14 +124,28 @@ in the `test` folder and allows everything else:
}
```

You can optionally specify a name and an error level ('error', 'warning' (the
default) and 'information') with them that will appear in some reporters:

```json
{
"forbidden": [{
"name": "no-src-to-test",
"level": "error",
"from": "^src",
"to": "^test"
}]
}
```

A more elaborate configuration:
- modules in `src` can get stuff from `src` and `node_modules`
- modules in `src` can not get stuff from test
- stuff in `node_modules` can call anything, except stuff
we wrote ourselves (in `src`, `bin` and `lib`)
- modules with the pattern `no-deps-at-all-plz` in their name
can't have dependencies to any module.
- modules with the pattern `no-external-deps-plz` can't have
- modules with the pattern `externalDependencyLess\.js` can't have
dependencies to stuff in `node_modules`.


@@ -152,16 +166,22 @@ A more elaborate configuration:
],
"forbidden": [{
"from": "^src",
"to": "^test"
"to": "^test",
"name": "no-src-to-test",
"level": "error"
},{
"from": "no-deps-at-all-plz",
"to": ".+"
"from": "dependencyless\\.js",
"to": ".+",
"comment": "level & name default to 'level' and 'unnamed'"
},{
"from": "no-external-deps-plz",
"to": "node_modules"
"from": "externalDependencyLess\\.js",
"to": "node_modules",
"level": "warning"
},{
"from": "node_modules",
"to": "^(src|test|lib)",
"name": "external-depends-on-you",
"level": "error",
"comment": "well, you never know ..."
}
]
2 changes: 2 additions & 0 deletions doc/sample-output.md
Original file line number Diff line number Diff line change
@@ -8,6 +8,8 @@ the `.dependency-cruiser.json` in the root of her project:
```json
{
"forbidden":[{
"name": "sub-not-allowed",
"level": "error",
"from": ".+",
"to": "sub"
}]
18 changes: 10 additions & 8 deletions src/extract/extractor.js
Original file line number Diff line number Diff line change
@@ -84,19 +84,21 @@ function extractDependencies(pFileName, pOptions) {
path.dirname(pFileName)
);

return {
module : pDependency.moduleName,
resolved : lResolved.resolved,
moduleSystem : pDependency.moduleSystem,
coreModule : lResolved.coreModule,
followable : lResolved.followable,
valid : validator.validate(
return Object.assign(
{
module : pDependency.moduleName,
resolved : lResolved.resolved,
moduleSystem : pDependency.moduleSystem,
coreModule : lResolved.coreModule,
followable : lResolved.followable
},
validator.validate(
pOptions.validate,
pOptions.rulesFile,
pFileName,
lResolved.resolved
)
};
);
}
)
.filter(pDep => utl.ignore(pDep.resolved, pOptions.exclude))
39 changes: 39 additions & 0 deletions src/validate/ruleSetNormalizer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
"use strict";

const VALID_LEVELS = /^(error|warning|information)$/;
const DEFAULT_LEVEL = 'warning';
const DEFAULT_RULE = 'unnamed';

function normalizeLevel (pLevel){
let lLevel = pLevel ? pLevel : DEFAULT_LEVEL;

return Boolean(lLevel.match(VALID_LEVELS)) ? lLevel : DEFAULT_LEVEL;
}

function normalizeName(pRule) {
return pRule ? pRule : DEFAULT_RULE;
}

function normalizeRule(pRule) {
return Object.assign(
pRule,
{
level : normalizeLevel(pRule.level),
name : normalizeName(pRule.name)
}
);
}

function normalize(pRuleSet) {
if (pRuleSet.hasOwnProperty("allowed")){
pRuleSet.allowed = pRuleSet.allowed.map(normalizeRule);
}

if (pRuleSet.hasOwnProperty("forbidden")){
pRuleSet.forbidden = pRuleSet.forbidden.map(normalizeRule);
}

return pRuleSet;
}

exports.normalize = normalize;
27 changes: 27 additions & 0 deletions src/validate/ruleSetValidator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
"use strict";

const safeRegex = require('safe-regex');

function checkRuleSafety(pRule) {
if (
!(
safeRegex(pRule.from) &&
safeRegex(pRule.to)
)
){
throw new Error(
`rule ${JSON.stringify(pRule, null, "")} has an unsafe regular expression. Bailing out.\n`
);
}
}

function validate(pRuleSet) {
if (pRuleSet.hasOwnProperty("allowed")){
pRuleSet.allowed.forEach(checkRuleSafety);
}
if (pRuleSet.hasOwnProperty("forbidden")){
pRuleSet.forbidden.forEach(checkRuleSafety);
}
}

exports.validate = validate;
62 changes: 29 additions & 33 deletions src/validate/validator.js
Original file line number Diff line number Diff line change
@@ -1,60 +1,56 @@
"use strict";

const _ = require("lodash");
const fs = require("fs");
const safeRegex = require('safe-regex');

function checkRuleSafety(pRule) {
if (
!(
safeRegex(pRule.from) &&
safeRegex(pRule.to)
)
){
throw new Error(
`rule ${JSON.stringify(pRule, null, "")} has an unsafe regular expression. Bailing out.\n`
);
}
}

function validateRuleSet(pRuleSet) {
if (pRuleSet.hasOwnProperty("allowed")){
pRuleSet.allowed.forEach(checkRuleSafety);
}
if (pRuleSet.hasOwnProperty("forbidden")){
pRuleSet.forbidden.forEach(checkRuleSafety);
}
}
const _ = require("lodash");
const fs = require("fs");
const ruleSetValidator = require('./ruleSetValidator');
const ruleSetNormalizer = require('./ruleSetNormalizer');

const readRules = _.memoize(
pRuleSetFile => {
let lRetval = JSON.parse(fs.readFileSync(pRuleSetFile, 'utf8'));

validateRuleSet(lRetval);
return lRetval;
ruleSetValidator.validate(lRetval);
return ruleSetNormalizer.normalize(lRetval);
}
);


function matchRule(pFrom, pTo) {
return pRule => pFrom.match(pRule.from) && pTo.match(pRule.to);
}

function validateAgainstRules(pRuleSet, pFrom, pTo) {
let lRetval = true;
let lMatchedRule = {};

if (pRuleSet.hasOwnProperty("allowed")){
lRetval = lRetval && pRuleSet.allowed.some(matchRule(pFrom, pTo));
lMatchedRule = pRuleSet.allowed.find(matchRule(pFrom, pTo));
if (!Boolean(lMatchedRule)){
return {
valid: false,
rule: {
level: "warning",
name: "not-in-allowed"
}
};
}
}
if (pRuleSet.hasOwnProperty("forbidden")){
lRetval = lRetval && !(pRuleSet.forbidden.some(matchRule(pFrom, pTo)));
lMatchedRule = pRuleSet.forbidden.find(matchRule(pFrom, pTo));
if (Boolean(lMatchedRule)){
return {
valid: false,
rule: {
level: lMatchedRule.level,
name : lMatchedRule.name
}
};
}
}
return lRetval;
return {valid:true};
}

function validate (pValidate, pRuleSetFile, pFrom, pTo) {
if (!pValidate) {
return true;
return {valid:true};
}
return validateAgainstRules(readRules(pRuleSetFile), pFrom, pTo);
}
6 changes: 6 additions & 0 deletions test/fixtures/rules.impossible-to-match-allowed.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"allowed":[{
"from": "only-this-one",
"to": "only-that-one"
}]
}
81 changes: 81 additions & 0 deletions test/validate.ruleSetNormalizer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
"use strict";
const expect = require('chai').expect;
const normalizer = require('../src/validate/ruleSetNormalizer');

describe("validator", () => {
it("leaves the empty ruleset alone", () => {
expect(normalizer.normalize({})).to.deep.equal({});
});

it("adds defaults for level and warning when they're not filled", () => {
expect(normalizer.normalize({
"allowed": [{
"from": ".+",
"to": ".+"
}]
})).to.deep.equal({
"allowed": [{
"from": ".+",
"to": ".+",
"level": "warning",
"name": "unnamed"
}]
});
});

it("corrects the level to a default when it's not a recognized one", () => {
expect(normalizer.normalize({
"allowed": [{
"from": ".+",
"to": ".+",
"level": "unrecognized",
"name": "all-ok"
}]
})).to.deep.equal({
"allowed": [{
"from": ".+",
"to": ".+",
"level": "warning",
"name": "all-ok"
}]
});
});

it("keeps the level if it's a recognized one", () => {
expect(normalizer.normalize({
"allowed": [{
"from": ".+",
"to": ".+",
"level": "error",
"name": "all-ok"
}]
})).to.deep.equal({
"allowed": [{
"from": ".+",
"to": ".+",
"level": "error",
"name": "all-ok"
}]
});
});

it("also works for 'forbidden' rules", () => {
expect(normalizer.normalize({
"forbidden": [{
"from": ".+",
"to": ".+",
"level": "error",
"name": "all-ok",
"comment": "this comment is kept"
}]
})).to.deep.equal({
"forbidden": [{
"from": ".+",
"to": ".+",
"level": "error",
"name": "all-ok",
"comment": "this comment is kept"
}]
});
});
});
24 changes: 18 additions & 6 deletions test/validate.validator.spec.js
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ describe("validator", () => {
"koos koets",
"robby van de kerkhof"
)
).to.equal(true);
).to.deep.equal({valid: true});
});

it("is ok with the 'everything allowed' validation", () => {
@@ -22,9 +22,21 @@ describe("validator", () => {
"koos koets",
"robby van de kerkhof"
)
).to.equal(true);
).to.deep.equal({valid: true});
});

it("is ok with the 'everything allowed' validation", () => {
expect(
validator.validate(
true,
"./test/fixtures/rules.impossible-to-match-allowed.json",
"koos koets",
"robby van de kerkhof"
)
).to.deep.equal({valid: false, rule: {level: "warning", "name": "not-in-allowed"}});
});


it("is ok with the 'nothing allowed' validation", () => {
expect(
validator.validate(
@@ -33,7 +45,7 @@ describe("validator", () => {
"koos koets",
"robby van de kerkhof"
)
).to.equal(false);
).to.deep.equal({valid: false, rule: {level: 'warning', name: 'unnamed'}});
});

it("node_modules inhibition - ok", () => {
@@ -44,7 +56,7 @@ describe("validator", () => {
"koos koets",
"robby van de kerkhof"
)
).to.equal(true);
).to.deep.equal({valid: true});
});

it("node_modules inhibition - transgression", () => {
@@ -55,7 +67,7 @@ describe("validator", () => {
"koos koets",
"./node_modules/evil-module"
)
).to.equal(false);
).to.deep.equal({valid: false, rule: {level: 'warning', name: 'unnamed'}});
});

it("bails out on scary regexps", () => {
@@ -66,7 +78,7 @@ describe("validator", () => {
"koos koets",
"robby van de kerkhof"
);
expect("not to be here").to.equal("still here, though");
expect("not to be here").to.deep.equal("still here, though");
} catch (e) {
expect(e).to.deep.equal(
Error(

0 comments on commit 52c5a5c

Please sign in to comment.