Skip to content

Commit a47b5b9

Browse files
soryy708ljharb
soryy708
authored and
ljharb
committed
[New] [Refactor] no-cycle: use scc algorithm to optimize; add skipErrorMessagePath for faster error messages
1 parent c0ac54b commit a47b5b9

File tree

6 files changed

+301
-1
lines changed

6 files changed

+301
-1
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
88

99
### Added
1010
- [`dynamic-import-chunkname`]: add `allowEmpty` option to allow empty leading comments ([#2942], thanks [@JiangWeixian])
11+
- [`no-cycle`]: use scc algorithm to optimize; add `skipErrorMessagePath` for faster error messages ([#2998], thanks [@soryy708])
1112

1213
### Changed
1314
- [Docs] `no-extraneous-dependencies`: Make glob pattern description more explicit ([#2944], thanks [@mulztob])
@@ -1115,6 +1116,7 @@ for info on changes for earlier releases.
11151116

11161117
[`memo-parser`]: ./memo-parser/README.md
11171118

1119+
[#2998]: https://github.com/import-js/eslint-plugin-import/pull/2998
11181120
[#2991]: https://github.com/import-js/eslint-plugin-import/pull/2991
11191121
[#2989]: https://github.com/import-js/eslint-plugin-import/pull/2989
11201122
[#2987]: https://github.com/import-js/eslint-plugin-import/pull/2987

‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@
105105
"eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8"
106106
},
107107
"dependencies": {
108+
"@rtsao/scc": "^1.1.0",
108109
"array-includes": "^3.1.7",
109110
"array.prototype.findlastindex": "^1.2.4",
110111
"array.prototype.flat": "^1.3.2",

‎src/rules/no-cycle.js

+23
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import resolve from 'eslint-module-utils/resolve';
77
import ExportMapBuilder from '../exportMap/builder';
8+
import StronglyConnectedComponentsBuilder from '../scc';
89
import { isExternalModule } from '../core/importType';
910
import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor';
1011
import docsUrl from '../docsUrl';
@@ -47,6 +48,11 @@ module.exports = {
4748
type: 'boolean',
4849
default: false,
4950
},
51+
skipErrorMessagePath: {
52+
description: 'for faster performance, but you lose the reported circular route',
53+
type: 'boolean',
54+
default: false,
55+
},
5056
})],
5157
},
5258

@@ -62,6 +68,8 @@ module.exports = {
6268
context,
6369
);
6470

71+
const scc = StronglyConnectedComponentsBuilder.get(myPath, context);
72+
6573
function checkSourceValue(sourceNode, importer) {
6674
if (ignoreModule(sourceNode.value)) {
6775
return; // ignore external modules
@@ -98,6 +106,21 @@ module.exports = {
98106
return; // no-self-import territory
99107
}
100108

109+
/* If we're in the same Strongly Connected Component,
110+
* Then there exists a path from each node in the SCC to every other node in the SCC,
111+
* Then there exists at least one path from them to us and from us to them,
112+
* Then we have a cycle between us.
113+
*/
114+
const hasDependencyCycle = scc[myPath] === scc[imported.path];
115+
if (!hasDependencyCycle) {
116+
return;
117+
}
118+
119+
if (options.skipErrorMessagePath) {
120+
context.report(importer, 'Dependency cycle detected.');
121+
return;
122+
}
123+
101124
const untraversed = [{ mget: () => imported, route: [] }];
102125
function detectCycle({ mget, route }) {
103126
const m = mget();

‎src/scc.js

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import calculateScc from '@rtsao/scc';
2+
import { hashObject } from 'eslint-module-utils/hash';
3+
import resolve from 'eslint-module-utils/resolve';
4+
import ExportMapBuilder from './exportMap/builder';
5+
import childContext from './exportMap/childContext';
6+
7+
let cache = new Map();
8+
9+
export default class StronglyConnectedComponentsBuilder {
10+
static clearCache() {
11+
cache = new Map();
12+
}
13+
14+
static get(source, context) {
15+
const path = resolve(source, context);
16+
if (path == null) { return null; }
17+
return StronglyConnectedComponentsBuilder.for(childContext(path, context));
18+
}
19+
20+
static for(context) {
21+
const cacheKey = context.cacheKey || hashObject(context).digest('hex');
22+
if (cache.has(cacheKey)) {
23+
return cache.get(cacheKey);
24+
}
25+
const scc = StronglyConnectedComponentsBuilder.calculate(context);
26+
cache.set(cacheKey, scc);
27+
return scc;
28+
}
29+
30+
static calculate(context) {
31+
const exportMap = ExportMapBuilder.for(context);
32+
const adjacencyList = this.exportMapToAdjacencyList(exportMap);
33+
const calculatedScc = calculateScc(adjacencyList);
34+
return StronglyConnectedComponentsBuilder.calculatedSccToPlainObject(calculatedScc);
35+
}
36+
37+
/** @returns {Map<string, Set<string>>} for each dep, what are its direct deps */
38+
static exportMapToAdjacencyList(initialExportMap) {
39+
const adjacencyList = new Map();
40+
// BFS
41+
function visitNode(exportMap) {
42+
if (!exportMap) {
43+
return;
44+
}
45+
exportMap.imports.forEach((v, importedPath) => {
46+
const from = exportMap.path;
47+
const to = importedPath;
48+
49+
if (!adjacencyList.has(from)) {
50+
adjacencyList.set(from, new Set());
51+
}
52+
53+
if (adjacencyList.get(from).has(to)) {
54+
return; // prevent endless loop
55+
}
56+
adjacencyList.get(from).add(to);
57+
visitNode(v.getter());
58+
});
59+
}
60+
visitNode(initialExportMap);
61+
// Fill gaps
62+
adjacencyList.forEach((values) => {
63+
values.forEach((value) => {
64+
if (!adjacencyList.has(value)) {
65+
adjacencyList.set(value, new Set());
66+
}
67+
});
68+
});
69+
return adjacencyList;
70+
}
71+
72+
/** @returns {Record<string, number>} for each key, its SCC's index */
73+
static calculatedSccToPlainObject(sccs) {
74+
const obj = {};
75+
sccs.forEach((scc, index) => {
76+
scc.forEach((node) => {
77+
obj[node] = index;
78+
});
79+
});
80+
return obj;
81+
}
82+
}

‎tests/src/rules/no-cycle.js

+28-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const testVersion = (specifier, t) => _testVersion(specifier, () => Object.assig
1717

1818
const testDialects = ['es6'];
1919

20-
ruleTester.run('no-cycle', rule, {
20+
const cases = {
2121
valid: [].concat(
2222
// this rule doesn't care if the cycle length is 0
2323
test({ code: 'import foo from "./foo.js"' }),
@@ -290,4 +290,31 @@ ruleTester.run('no-cycle', rule, {
290290
],
291291
}),
292292
),
293+
};
294+
295+
ruleTester.run('no-cycle', rule, {
296+
valid: flatMap(cases.valid, (testCase) => [
297+
testCase,
298+
{
299+
...testCase,
300+
code: `${testCase.code} // skipErrorMessagePath=true`,
301+
options: [{
302+
...testCase.options && testCase.options[0] || {},
303+
skipErrorMessagePath: true,
304+
}],
305+
},
306+
]),
307+
308+
invalid: flatMap(cases.invalid, (testCase) => [
309+
testCase,
310+
{
311+
...testCase,
312+
code: `${testCase.code} // skipErrorMessagePath=true`,
313+
options: [{
314+
...testCase.options && testCase.options[0] || {},
315+
skipErrorMessagePath: true,
316+
}],
317+
errors: [error(`Dependency cycle detected.`)],
318+
},
319+
]),
293320
});

‎tests/src/scc.js

+165
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import sinon from 'sinon';
2+
import { expect } from 'chai';
3+
import StronglyConnectedComponentsBuilder from '../../src/scc';
4+
import ExportMapBuilder from '../../src/exportMap/builder';
5+
6+
function exportMapFixtureBuilder(path, imports) {
7+
return {
8+
path,
9+
imports: new Map(imports.map((imp) => [imp.path, { getter: () => imp }])),
10+
};
11+
}
12+
13+
describe('Strongly Connected Components Builder', () => {
14+
afterEach(() => ExportMapBuilder.for.restore());
15+
afterEach(() => StronglyConnectedComponentsBuilder.clearCache());
16+
17+
describe('When getting an SCC', () => {
18+
const source = '';
19+
const context = {
20+
settings: {},
21+
parserOptions: {},
22+
parserPath: '',
23+
};
24+
25+
describe('Given two files', () => {
26+
describe('When they don\'t cycle', () => {
27+
it('Should return foreign SCCs', () => {
28+
sinon.stub(ExportMapBuilder, 'for').returns(
29+
exportMapFixtureBuilder('foo.js', [exportMapFixtureBuilder('bar.js', [])]),
30+
);
31+
const actual = StronglyConnectedComponentsBuilder.for(source, context);
32+
expect(actual).to.deep.equal({ 'foo.js': 1, 'bar.js': 0 });
33+
});
34+
});
35+
36+
describe('When they do cycle', () => {
37+
it('Should return same SCC', () => {
38+
sinon.stub(ExportMapBuilder, 'for').returns(
39+
exportMapFixtureBuilder('foo.js', [
40+
exportMapFixtureBuilder('bar.js', [
41+
exportMapFixtureBuilder('foo.js', []),
42+
]),
43+
]),
44+
);
45+
const actual = StronglyConnectedComponentsBuilder.for(source, context);
46+
expect(actual).to.deep.equal({ 'foo.js': 0, 'bar.js': 0 });
47+
});
48+
});
49+
});
50+
51+
describe('Given three files', () => {
52+
describe('When they form a line', () => {
53+
describe('When A -> B -> C', () => {
54+
it('Should return foreign SCCs', () => {
55+
sinon.stub(ExportMapBuilder, 'for').returns(
56+
exportMapFixtureBuilder('foo.js', [
57+
exportMapFixtureBuilder('bar.js', [
58+
exportMapFixtureBuilder('buzz.js', []),
59+
]),
60+
]),
61+
);
62+
const actual = StronglyConnectedComponentsBuilder.for(source, context);
63+
expect(actual).to.deep.equal({ 'foo.js': 2, 'bar.js': 1, 'buzz.js': 0 });
64+
});
65+
});
66+
67+
describe('When A -> B <-> C', () => {
68+
it('Should return 2 SCCs, A on its own', () => {
69+
sinon.stub(ExportMapBuilder, 'for').returns(
70+
exportMapFixtureBuilder('foo.js', [
71+
exportMapFixtureBuilder('bar.js', [
72+
exportMapFixtureBuilder('buzz.js', [
73+
exportMapFixtureBuilder('bar.js', []),
74+
]),
75+
]),
76+
]),
77+
);
78+
const actual = StronglyConnectedComponentsBuilder.for(source, context);
79+
expect(actual).to.deep.equal({ 'foo.js': 1, 'bar.js': 0, 'buzz.js': 0 });
80+
});
81+
});
82+
83+
describe('When A <-> B -> C', () => {
84+
it('Should return 2 SCCs, C on its own', () => {
85+
sinon.stub(ExportMapBuilder, 'for').returns(
86+
exportMapFixtureBuilder('foo.js', [
87+
exportMapFixtureBuilder('bar.js', [
88+
exportMapFixtureBuilder('buzz.js', []),
89+
exportMapFixtureBuilder('foo.js', []),
90+
]),
91+
]),
92+
);
93+
const actual = StronglyConnectedComponentsBuilder.for(source, context);
94+
expect(actual).to.deep.equal({ 'foo.js': 1, 'bar.js': 1, 'buzz.js': 0 });
95+
});
96+
});
97+
98+
describe('When A <-> B <-> C', () => {
99+
it('Should return same SCC', () => {
100+
sinon.stub(ExportMapBuilder, 'for').returns(
101+
exportMapFixtureBuilder('foo.js', [
102+
exportMapFixtureBuilder('bar.js', [
103+
exportMapFixtureBuilder('foo.js', []),
104+
exportMapFixtureBuilder('buzz.js', [
105+
exportMapFixtureBuilder('bar.js', []),
106+
]),
107+
]),
108+
]),
109+
);
110+
const actual = StronglyConnectedComponentsBuilder.for(source, context);
111+
expect(actual).to.deep.equal({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 });
112+
});
113+
});
114+
});
115+
116+
describe('When they form a loop', () => {
117+
it('Should return same SCC', () => {
118+
sinon.stub(ExportMapBuilder, 'for').returns(
119+
exportMapFixtureBuilder('foo.js', [
120+
exportMapFixtureBuilder('bar.js', [
121+
exportMapFixtureBuilder('buzz.js', [
122+
exportMapFixtureBuilder('foo.js', []),
123+
]),
124+
]),
125+
]),
126+
);
127+
const actual = StronglyConnectedComponentsBuilder.for(source, context);
128+
expect(actual).to.deep.equal({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 });
129+
});
130+
});
131+
132+
describe('When they form a Y', () => {
133+
it('Should return 3 distinct SCCs', () => {
134+
sinon.stub(ExportMapBuilder, 'for').returns(
135+
exportMapFixtureBuilder('foo.js', [
136+
exportMapFixtureBuilder('bar.js', []),
137+
exportMapFixtureBuilder('buzz.js', []),
138+
]),
139+
);
140+
const actual = StronglyConnectedComponentsBuilder.for(source, context);
141+
expect(actual).to.deep.equal({ 'foo.js': 2, 'bar.js': 0, 'buzz.js': 1 });
142+
});
143+
});
144+
145+
describe('When they form a Mercedes', () => {
146+
it('Should return 1 SCC', () => {
147+
sinon.stub(ExportMapBuilder, 'for').returns(
148+
exportMapFixtureBuilder('foo.js', [
149+
exportMapFixtureBuilder('bar.js', [
150+
exportMapFixtureBuilder('foo.js', []),
151+
exportMapFixtureBuilder('buzz.js', []),
152+
]),
153+
exportMapFixtureBuilder('buzz.js', [
154+
exportMapFixtureBuilder('foo.js', []),
155+
exportMapFixtureBuilder('bar.js', []),
156+
]),
157+
]),
158+
);
159+
const actual = StronglyConnectedComponentsBuilder.for(source, context);
160+
expect(actual).to.deep.equal({ 'foo.js': 0, 'bar.js': 0, 'buzz.js': 0 });
161+
});
162+
});
163+
});
164+
});
165+
});

0 commit comments

Comments
 (0)