perf: improve rule no-cycle using strongly connected components (#111)

.changeset/silent-pumas-sell.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-import-x": patch
+---
+
+Drastically improve `no-cycle`'s performance by skipping unnecessary BFSes using [Tarjan's SCC](https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm).

package.json

@@ -48,6 +48,7 @@
     "eslint": "^8.56.0 || ^9.0.0-0"
   },
   "dependencies": {
+    "@rtsao/scc": "^1.1.0",
     "@typescript-eslint/utils": "^7.4.0",
     "debug": "^4.3.4",
     "doctrine": "^3.0.0",

src/rules/no-cycle.ts

@@ -5,6 +5,7 @@
 import type { DeclarationMetadata, ModuleOptions } from '../utils'
 import {
   ExportMap,
+  StronglyConnectedComponents,
   isExternalModule,
   createRule,
   moduleVisitor,
@@ -88,6 +89,8 @@ export = createRule<[Options?], MessageId>({
           isExternalModule(name, resolve(name, context)!, context)
       : () => false
 
+    const scc = StronglyConnectedComponents.get(filename, context)
+
     return {
       ...moduleVisitor(function checkSourceValue(sourceNode, importer) {
         if (ignoreModule(sourceNode.value)) {
@@ -127,6 +130,18 @@ export = createRule<[Options?], MessageId>({
           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.
+         */
+        if (scc) {
+          const hasDependencyCycle = scc[filename] === scc[imported.path]
+          if (!hasDependencyCycle) {
+            return
+          }
+        }
+
         const untraversed: Traverser[] = [{ mget: () => imported, route: [] }]
 
         function detectCycle({ mget, route }: Traverser) {

src/utils/export-map.ts

@@ -1094,7 +1094,7 @@ export function recursivePatternCapture(
  * don't hold full context object in memory, just grab what we need.
  * also calculate a cacheKey, where parts of the cacheKey hash are memoized
  */
-function childContext(
+export function childContext(
   path: string,
   context: RuleContext | ChildContext,
 ): ChildContext {

src/utils/index.ts

@@ -17,6 +17,7 @@ export * from './pkg-dir'
 export * from './pkg-up'
 export * from './read-pkg-up'
 export * from './resolve'
+export * from './scc'
 export * from './static-require'
 export * from './unambiguous'
 export * from './visit'

src/utils/scc.ts

@@ -0,0 +1,91 @@
+import calculateScc from '@rtsao/scc'
+
+import type { ChildContext, RuleContext } from '../types'
+
+import { ExportMap, childContext } from './export-map'
+import { resolve } from './resolve'
+
+const cache = new Map<string, Record<string, number>>()
+
+export const StronglyConnectedComponents = {
+  clearCache() {
+    cache.clear()
+  },
+
+  get(source: string, context: RuleContext) {
+    const path = resolve(source, context)
+    if (path == null) {
+      return null
+    }
+    return StronglyConnectedComponents.for(childContext(path, context))
+  },
+
+  for(context: ChildContext) {
+    const cacheKey = context.cacheKey
+    if (cache.has(cacheKey)) {
+      return cache.get(cacheKey)!
+    }
+    const scc = StronglyConnectedComponents.calculate(context)
+    cache.set(cacheKey, scc)
+    return scc
+  },
+
+  calculate(context: ChildContext) {
+    const exportMap = ExportMap.for(context)
+    const adjacencyList =
+      StronglyConnectedComponents.exportMapToAdjacencyList(exportMap)
+    const calculatedScc = calculateScc(adjacencyList)
+    return StronglyConnectedComponents.calculatedSccToPlainObject(calculatedScc)
+  },
+
+  exportMapToAdjacencyList(initialExportMap: ExportMap | null) {
+    /** for each dep, what are its direct deps */
+    const adjacencyList = new Map<string, Set<string>>()
+    // BFS
+    function visitNode(exportMap: ExportMap | null) {
+      if (!exportMap) {
+        return
+      }
+      for (const [importedPath, v] of exportMap.imports.entries()) {
+        const from = exportMap.path
+        const to = importedPath
+
+        if (!adjacencyList.has(from)) {
+          adjacencyList.set(from, new Set())
+        }
+
+        const set = adjacencyList.get(from)!
+
+        if (set.has(to)) {
+          continue // prevent endless loop
+        }
+        set.add(to)
+        visitNode(v.getter())
+      }
+    }
+    visitNode(initialExportMap)
+    // Fill gaps
+    // eslint-disable-next-line unicorn/no-array-for-each -- Map.forEach, and it is way faster
+    adjacencyList.forEach(values => {
+      // eslint-disable-next-line unicorn/no-array-for-each -- Set.forEach
+      values.forEach(value => {
+        if (!adjacencyList.has(value)) {
+          adjacencyList.set(value, new Set())
+        }
+      })
+    })
+
+    return adjacencyList
+  },
+
+  calculatedSccToPlainObject(sccs: Array<Set<string>>) {
+    /** for each key, its SCC's index */
+    const obj: Record<string, number> = {}
+    for (const [index, scc] of sccs.entries()) {
+      for (const node of scc) {
+        obj[node] = index
+      }
+    }
+    return obj
+  },
+}

test/utils/scc.spec.ts

@@ -0,0 +1,193 @@
+// import sinon from 'sinon';
+import { testContext } from '../utils'
+
+import {
+  StronglyConnectedComponents,
+  ExportMap,
+  childContext as buildChildContext,
+} from 'eslint-plugin-import-x/utils'
+
+function exportMapFixtureBuilder(
+  path: string,
+  imports: ExportMap[],
+): ExportMap {
+  return {
+    path,
+    imports: new Map(
+      imports.map(imp => [
+        imp.path,
+        { getter: () => imp, declarations: new Set() },
+      ]),
+    ),
+  } as ExportMap
+}
+
+describe('Strongly Connected Components Builder', () => {
+  afterEach(() => StronglyConnectedComponents.clearCache())
+
+  describe('When getting an SCC', () => {
+    const source = ''
+    const ruleContext = testContext({})
+    const childContext = buildChildContext(source, ruleContext)
+
+    describe('Given two files', () => {
+      describe("When they don't cycle", () => {
+        it('Should return foreign SCCs', () => {
+          jest
+            .spyOn(ExportMap, 'for')
+            .mockReturnValue(
+              exportMapFixtureBuilder('foo.js', [
+                exportMapFixtureBuilder('bar.js', []),
+              ]),
+            )
+          const actual = StronglyConnectedComponents.for(childContext)
+          expect(actual).toEqual({ 'foo.js': 1, 'bar.js': 0 })
+        })
+      })
+
+      describe.skip('When they do cycle', () => {
+        it('Should return same SCC', () => {
+          jest
+            .spyOn(ExportMap, 'for')
+            .mockReturnValue(
+              exportMapFixtureBuilder('foo.js', [
+                exportMapFixtureBuilder('bar.js', [
+                  exportMapFixtureBuilder('foo.js', []),
+                ]),
+              ]),
+            )
+          const actual = StronglyConnectedComponents.get(source, ruleContext)
+          expect(actual).toEqual({ 'foo.js': 0, 'bar.js': 0 })
+        })
+      })
+    })
+
+    describe('Given three files', () => {
+      describe('When they form a line', () => {
+        describe('When A -> B -> C', () => {
+          it('Should return foreign SCCs', () => {
+            jest
+              .spyOn(ExportMap, 'for')
+              .mockReturnValue(
+                exportMapFixtureBuilder('foo.js', [
+                  exportMapFixtureBuilder('bar.js', [
+                    exportMapFixtureBuilder('buzz.js', []),
+                  ]),
+                ]),
+              )
+            const actual = StronglyConnectedComponents.for(childContext)
+            expect(actual).toEqual({ 'foo.js': 2, 'bar.js': 1, 'buzz.js': 0 })
+          })
+        })
+
+        describe('When A -> B <-> C', () => {
+          it('Should return 2 SCCs, A on its own', () => {
+            jest
+              .spyOn(ExportMap, 'for')
+              .mockReturnValue(
+                exportMapFixtureBuilder('foo.js', [
+                  exportMapFixtureBuilder('bar.js', [
+                    exportMapFixtureBuilder('buzz.js', [
+                      exportMapFixtureBuilder('bar.js', []),
+                    ]),
+                  ]),
+                ]),
+              )
+            const actual = StronglyConnectedComponents.for(childContext)
+            expect(actual).toEqual({ 'foo.js': 1, 'bar.js': 0, 'buzz.js': 0 })
+          })
+        })
+
+        describe('When A <-> B -> C', () => {
+          it('Should return 2 SCCs, C on its own', () => {
+            jest
+              .spyOn(ExportMap, 'for')
+              .mockReturnValue(
+                exportMapFixtureBuilder('foo.js', [
+                  exportMapFixtureBuilder('bar.js', [
+                    exportMapFixtureBuilder('buzz.js', []),
+                    exportMapFixtureBuilder('foo.js', []),
+                  ]),
+                ]),
+              )
+            const actual = StronglyConnectedComponents.for(childContext)
+            expect(actual).toEqual({ 'foo.js': 1, 'bar.js': 1, 'buzz.js': 0 })
+          })
+        })
+
+        describe('When A <-> B <-> C', () => {
+          it('Should return same SCC', () => {
+            jest
+              .spyOn(ExportMap, 'for')
+              .mockReturnValue(
+                exportMapFixtureBuilder('foo.js', [
+                  exportMapFixtureBuilder('bar.js', [
+                    exportMapFixtureBuilder('foo.js', []),
+                    exportMapFixtureBuilder('buzz.js', [
+                      exportMapFixtureBuilder('bar.js', []),
+                    ]),
+                  ]),
+                ]),
+              )
+            const actual = StronglyConnectedComponents.for(childContext)
+            expect(actual).toEqual({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 })
+          })
+        })
+      })
+
+      describe('When they form a loop', () => {
+        it('Should return same SCC', () => {
+          jest
+            .spyOn(ExportMap, 'for')
+            .mockReturnValue(
+              exportMapFixtureBuilder('foo.js', [
+                exportMapFixtureBuilder('bar.js', [
+                  exportMapFixtureBuilder('buzz.js', [
+                    exportMapFixtureBuilder('foo.js', []),
+                  ]),
+                ]),
+              ]),
+            )
+          const actual = StronglyConnectedComponents.for(childContext)
+          expect(actual).toEqual({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 })
+        })
+      })
+
+      describe('When they form a Y', () => {
+        it('Should return 3 distinct SCCs', () => {
+          jest
+            .spyOn(ExportMap, 'for')
+            .mockReturnValue(
+              exportMapFixtureBuilder('foo.js', [
+                exportMapFixtureBuilder('bar.js', []),
+                exportMapFixtureBuilder('buzz.js', []),
+              ]),
+            )
+          const actual = StronglyConnectedComponents.for(childContext)
+          expect(actual).toEqual({ 'foo.js': 2, 'bar.js': 0, 'buzz.js': 1 })
+        })
+      })
+
+      describe('When they form a Mercedes', () => {
+        it('Should return 1 SCC', () => {
+          jest
+            .spyOn(ExportMap, 'for')
+            .mockReturnValue(
+              exportMapFixtureBuilder('foo.js', [
+                exportMapFixtureBuilder('bar.js', [
+                  exportMapFixtureBuilder('foo.js', []),
+                  exportMapFixtureBuilder('buzz.js', []),
+                ]),
+                exportMapFixtureBuilder('buzz.js', [
+                  exportMapFixtureBuilder('foo.js', []),
+                  exportMapFixtureBuilder('bar.js', []),
+                ]),
+              ]),
+            )
+          const actual = StronglyConnectedComponents.for(childContext)
+          expect(actual).toEqual({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 })
+        })
+      })
+    })
+  })
+})

yarn.lock

@@ -1886,6 +1886,11 @@
   dependencies:
     "@xml-tools/parser" "^1.0.11"
 
+"@rtsao/scc@^1.1.0":
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/@rtsao/scc/-/scc-1.1.0.tgz#927dd2fae9bc3361403ac2c7a00c32ddce9ad7e8"
+  integrity sha512-zt6OdqaDoOnJ1ZYsCYGt9YmWzDXl4vQdKTyJev62gFhRGKdx7mcT54V9KIjg+d2wi9EXsPvAPKe7i7WjfVWB8g==
+
 "@sinclair/typebox@^0.27.8":
   version "0.27.8"
   resolved "https://registry.yarnpkg.com/@sinclair/typebox/-/typebox-0.27.8.tgz#6667fac16c436b5434a387a34dedb013198f6e6e"