Skip to content

Commit af1e2f0

Browse files
cola119cola119
committed
tools: add sort-requires rule
1 parent aa7cd87 commit af1e2f0

4 files changed

Lines changed: 231 additions & 17 deletions

File tree

‎test/parallel/test-eslint-sort-requires.js‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if ((!common.hasCrypto) || (!common.hasIntl)) {
5+
common.skip('ESLint tests require crypto and Intl');
6+
}
7+
8+
common.skipIfEslintMissing();
9+
10+
const RuleTester = require('../../tools/node_modules/eslint').RuleTester;
11+
const rule = require('../../tools/eslint-rules/sort-requires');
12+
13+
new RuleTester({
14+
parserOptions: { ecmaVersion: 6 },
15+
})
16+
.run('sort-requires', rule, {
17+
valid: [
18+
"const A = require('path')",
19+
'const A = primordials',
20+
"const { A } = require('path')",
21+
'const { A } = primordials',
22+
"const { A, B } = require('path')",
23+
'const { A, B } = primordials',
24+
"const { B: a, A: b } = require('path')",
25+
'const { B: a, A: b } = primordials',
26+
`const {
27+
A,
28+
B,
29+
C,
30+
} = require('path')`,
31+
`const {
32+
A,
33+
B,
34+
C,
35+
} = primordials`,
36+
],
37+
invalid: [
38+
{
39+
code: "const { B, A } = require('path')",
40+
errors: [{ message: 'A should occur before B' }],
41+
output: "const { A, B } = require('path')",
42+
},
43+
{
44+
code: 'const { B, A } = primordials',
45+
errors: [{ message: 'A should occur before B' }],
46+
output: 'const { A, B } = primordials',
47+
},
48+
{
49+
code: "const { A: b, B: a } = require('path')",
50+
errors: [{ message: 'a should occur before b' }],
51+
output: "const { B: a, A: b } = require('path')",
52+
},
53+
{
54+
code: 'const { A: b, B: a } = primordials',
55+
errors: [{ message: 'a should occur before b' }],
56+
output: 'const { B: a, A: b } = primordials',
57+
},
58+
{
59+
code: "const { C, B, A } = require('path')",
60+
errors: [{ message: 'B should occur before C' }],
61+
output: "const { A, B, C } = require('path')",
62+
},
63+
{
64+
code: 'const { C, B, A } = primordials',
65+
errors: [{ message: 'B should occur before C' }],
66+
output: 'const { A, B, C } = primordials',
67+
},
68+
// Test for line breaks
69+
{
70+
code: `const {
71+
C,
72+
B,
73+
A
74+
} = require('path')`,
75+
errors: [{ message: 'B should occur before C' }],
76+
output: `const {
77+
A,
78+
B,
79+
C
80+
} = require('path')`,
81+
},
82+
{
83+
code: `const {
84+
C,
85+
B,
86+
A
87+
} = primordials`,
88+
errors: [{ message: 'B should occur before C' }],
89+
output: `const {
90+
A,
91+
B,
92+
C
93+
} = primordials`,
94+
},
95+
// Test for comments
96+
// If there is a comment, this rule changes nothing.
97+
{
98+
code: `const {
99+
// comment for B
100+
B,
101+
A
102+
} = require('path')`,
103+
errors: [{ message: 'A should occur before B' }],
104+
output: null,
105+
},
106+
{
107+
code: `const {
108+
// comment for B
109+
B,
110+
A
111+
} = primordials`,
112+
errors: [{ message: 'A should occur before B' }],
113+
output: null,
114+
},
115+
]
116+
});

‎tools/eslint-rules/no-duplicate-requires.js‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,12 @@
44
*/
55
'use strict';
66

7-
const { isRequireCall, isString } = require('./rules-utils.js');
7+
const { isTopLevelRequireCall, isString } = require('./rules-utils.js');
88

99
//------------------------------------------------------------------------------
1010
// Rule Definition
1111
//------------------------------------------------------------------------------
1212

13-
const secondLevelTypes = [
14-
'FunctionDeclaration', 'FunctionExpression', 'ArrowFunctionExpression',
15-
'ClassBody', 'MethodDefinition',
16-
];
17-
18-
function isTopLevel(node) {
19-
while (!secondLevelTypes.includes(node.type)) {
20-
node = node.parent;
21-
if (!node) {
22-
return true;
23-
}
24-
}
25-
return false;
26-
}
27-
2813
module.exports = (context) => {
2914
if (context.parserOptions.sourceType === 'module') {
3015
return {};
@@ -43,7 +28,7 @@ module.exports = (context) => {
4328

4429
const rules = {
4530
CallExpression: (node) => {
46-
if (isRequireCall(node) && isTopLevel(node)) {
31+
if (isTopLevelRequireCall(node)) {
4732
const moduleName = getRequiredModuleNameFromCall(node);
4833
if (moduleName === undefined) {
4934
return;

‎tools/eslint-rules/rules-utils.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,23 @@ function isRequireCall(node) {
88
}
99
module.exports.isRequireCall = isRequireCall;
1010

11+
module.exports.isTopLevelRequireCall = function(node) {
12+
const secondLevelTypes = [
13+
'FunctionDeclaration', 'FunctionExpression', 'ArrowFunctionExpression',
14+
'ClassBody', 'MethodDefinition',
15+
];
16+
const isTopLevel = (node) => {
17+
while (!secondLevelTypes.includes(node.type)) {
18+
node = node.parent;
19+
if (!node) {
20+
return true;
21+
}
22+
}
23+
return false;
24+
};
25+
return isRequireCall(node) && isTopLevel(node);
26+
};
27+
1128
module.exports.isString = function(node) {
1229
return node && node.type === 'Literal' && typeof node.value === 'string';
1330
};

‎tools/eslint-rules/sort-requires.js‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
'use strict';
2+
3+
const { isTopLevelRequireCall } = require('./rules-utils.js');
4+
5+
const getRequiredPropertiesFromRequireCallExpression = (node) => {
6+
if (node?.parent?.id?.type === 'ObjectPattern') {
7+
return node.parent.id.properties;
8+
}
9+
};
10+
11+
const isPrimordialsDeclarator = (node) => {
12+
return (
13+
node?.type === 'VariableDeclarator' && node?.init?.name === 'primordials'
14+
);
15+
};
16+
17+
const getRequiredPropertiesFromPrimordialsDeclarator = (node) => {
18+
if (node?.id?.type === 'ObjectPattern') {
19+
return node.id.properties;
20+
}
21+
};
22+
23+
const getPropertyValueName = (property) => property.value.name;
24+
const compareProperty = (a, b) => (getPropertyValueName(a) > getPropertyValueName(b) ? 1 : -1);
25+
26+
const findOutOfOrderIndex = (properties) => {
27+
for (let i = 0; i < properties.length - 1; i++) {
28+
if (compareProperty(properties[i], properties[i + 1]) > 0) return i;
29+
}
30+
return -1;
31+
};
32+
33+
module.exports = {
34+
meta: {
35+
fixable: 'code',
36+
},
37+
create(context) {
38+
const sourceCode = context.getSourceCode();
39+
const hasComment = (nodeOrToken) =>
40+
sourceCode.getCommentsBefore(nodeOrToken).length ||
41+
sourceCode.getCommentsAfter(nodeOrToken).length;
42+
43+
const reportOutOfOrder = (target, requiredProperties, outOfOrderIndex) => {
44+
const before = requiredProperties[outOfOrderIndex];
45+
const after = requiredProperties[outOfOrderIndex + 1];
46+
context.report({
47+
node: target,
48+
message: `${after.value.name} should occur before ${before.value.name}`,
49+
fix(fixer) {
50+
if (requiredProperties.some(hasComment)) {
51+
return null;
52+
}
53+
return fixer.replaceTextRange([
54+
requiredProperties[0].range[0],
55+
requiredProperties[requiredProperties.length - 1].range[1],
56+
], requiredProperties
57+
.slice() // to avoid mutation
58+
.sort(compareProperty)
59+
.reduce((text, property, index) => {
60+
const suffix =
61+
index === requiredProperties.length - 1 ?
62+
'' :
63+
sourceCode
64+
.getText()
65+
.slice(
66+
requiredProperties[index].range[1],
67+
requiredProperties[index + 1].range[0]
68+
);
69+
return text + sourceCode.getText(property) + suffix;
70+
}, ''));
71+
},
72+
});
73+
};
74+
75+
return {
76+
CallExpression(node) {
77+
if (!isTopLevelRequireCall(node)) return;
78+
const requiredProperties =
79+
getRequiredPropertiesFromRequireCallExpression(node);
80+
if (!requiredProperties) return;
81+
const outOfOrderIndex = findOutOfOrderIndex(requiredProperties);
82+
if (outOfOrderIndex === -1) return;
83+
reportOutOfOrder(node, requiredProperties, outOfOrderIndex);
84+
},
85+
VariableDeclarator(node) {
86+
if (!isPrimordialsDeclarator(node)) return;
87+
const requiredProperties =
88+
getRequiredPropertiesFromPrimordialsDeclarator(node);
89+
if (!requiredProperties) return;
90+
const outOfOrderIndex = findOutOfOrderIndex(requiredProperties);
91+
if (outOfOrderIndex === -1) return;
92+
reportOutOfOrder(node, requiredProperties, outOfOrderIndex);
93+
},
94+
};
95+
},
96+
};

0 commit comments

Comments
 (0)