optimize no-cycle rule using strongly connected components

CHANGELOG.md

@@ -13,6 +13,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 
 ### Fixed
 - [`no-extraneous-dependencies`]: allow wrong path ([#3012], thanks [@chabb])
+- [`no-cycle`]: use scc algorithm to optimize ([#2998], thanks [@soryy708])
 
 ### Changed
 - [Docs] `no-extraneous-dependencies`: Make glob pattern description more explicit ([#2944], thanks [@mulztob])
@@ -1123,6 +1124,7 @@ for info on changes for earlier releases.
 [#3012]: https://github.com/import-js/eslint-plugin-import/pull/3012
 [#3011]: https://github.com/import-js/eslint-plugin-import/pull/3011
 [#3004]: https://github.com/import-js/eslint-plugin-import/pull/3004
+[#2998]: https://github.com/import-js/eslint-plugin-import/pull/2998
 [#2991]: https://github.com/import-js/eslint-plugin-import/pull/2991
 [#2989]: https://github.com/import-js/eslint-plugin-import/pull/2989
 [#2987]: https://github.com/import-js/eslint-plugin-import/pull/2987

package.json

@@ -105,6 +105,7 @@
     "eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8"
   },
   "dependencies": {
+    "@rtsao/scc": "^1.1.0",
     "array-includes": "^3.1.8",
     "array.prototype.findlastindex": "^1.2.5",
     "array.prototype.flat": "^1.3.2",

src/rules/no-cycle.js

@@ -5,6 +5,7 @@
 
 import resolve from 'eslint-module-utils/resolve';
 import ExportMapBuilder from '../exportMap/builder';
+import StronglyConnectedComponentsBuilder from '../scc';
 import { isExternalModule } from '../core/importType';
 import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor';
 import docsUrl from '../docsUrl';
@@ -47,6 +48,11 @@ module.exports = {
         type: 'boolean',
         default: false,
       },
+      disableScc: {
+        description: 'When true, don\'t calculate a strongly-connected-components graph. SCC is used to reduce the time-complexity of cycle detection, but adds overhead.',
+        type: 'boolean',
+        default: false,
+      },
     })],
   },
 
@@ -62,6 +68,8 @@ module.exports = {
       context,
     );
 
+    const scc = options.disableScc ? {} : StronglyConnectedComponentsBuilder.get(myPath, context);
+
     function checkSourceValue(sourceNode, importer) {
       if (ignoreModule(sourceNode.value)) {
         return; // ignore external modules
@@ -98,6 +106,16 @@ module.exports = {
         return;  // no-self-import territory
       }
 
+      /* If we're in the same Strongly Connected Component,
+       * Then there exists a path from each node in the SCC to every other node in the SCC,
+       * Then there exists at least one path from them to us and from us to them,
+       * Then we have a cycle between us.
+       */
+      const hasDependencyCycle = options.disableScc || scc[myPath] === scc[imported.path];
+      if (!hasDependencyCycle) {
+        return;
+      }
+
       const untraversed = [{ mget: () => imported, route: [] }];
       function detectCycle({ mget, route }) {
         const m = mget();
@@ -106,6 +124,9 @@ module.exports = {
         traversed.add(m.path);
 
         for (const [path, { getter, declarations }] of m.imports) {
+          // If we're in different SCCs, we can't have a circular dependency
+          if (!options.disableScc && scc[myPath] !== scc[path]) { continue; }
+
           if (traversed.has(path)) { continue; }
           const toTraverse = [...declarations].filter(({ source, isOnlyImportingTypes }) => !ignoreModule(source.value)
             // Ignore only type imports

src/scc.js

@@ -0,0 +1,86 @@
+import calculateScc from '@rtsao/scc';
+import { hashObject } from 'eslint-module-utils/hash';
+import resolve from 'eslint-module-utils/resolve';
+import ExportMapBuilder from './exportMap/builder';
+import childContext from './exportMap/childContext';
+
+let cache = new Map();
+
+export default class StronglyConnectedComponentsBuilder {
+  static clearCache() {
+    cache = new Map();
+  }
+
+  static get(source, context) {
+    const path = resolve(source, context);
+    if (path == null) { return null; }
+    return StronglyConnectedComponentsBuilder.for(childContext(path, context));
+  }
+
+  static for(context) {
+    const cacheKey = context.cacheKey || hashObject(context).digest('hex');
+    if (cache.has(cacheKey)) {
+      return cache.get(cacheKey);
+    }
+    const scc = StronglyConnectedComponentsBuilder.calculate(context);
+    cache.set(cacheKey, scc);
+    return scc;
+  }
+
+  static calculate(context) {
+    const exportMap = ExportMapBuilder.for(context);
+    const adjacencyList = this.exportMapToAdjacencyList(exportMap);
+    const calculatedScc = calculateScc(adjacencyList);
+    return StronglyConnectedComponentsBuilder.calculatedSccToPlainObject(calculatedScc);
+  }
+
+  /** @returns {Map<string, Set<string>>} for each dep, what are its direct deps */
+  static exportMapToAdjacencyList(initialExportMap) {
+    const adjacencyList = new Map();
+    // BFS
+    function visitNode(exportMap) {
+      if (!exportMap) {
+        return;
+      }
+      exportMap.imports.forEach((v, importedPath) => {
+        const from = exportMap.path;
+        const to = importedPath;
+
+        // Ignore type-only imports, because we care only about SCCs of value imports
+        const toTraverse = [...v.declarations].filter(({ isOnlyImportingTypes }) => !isOnlyImportingTypes);
+        if (toTraverse.length === 0) { return; }
+
+        if (!adjacencyList.has(from)) {
+          adjacencyList.set(from, new Set());
+        }
+
+        if (adjacencyList.get(from).has(to)) {
+          return; // prevent endless loop
+        }
+        adjacencyList.get(from).add(to);
+        visitNode(v.getter());
+      });
+    }
+    visitNode(initialExportMap);
+    // Fill gaps
+    adjacencyList.forEach((values) => {
+      values.forEach((value) => {
+        if (!adjacencyList.has(value)) {
+          adjacencyList.set(value, new Set());
+        }
+      });
+    });
+    return adjacencyList;
+  }
+
+  /** @returns {Record<string, number>} for each key, its SCC's index */
+  static calculatedSccToPlainObject(sccs) {
+    const obj = {};
+    sccs.forEach((scc, index) => {
+      scc.forEach((node) => {
+        obj[node] = index;
+      });
+    });
+    return obj;
+  }
+}

tests/src/rules/no-cycle.js

@@ -17,7 +17,7 @@ const testVersion = (specifier, t) => _testVersion(specifier, () => Object.assig
 
 const testDialects = ['es6'];
 
-ruleTester.run('no-cycle', rule, {
+const cases = {
   valid: [].concat(
     // this rule doesn't care if the cycle length is 0
     test({ code: 'import foo from "./foo.js"' }),
@@ -290,4 +290,30 @@ ruleTester.run('no-cycle', rule, {
       ],
     }),
   ),
+};
+
+ruleTester.run('no-cycle', rule, {
+  valid: flatMap(cases.valid, (testCase) => [
+    testCase,
+    {
+      ...testCase,
+      code: `${testCase.code} // disableScc=true`,
+      options: [{
+        ...testCase.options && testCase.options[0] || {},
+        disableScc: true,
+      }],
+    },
+  ]),
+
+  invalid: flatMap(cases.invalid, (testCase) => [
+    testCase,
+    {
+      ...testCase,
+      code: `${testCase.code} // disableScc=true`,
+      options: [{
+        ...testCase.options && testCase.options[0] || {},
+        disableScc: true,
+      }],
+    },
+  ]),
 });

tests/src/scc.js

@@ -0,0 +1,179 @@
+import sinon from 'sinon';
+import { expect } from 'chai';
+import StronglyConnectedComponentsBuilder from '../../src/scc';
+import ExportMapBuilder from '../../src/exportMap/builder';
+
+function exportMapFixtureBuilder(path, imports, isOnlyImportingTypes = false) {
+  return {
+    path,
+    imports: new Map(imports.map((imp) => [imp.path, { getter: () => imp, declarations: [{ isOnlyImportingTypes }] }])),
+  };
+}
+
+describe('Strongly Connected Components Builder', () => {
+  afterEach(() => ExportMapBuilder.for.restore());
+  afterEach(() => StronglyConnectedComponentsBuilder.clearCache());
+
+  describe('When getting an SCC', () => {
+    const source = '';
+    const context = {
+      settings: {},
+      parserOptions: {},
+      parserPath: '',
+    };
+
+    describe('Given two files', () => {
+      describe('When they don\'t value-cycle', () => {
+        it('Should return foreign SCCs', () => {
+          sinon.stub(ExportMapBuilder, 'for').returns(
+            exportMapFixtureBuilder('foo.js', [exportMapFixtureBuilder('bar.js', [])]),
+          );
+          const actual = StronglyConnectedComponentsBuilder.for(source, context);
+          expect(actual).to.deep.equal({ 'foo.js': 1, 'bar.js': 0 });
+        });
+      });
+
+      describe('When they do value-cycle', () => {
+        it('Should return same SCC', () => {
+          sinon.stub(ExportMapBuilder, 'for').returns(
+            exportMapFixtureBuilder('foo.js', [
+              exportMapFixtureBuilder('bar.js', [
+                exportMapFixtureBuilder('foo.js', [exportMapFixtureBuilder('bar.js', [])]),
+              ]),
+            ]),
+          );
+          const actual = StronglyConnectedComponentsBuilder.for(source, context);
+          expect(actual).to.deep.equal({ 'foo.js': 0, 'bar.js': 0 });
+        });
+      });
+
+      describe('When they type-cycle', () => {
+        it('Should return foreign SCCs', () => {
+          sinon.stub(ExportMapBuilder, 'for').returns(
+            exportMapFixtureBuilder('foo.js', [
+              exportMapFixtureBuilder('bar.js', [
+                exportMapFixtureBuilder('foo.js', []),
+              ], true),
+            ]),
+          );
+          const actual = StronglyConnectedComponentsBuilder.for(source, context);
+          expect(actual).to.deep.equal({ 'foo.js': 1, 'bar.js': 0 });
+        });
+      });
+    });
+
+    describe('Given three files', () => {
+      describe('When they form a line', () => {
+        describe('When A -> B -> C', () => {
+          it('Should return foreign SCCs', () => {
+            sinon.stub(ExportMapBuilder, 'for').returns(
+              exportMapFixtureBuilder('foo.js', [
+                exportMapFixtureBuilder('bar.js', [
+                  exportMapFixtureBuilder('buzz.js', []),
+                ]),
+              ]),
+            );
+            const actual = StronglyConnectedComponentsBuilder.for(source, context);
+            expect(actual).to.deep.equal({ 'foo.js': 2, 'bar.js': 1, 'buzz.js': 0 });
+          });
+        });
+
+        describe('When A -> B <-> C', () => {
+          it('Should return 2 SCCs, A on its own', () => {
+            sinon.stub(ExportMapBuilder, 'for').returns(
+              exportMapFixtureBuilder('foo.js', [
+                exportMapFixtureBuilder('bar.js', [
+                  exportMapFixtureBuilder('buzz.js', [
+                    exportMapFixtureBuilder('bar.js', []),
+                  ]),
+                ]),
+              ]),
+            );
+            const actual = StronglyConnectedComponentsBuilder.for(source, context);
+            expect(actual).to.deep.equal({ 'foo.js': 1, 'bar.js': 0, 'buzz.js': 0 });
+          });
+        });
+
+        describe('When A <-> B -> C', () => {
+          it('Should return 2 SCCs, C on its own', () => {
+            sinon.stub(ExportMapBuilder, 'for').returns(
+              exportMapFixtureBuilder('foo.js', [
+                exportMapFixtureBuilder('bar.js', [
+                  exportMapFixtureBuilder('buzz.js', []),
+                  exportMapFixtureBuilder('foo.js', []),
+                ]),
+              ]),
+            );
+            const actual = StronglyConnectedComponentsBuilder.for(source, context);
+            expect(actual).to.deep.equal({ 'foo.js': 1, 'bar.js': 1, 'buzz.js': 0 });
+          });
+        });
+
+        describe('When A <-> B <-> C', () => {
+          it('Should return same SCC', () => {
+            sinon.stub(ExportMapBuilder, 'for').returns(
+              exportMapFixtureBuilder('foo.js', [
+                exportMapFixtureBuilder('bar.js', [
+                  exportMapFixtureBuilder('foo.js', []),
+                  exportMapFixtureBuilder('buzz.js', [
+                    exportMapFixtureBuilder('bar.js', []),
+                  ]),
+                ]),
+              ]),
+            );
+            const actual = StronglyConnectedComponentsBuilder.for(source, context);
+            expect(actual).to.deep.equal({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 });
+          });
+        });
+      });
+
+      describe('When they form a loop', () => {
+        it('Should return same SCC', () => {
+          sinon.stub(ExportMapBuilder, 'for').returns(
+            exportMapFixtureBuilder('foo.js', [
+              exportMapFixtureBuilder('bar.js', [
+                exportMapFixtureBuilder('buzz.js', [
+                  exportMapFixtureBuilder('foo.js', []),
+                ]),
+              ]),
+            ]),
+          );
+          const actual = StronglyConnectedComponentsBuilder.for(source, context);
+          expect(actual).to.deep.equal({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 });
+        });
+      });
+
+      describe('When they form a Y', () => {
+        it('Should return 3 distinct SCCs', () => {
+          sinon.stub(ExportMapBuilder, 'for').returns(
+            exportMapFixtureBuilder('foo.js', [
+              exportMapFixtureBuilder('bar.js', []),
+              exportMapFixtureBuilder('buzz.js', []),
+            ]),
+          );
+          const actual = StronglyConnectedComponentsBuilder.for(source, context);
+          expect(actual).to.deep.equal({ 'foo.js': 2, 'bar.js': 0, 'buzz.js': 1 });
+        });
+      });
+
+      describe('When they form a Mercedes', () => {
+        it('Should return 1 SCC', () => {
+          sinon.stub(ExportMapBuilder, 'for').returns(
+            exportMapFixtureBuilder('foo.js', [
+              exportMapFixtureBuilder('bar.js', [
+                exportMapFixtureBuilder('foo.js', []),
+                exportMapFixtureBuilder('buzz.js', []),
+              ]),
+              exportMapFixtureBuilder('buzz.js', [
+                exportMapFixtureBuilder('foo.js', []),
+                exportMapFixtureBuilder('bar.js', []),
+              ]),
+            ]),
+          );
+          const actual = StronglyConnectedComponentsBuilder.for(source, context);
+          expect(actual).to.deep.equal({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 });
+        });
+      });
+    });
+  });
+});