perf(resolve): only read license/ deprecation when used in rule set (s…

…verweij#412)

When 'normalising' the options, it determines whether a license check and/ or a deprecation check against external dependencies is necessary. If it isn't necessary skipping these checks saves a significant amount of time (averages over consecutive runs on a 2,6 GHz Quad-Core Intel Core i7 with an SSD with APFS - macOS 10.15.7);

- For dependency-cruiser's own scan it saved ~1.5s (for the license check) and ~0.3s (for the deprecation check) on a total run time taking ~3.2s (for 395 modules, 939 dependencies) before these improvements.
- For yarn 'berry' it saved ~2s (license check + deprecation check) on a total run time originaly taking ~9.5s (for 745 modules, 1262 dependencies using ts -> ast ) before these improvements.

.gitignore

@@ -17,6 +17,8 @@ coverage
 jsdoc
 dependency-violations.html
 .cache
+*.cpuprofile
+*.heapprofile
 
 # node and npm
 /node_modules/

src/enrich/summarize/summarize-modules.js

@@ -1,6 +1,6 @@
 const _flattenDeep = require("lodash/flattenDeep");
 const _get = require("lodash/get");
-const findRuleByName = require("../../graph-utl/find-rule-by-name");
+const { findRuleByName } = require("../../graph-utl/rule-set");
 const compare = require("../../graph-utl/compare");
 const deDuplicateViolations = require("./de-duplicate-violations");
 

src/extract/resolve/determine-dependency-types.js

@@ -104,6 +104,7 @@ function determineNodeModuleDependencyTypes(
   );
 
   if (
+    pResolveOptions.resolveDeprecations &&
     localNpmHelpers.dependencyIsDeprecated(
       pModuleName,
       pFileDirectory,

src/extract/resolve/local-npm-helpers.js

@@ -128,7 +128,7 @@ function dependencyIsDeprecated(pModule, pBaseDirectory, pResolveOptions) {
 /**
  * Returns the license of pModule as resolved to pBaseDirectory - if any
  *
- * @param  {string} pModule  The module to get the deprecation status of
+ * @param  {string} pModule  The module to get the license of
  * @param  {string} pBaseDirectory The base dir. Defaults to '.'
  * @param  {any} pResolveOptions options for the resolver
  * @return {string}          The module's license string, or '' in case

src/extract/resolve/resolve-amd.js

@@ -69,10 +69,13 @@ module.exports = function resolveAMD(
       !Boolean(isCore(pRawModuleName)) && !fileExists(lResolvedPath),
   };
 
-  // we might want to use resolve options instead of {} here
   return {
     ...lReturnValue,
-    ...resolveHelpers.addLicenseAttribute(lModuleName, pBaseDirectory, {}),
+    ...resolveHelpers.addLicenseAttribute(
+      lModuleName,
+      pBaseDirectory,
+      pResolveOptions
+    ),
     dependencyTypes: determineDependencyTypes(
       lReturnValue,
       lModuleName,

src/extract/resolve/resolve-helpers.js

@@ -3,14 +3,16 @@ const localNpmHelpers = require("./local-npm-helpers");
 module.exports = {
   addLicenseAttribute(pModuleName, pBaseDirectory, pResolveOptions) {
     let lReturnValue = {};
-    const lLicense = localNpmHelpers.getLicense(
-      pModuleName,
-      pBaseDirectory,
-      pResolveOptions
-    );
+    if (pResolveOptions.resolveLicenses) {
+      const lLicense = localNpmHelpers.getLicense(
+        pModuleName,
+        pBaseDirectory,
+        pResolveOptions
+      );
 
-    if (Boolean(lLicense)) {
-      lReturnValue.license = lLicense;
+      if (Boolean(lLicense)) {
+        lReturnValue.license = lLicense;
+      }
     }
     return lReturnValue;
   },

src/graph-utl/rule-set.js

@@ -0,0 +1,57 @@
+const _get = require("lodash/get");
+const _has = require("lodash/has");
+
+/**
+ * Finds the first rule in the rule set that has name pName,
+ * and undefined if no such rule exists/ the rule is an 'allowed'
+ * rule.
+ *
+ * (this thing probably belongs in a model-like folder and not in utl)
+ *
+ * @param {import("../../types/configuration").IConfiguration} pRuleSet - The rule set to search in
+ * @param {string} pName - The rule name to look for
+ * @return {import("../../types/rule-set").IForbiddenRuleType|import("../../types/rule-set").IAllowedRuleType} - a rule (or 'undefined' if nothing found)
+ */
+function findRuleByName(pRuleSet, pName) {
+  return _get(pRuleSet, "forbidden", []).find(
+    (pForbiddenRule) => pForbiddenRule.name === pName
+  );
+}
+
+/**
+ * Returns true if the rule set has a rule that uses the 'license' or
+ * 'licenseNot' attribute.
+ *
+ * Returns false in all other cases
+ *
+ * @param {import('../../../types/rule-set').IFlattenedRuleSet} pRuleSet
+ * @return {boolean}
+ */
+function ruleSetHasLicenseRule(pRuleSet) {
+  return (
+    _get(pRuleSet, "forbidden", []).some(
+      (pRule) => _has(pRule.to, "license") || _has(pRule.to, "licenseNot")
+    ) ||
+    _get(pRuleSet, "allowed", []).some(
+      (pRule) => _has(pRule.to, "license") || _has(pRule.to, "licenseNot")
+    )
+  );
+}
+function ruleSetHasDeprecationRule(pRuleSet) {
+  return (
+    _get(pRuleSet, "forbidden", []).some((pRule) =>
+      _get(pRule.to, "dependencyTypes", []).includes("deprecated")
+    ) ||
+    _get(pRuleSet, "allowed", []).some((pRule) =>
+      _get(pRule.to, "dependencyTypes", []).includes("deprecated")
+    )
+  );
+}
+
+module.exports = {
+  findRuleByName,
+  ruleSetHasLicenseRule,
+  ruleSetHasDeprecationRule,
+};
+
+// file deepcode ignore valid-jsdoc: deepcode believes this isn't valid, but likely it just isn't privy on imports as types

src/main/resolve-options/normalize.js

@@ -6,6 +6,10 @@ const enhancedResolve = require("enhanced-resolve");
 const PnpWebpackPlugin = require("pnp-webpack-plugin");
 const TsConfigPathsPlugin = require("tsconfig-paths-webpack-plugin");
 const transpileMeta = require("../../extract/transpile/meta");
+const {
+  ruleSetHasLicenseRule,
+  ruleSetHasDeprecationRule,
+} = require("../../graph-utl/rule-set");
 
 const DEFAULT_CACHE_DURATION = 4000;
 const DEFAULT_RESOLVE_OPTIONS = {
@@ -111,6 +115,7 @@ module.exports = function normalizeResolveOptions(
   pOptions,
   pTSConfig
 ) {
+  const lRuleSet = _get(pOptions, "ruleSet", {});
   return compileResolveOptions(
     {
       /*
@@ -130,6 +135,8 @@ module.exports = function normalizeResolveOptions(
       externalModuleResolutionStrategy:
         pOptions.externalModuleResolutionStrategy,
       combinedDependencies: pOptions.combinedDependencies,
+      resolveLicenses: ruleSetHasLicenseRule(lRuleSet),
+      resolveDeprecations: ruleSetHasDeprecationRule(lRuleSet),
       ...(pResolveOptions || {}),
     },
     pTSConfig || {},

src/report/error.js

@@ -2,7 +2,7 @@ const chalk = require("chalk");
 const figures = require("figures");
 
 const _get = require("lodash/get");
-const findRuleByName = require("../graph-utl/find-rule-by-name");
+const { findRuleByName } = require("../graph-utl/rule-set");
 const wrapAndIndent = require("../utl/wrap-and-indent");
 
 const SEVERITY2CHALK = {

test/extract/get-dependencies.spec.js

@@ -35,7 +35,7 @@ function runFixture(pFixture) {
         pFixture.input.fileName,
         normalizeCruiseOptions(lOptions),
         normalizeResolveOptions(
-          { bustTheCache: true },
+          { bustTheCache: true, resolveLicenses: true },
           normalizeCruiseOptions(lOptions)
         )
       )

test/extract/resolve/local-npm-helpers.spec.js

@@ -195,4 +195,17 @@ describe("extract/resolve/localNpmHelpers.getLicense", () => {
     ).to.equal("GPL-3.0");
   });
 });
+
+describe("extract/resolve/localNpmHelpers.dependencyIsDeprecated", () => {
+  it("returns false if the module does not exist", () => {
+    expect(
+      localNpmHelpers.dependencyIsDeprecated(
+        "this-module-does-not-exist",
+        ".",
+        {}
+      )
+    ).to.equal(false);
+  });
+});
+
 /* eslint no-unused-expressions: 0 */

test/graph-utl/rule-set.spec.js

@@ -0,0 +1,118 @@
+/* eslint-disable no-unused-expressions */
+const { expect } = require("chai");
+const {
+  findRuleByName,
+  ruleSetHasLicenseRule,
+  ruleSetHasDeprecationRule,
+} = require("../../src/graph-utl/rule-set");
+
+describe("graph-utl/rule-set - findRuleByName", () => {
+  const lRuleSet = {
+    forbidden: [{ name: "a-rule", severity: "warn", from: {}, to: {} }],
+  };
+
+  it("returns undefined for null rule set/ null rule name", () => {
+    expect(findRuleByName(null, null)).to.be.undefined;
+  });
+  it("returns undefined for empty rule set/ null rule name", () => {
+    expect(findRuleByName({}, null)).to.be.undefined;
+  });
+  it("returns undefined for empty rule set/ non-null rule name", () => {
+    expect(findRuleByName({}, "non-null-rule-name")).to.be.undefined;
+  });
+  it("returns undefined for undefined rule set/ non-null rule name", () => {
+    // eslint-disable-next-line no-undefined
+    expect(findRuleByName(undefined, "non-null-rule-name")).to.be.undefined;
+  });
+  it("returns undefined if the rule is not in there", () => {
+    // eslint-disable-next-line no-undefined
+    expect(findRuleByName(lRuleSet, "another-rule")).to.be.undefined;
+  });
+  it("returns the rule if the rule is in there", () => {
+    expect(findRuleByName(lRuleSet, "a-rule")).to.deep.equal({
+      name: "a-rule",
+      severity: "warn",
+      from: {},
+      to: {},
+    });
+  });
+});
+
+describe("graph-utl/rule-set - ruleSetHasLicenseRule", () => {
+  it("returns false for an empty rule set", () => {
+    expect(ruleSetHasLicenseRule({})).to.equal(false);
+  });
+  it("returns false when rule set no rules with license-like attributes", () => {
+    expect(
+      ruleSetHasLicenseRule({
+        forbidden: [{ from: {}, to: {} }],
+        allowed: [{ from: {}, to: {} }],
+      })
+    ).to.equal(false);
+  });
+  it("returns true when rule set has a forbidden rule with a license attribute", () => {
+    expect(
+      ruleSetHasLicenseRule({
+        forbidden: [{ from: {}, to: { license: "commercial" } }],
+      })
+    ).to.equal(true);
+  });
+  it("returns true when rule set doesn't have a forbidden rule with license like attributes", () => {
+    expect(
+      ruleSetHasLicenseRule({
+        forbidden: [{ from: {}, to: { licenseNot: "" } }],
+      })
+    ).to.equal(true);
+  });
+  it("returns true when rule set has a forbidden rule with a licenseNot attribute", () => {
+    expect(
+      ruleSetHasLicenseRule({
+        forbidden: [{ from: {}, to: { licenseNot: "MIT" } }],
+      })
+    ).to.equal(true);
+  });
+  it("returns true when rule set has an allowed rule with a license attribute", () => {
+    expect(
+      ruleSetHasLicenseRule({
+        allowed: [{ from: {}, to: { license: "commercial" } }],
+      })
+    ).to.equal(true);
+  });
+  it("returns true when rule set has an allowed rule with a licenseNot attribute", () => {
+    expect(
+      ruleSetHasLicenseRule({
+        allowed: [{ from: {}, to: { licenseNot: "MIT" } }],
+      })
+    ).to.equal(true);
+  });
+});
+
+describe("graph-utl/rule-set - ruleSetHasDeprecation", () => {
+  it("returns false for an empty rule set", () => {
+    expect(ruleSetHasDeprecationRule({})).to.equal(false);
+  });
+  it("returns false when rule set no rules with license-like attributes", () => {
+    expect(
+      ruleSetHasDeprecationRule({
+        forbidden: [{ from: {}, to: { dependencyTypes: ["npm"] } }],
+        allowed: [{ from: {}, to: {} }],
+      })
+    ).to.equal(false);
+  });
+  it("returns true when there's a 'forbidden' rule that checks for external module deprecation", () => {
+    expect(
+      ruleSetHasDeprecationRule({
+        forbidden: [{ from: {}, to: { dependencyTypes: ["deprecated"] } }],
+        allowed: [{ from: {}, to: {} }],
+      })
+    ).to.equal(true);
+  });
+  it("returns true when there's an 'allowed' rule that checks for external module deprecation", () => {
+    expect(
+      ruleSetHasDeprecationRule({
+        forbidden: [{ from: {}, to: {} }],
+        allowed: [{ from: {}, to: { dependencyTypes: ["deprecated"] } }],
+      })
+    ).to.equal(true);
+  });
+});

test/main/fixtures/cruise-reporterless/amd.json

@@ -21,7 +21,8 @@
             "exoticallyRequired": false,
             "matchesDoNotFollow": false,
             "couldNotResolve": true,
-            "valid": true
+            "valid": true,
+            "license": "MIT"
           },
           {
             "module": "other-module-in-the-config",

test/main/main.cruise-reporterless.spec.js

@@ -18,6 +18,8 @@ function runRecursiveFixture(pFixture) {
     it(pFixture.title, () => {
       let lResult = cruise([pFixture.input.fileName], pFixture.input.options, {
         bustTheCache: true,
+        resolveLicenses: true,
+        resolveDeprecations: true,
       }).output;
 
       expect(lResult).to.be.jsonSchema(cruiseResultSchema);

test/main/main.cruise.type-only-module-references.spec.js

@@ -27,7 +27,7 @@ describe("main.cruise - type only module references", () => {
       {
         tsPreCompilationDeps: true,
       },
-      { bustTheCache: true }
+      { bustTheCache: true, resolveLicenses: true }
     );
 
     expect(lResult.output).to.deep.equal(output);