🔧 separates circularity algorithm better

- the algorithm is now in a module without the stuff that adds its results
  to the dependency tree
- this means it's easier to unit test & hence safely refactor
- adds unit tests

.istanbul.yml

@@ -1,3 +1,3 @@
 instrumentation:
-    excludes: ["test/**/*", "src/cli/index.js", "coverage/**/*", "tmp*", "utl/**/*"]
+    excludes: ["test/**/*", "src/cli/index.js", "src/report/*.template.js", "coverage/**/*", "tmp*", "utl/**/*"]
     include-all-sources: true

Makefile

@@ -127,7 +127,7 @@ src/main/index.js: \
 
 src/extract/index.js: \
 	src/extract/addValidations.js \
-	src/extract/detectCircularity.js \
+	src/extract/dependencyEndsUpAtFrom.js \
 	src/extract/extract.js \
 	src/extract/gatherInitialSources.js \
 	src/extract/summarize.js
@@ -210,7 +210,7 @@ src/validate/validateRuleSet.js: \
 ALL_SRC=src/main/index.js \
 	package.json \
 	src/extract/addValidations.js \
-	src/extract/detectCircularity.js \
+	src/extract/dependencyEndsUpAtFrom.js \
 	src/extract/extract-AMD.js \
 	src/extract/extract-ES6.js \
 	src/extract/extract-commonJS.js \
@@ -271,7 +271,7 @@ src/main/index.js: \
 
 src/extract/index.js: \
 	src/extract/addValidations.js \
-	src/extract/detectCircularity.js \
+	src/extract/dependencyEndsUpAtFrom.js \
 	src/extract/extract.js \
 	src/extract/gatherInitialSources.js \
 	src/extract/summarize.js
@@ -370,6 +370,9 @@ test/cli/normalizeOptions.spec.js: \
 test/cli/validateParameters.spec.js: \
 	src/cli/validateParameters.js
 
+test/extract/dependencyEndsUpAtFrom.spec.js: \
+	src/extract/dependencyEndsUpAtFrom.js
+
 test/extract/extract-composite.spec.js: \
 	src/extract/index.js \
 	src/extract/jsonschema.json

src/cli/initRules.js

@@ -17,6 +17,17 @@ function fileExists(pFile) {
     return true;
 }
 
+/**
+ * Creates a .dependency-cruiser config with a set of basic validations
+ * to the current directory.
+ *
+ * @returns {void} Nothing
+ * @throws {Error}  An error object with the root cause of the problem
+ *                  as a description:
+ *                  - file already exists
+ *                  - writing to the file doesn't work
+ *
+ */
 module.exports = () => {
     if (fileExists(DEPENDENCY_CRUISER_CONFIG)) {
         throw Error(`A '${DEPENDENCY_CRUISER_CONFIG}' already exists here - leaving it be.\n`);

src/cli/normalizeOptions.js

@@ -3,6 +3,7 @@
 const _ = require("lodash");
 
 const DEFAULT_MODULE_SYSTEMS = ["cjs", "amd", "es6"];
+const DEFAULT_RULES_FILE_NAME = ".dependency-cruiser.json";
 
 function normalizeModuleSystems(pSystemList) {
     if (_.isString(pSystemList)) {
@@ -18,12 +19,19 @@ function normalizeModuleSystems(pSystemList) {
 
 function determineRulesFileName(pValidate) {
     if (typeof pValidate === 'boolean' && pValidate){
-        return ".dependency-cruiser.json";
+        return DEFAULT_RULES_FILE_NAME;
     } else {
         return pValidate;
     }
 }
 
+/**
+ * returns the pOptions, so that the returned value contains a
+ * valid value for each possible option
+ *
+ * @param  {object} pOptions [description]
+ * @return {object}          [description]
+ */
 module.exports = (pOptions) => {
     pOptions = _.defaults(pOptions, {
         exclude: "",

src/extract/dependencyEndsUpAtFrom.js

@@ -0,0 +1,44 @@
+/**
+ * Returns true if the graph behind pTo contains pFrom.
+ * Returns false in all other cases
+ *
+ * @param  {object} pGraph   The graph in which to test this condition
+ * @param  {string} pFrom    The 'source' attribute of the node to be tested
+ *                           (source uniquely identifying a node)
+ * @param  {string} pTo      The 'source' attribute of the 'to' node to
+ *                           be traversed
+ * @param  {Set} pVisited    The set of nodes visited hithereto (optional
+ *                           attribute - there's no need to pass it when
+ *                           calling it; it's used by the algorithm to
+ *                           determine the stop condition)
+ * @return {boolean}        see description above
+ */
+function dependencyEndsUpAtFrom(pGraph, pFrom, pTo, pVisited) {
+    pVisited = pVisited || new Set();
+
+    const lToNode = pGraph.filter(pNode => pNode.source === pTo)[0];
+
+    /* about the absence of checks whether attributes/ objects actually
+     * exist:
+     * - it saves CPU cycles to the effect of being ~30% faster than with the
+     *   checks
+     * - lToNode: is guaranteed to be there by the extract/ complete in index.js
+     * - lToNode.dependencies is a mandatory attribute (as per json schema)
+     * - pToToNode.resolved is a mandatory attribute (as per json schema)
+     */
+    return lToNode.dependencies.filter(
+            pToToNode => !pVisited.has(pToToNode.resolved)
+        ).some(
+            pToToNode =>
+                pToToNode.resolved === pFrom
+                ? true
+                : dependencyEndsUpAtFrom(
+                    pGraph,
+                    pFrom,
+                    pToToNode.resolved,
+                    pVisited.add(pToToNode.resolved)
+                )
+        );
+}
+
+module.exports = dependencyEndsUpAtFrom;

src/extract/index.js

@@ -1,12 +1,12 @@
 "use strict";
 
-const _                 = require('lodash');
+const _                      = require('lodash');
 
-const extract           = require('./extract');
-const detectCircularity = require('./detectCircularity');
-const gather            = require('./gatherInitialSources');
-const summarize         = require('./summarize');
-const addValidations    = require('./addValidations');
+const extract                = require('./extract');
+const dependencyEndsUpAtFrom = require('./dependencyEndsUpAtFrom');
+const gather                 = require('./gatherInitialSources');
+const summarize              = require('./summarize');
+const addValidations         = require('./addValidations');
 
 function extractRecursive (pFileName, pOptions, pVisited) {
     pOptions = pOptions || {};
@@ -118,6 +118,40 @@ function circularityDetectionNecessary(pOptions) {
     return false;
 }
 
+function addCircularityCheckToDependency (pToDep, pGraph, pFrom) {
+    return Object.assign(
+        {},
+        pToDep,
+        {
+            circular: dependencyEndsUpAtFrom(pGraph, pFrom, pToDep.resolved)
+        }
+    );
+}
+
+/**
+ * Runs through all dependencies, for each of them determines
+ * whether it's (part of a) circular (relationship) and returns the
+ * dependencies with that added.
+ *
+ * @param  {Object} pDependencies [description]
+ * @return {Object}               the same dependencies, but for each
+ *                                of them added whether or not it is
+ *                                part of
+ */
+function addCircularityCheckToGraph (pDependencies) {
+    return pDependencies.map(
+        pNode => Object.assign(
+            {},
+            pNode,
+            {
+                dependencies: pNode.dependencies.map(
+                    pToDep => addCircularityCheckToDependency(pToDep, pDependencies, pNode.source)
+                )
+            }
+        )
+    );
+}
+
 module.exports = (pFileDirArray, pOptions, pCallback) => {
     const lCallback = pCallback ? pCallback : (pInput => pInput);
 
@@ -127,7 +161,7 @@ module.exports = (pFileDirArray, pOptions, pCallback) => {
      .value();
 
     if (circularityDetectionNecessary(pOptions)){
-        lDependencies = detectCircularity(lDependencies);
+        lDependencies = addCircularityCheckToGraph(lDependencies);
     }
 
     lDependencies = addValidations(

src/validate/readRuleSet.js

@@ -14,6 +14,8 @@ const normalizeRuleSet = require('./normalizeRuleSet');
  * @param  {object} pRuleSetJSON    The to be validated ruleset (you can both pass
  *                                  an object or JSON as a string)
  * @return {object}                 The validated & normalized rule set
+ * @throws {Error}                  An error with the reason for the error as
+ *                                  a message
  */
 module.exports = pRuleSetJSON =>
     normalizeRuleSet(

src/validate/validateRuleSet.js

@@ -44,6 +44,8 @@ function checkRuleSafety(pRule) {
  *
  * @param  {object} pRuleSet The ruleset to validate
  * @return {object}          The ruleset as passed
+ * @throws {Error}           An error with the reason for the error as
+ *                           a message
  */
 module.exports = (pRuleSet) => {
     validateAgainstSchema(ruleSchema, pRuleSet);