diff --git a/.changeset/silent-pumas-sell.md b/.changeset/silent-pumas-sell.md new file mode 100644 index 000000000..728dba057 --- /dev/null +++ b/.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). diff --git a/package.json b/package.json index ea5921865..cad54b543 100644 --- a/package.json +++ b/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", diff --git a/src/rules/no-cycle.ts b/src/rules/no-cycle.ts index 58a0a20e1..f06b120c8 100644 --- a/src/rules/no-cycle.ts +++ b/src/rules/no-cycle.ts @@ -5,6 +5,7 @@ import type { DeclarationMetadata, ModuleOptions } from '../utils' import { ExportMap, + StronglyConnectedComponents, isExternalModule, createRule, moduleVisitor, @@ -87,6 +88,8 @@ export = createRule<[Options?], MessageId>({ options.ignoreExternal && isExternalModule(name, resolve(name, context)!, context) + const scc = StronglyConnectedComponents.get(filename, context) + return { ...moduleVisitor(function checkSourceValue(sourceNode, importer) { if (ignoreModule(sourceNode.value)) { @@ -126,6 +129,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) { diff --git a/src/utils/export-map.ts b/src/utils/export-map.ts index 58f42b6ba..48827b7ba 100644 --- a/src/utils/export-map.ts +++ b/src/utils/export-map.ts @@ -1083,7 +1083,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 { diff --git a/src/utils/index.ts b/src/utils/index.ts index 6e0b8c986..cbf25f257 100644 --- a/src/utils/index.ts +++ b/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' diff --git a/src/utils/scc.ts b/src/utils/scc.ts new file mode 100644 index 000000000..2d4b71149 --- /dev/null +++ b/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>() + +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>() + // 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>) { + /** for each key, its SCC's index */ + const obj: Record = {} + for (const [index, scc] of sccs.entries()) { + for (const node of scc) { + obj[node] = index + } + } + return obj + }, +} diff --git a/test/utils/scc.spec.ts b/test/utils/scc.spec.ts new file mode 100644 index 000000000..150575ae6 --- /dev/null +++ b/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 }) + }) + }) + }) + }) +}) diff --git a/yarn.lock b/yarn.lock index 760e281b4..2a185b9c2 100644 --- a/yarn.lock +++ b/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"