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/src/rules/no-cycle.ts b/src/rules/no-cycle.ts index da94205b6..f06b120c8 100644 --- a/src/rules/no-cycle.ts +++ b/src/rules/no-cycle.ts @@ -88,7 +88,7 @@ export = createRule<[Options?], MessageId>({ options.ignoreExternal && isExternalModule(name, resolve(name, context)!, context) - const scc = StronglyConnectedComponents.get(filename, context); + const scc = StronglyConnectedComponents.get(filename, context) return { ...moduleVisitor(function checkSourceValue(sourceNode, importer) { @@ -135,9 +135,9 @@ export = createRule<[Options?], MessageId>({ * Then we have a cycle between us. */ if (scc) { - const hasDependencyCycle = scc[filename] === scc[imported.path]; + const hasDependencyCycle = scc[filename] === scc[imported.path] if (!hasDependencyCycle) { - return; + return } } diff --git a/src/utils/scc.ts b/src/utils/scc.ts index 87873946b..2d4b71149 100644 --- a/src/utils/scc.ts +++ b/src/utils/scc.ts @@ -1,83 +1,91 @@ -import calculateScc from '@rtsao/scc'; -import { resolve } from './resolve'; -import { ExportMap, childContext } from './export-map'; -import type { ChildContext, RuleContext } from '../types'; +import calculateScc from '@rtsao/scc' -let cache = new Map>(); +import type { ChildContext, RuleContext } from '../types' -export class StronglyConnectedComponents { - static clearCache() { +import { ExportMap, childContext } from './export-map' +import { resolve } from './resolve' + +const cache = new Map>() + +export const StronglyConnectedComponents = { + clearCache() { cache.clear() - } + }, - static get(source: string, context: RuleContext) { - const path = resolve(source, context); - if (path == null) { return null; } - return StronglyConnectedComponents.for(childContext(path, context)); - } + get(source: string, context: RuleContext) { + const path = resolve(source, context) + if (path == null) { + return null + } + return StronglyConnectedComponents.for(childContext(path, context)) + }, - static for(context: ChildContext) { + for(context: ChildContext) { const cacheKey = context.cacheKey if (cache.has(cacheKey)) { - return cache.get(cacheKey)!; + return cache.get(cacheKey)! } - const scc = StronglyConnectedComponents.calculate(context); - cache.set(cacheKey, scc); - return scc; - } + const scc = StronglyConnectedComponents.calculate(context) + cache.set(cacheKey, scc) + return scc + }, - static calculate(context: ChildContext) { - const exportMap = ExportMap.for(context); - const adjacencyList = StronglyConnectedComponents.exportMapToAdjacencyList(exportMap); - const calculatedScc = calculateScc(adjacencyList); - return StronglyConnectedComponents.calculatedSccToPlainObject(calculatedScc); - } + calculate(context: ChildContext) { + const exportMap = ExportMap.for(context) + const adjacencyList = + StronglyConnectedComponents.exportMapToAdjacencyList(exportMap) + const calculatedScc = calculateScc(adjacencyList) + return StronglyConnectedComponents.calculatedSccToPlainObject(calculatedScc) + }, - static exportMapToAdjacencyList(initialExportMap: ExportMap | null) { + exportMapToAdjacencyList(initialExportMap: ExportMap | null) { /** for each dep, what are its direct deps */ - const adjacencyList = new Map>(); + const adjacencyList = new Map>() // BFS function visitNode(exportMap: ExportMap | null) { if (!exportMap) { - return; + return } - exportMap.imports.forEach((v, importedPath) => { - const from = exportMap.path; - const to = importedPath; + for (const [importedPath, v] of exportMap.imports.entries()) { + const from = exportMap.path + const to = importedPath if (!adjacencyList.has(from)) { - adjacencyList.set(from, new Set()); + adjacencyList.set(from, new Set()) } - const set = adjacencyList.get(from)!; + const set = adjacencyList.get(from)! if (set.has(to)) { - return; // prevent endless loop + continue // prevent endless loop } - set.add(to); - visitNode(v.getter()); - }); + set.add(to) + visitNode(v.getter()) + } } - visitNode(initialExportMap); + visitNode(initialExportMap) // Fill gaps - adjacencyList.forEach((values) => { - values.forEach((value) => { + // 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()); + adjacencyList.set(value, new Set()) } - }); - }); - return adjacencyList; - } + }) + }) - static calculatedSccToPlainObject(sccs: Set[]) { - /** for each key, its SCC's index */ - const obj: Record = {}; - sccs.forEach((scc, index) => { - scc.forEach((node) => { - obj[node] = index; - }); - }); - return obj; - } + 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 index 56f9f0833..150575ae6 100644 --- a/test/utils/scc.spec.ts +++ b/test/utils/scc.spec.ts @@ -1,160 +1,193 @@ // import sinon from 'sinon'; -import { StronglyConnectedComponents, ExportMap, childContext as buildChildContext } from 'eslint-plugin-import-x/utils'; -import { testContext } from '../utils'; +import { testContext } from '../utils' -function exportMapFixtureBuilder(path: string, imports: ExportMap[]): ExportMap { +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; + imports: new Map( + imports.map(imp => [ + imp.path, + { getter: () => imp, declarations: new Set() }, + ]), + ), + } as ExportMap } describe('Strongly Connected Components Builder', () => { - afterEach(() => StronglyConnectedComponents.clearCache()); + afterEach(() => StronglyConnectedComponents.clearCache()) describe('When getting an SCC', () => { - const source = ''; - const ruleContext = testContext({}); - const childContext = buildChildContext(source, ruleContext); + const source = '' + const ruleContext = testContext({}) + const childContext = buildChildContext(source, ruleContext) describe('Given two files', () => { - describe('When they don\'t cycle', () => { + 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 }); - }); - }); + 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', []), + 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 }); - }); - }); - }); + ) + 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', []), + 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 }); - }); - }); + ) + 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', []), + 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 }); - }); - }); + ) + 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', []), + 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 }); - }); - }); + ) + 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', []), + 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 }); - }); - }); - }); + ) + 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', []), + 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 }); - }); - }); + ) + 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 }); - }); - }); + 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', []), + 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 }); - }); - }); - }); - }); -}); + ) + const actual = StronglyConnectedComponents.for(childContext) + expect(actual).toEqual({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 }) + }) + }) + }) + }) +})