Skip to content

Commit cb0b86e

Browse files
webpro43081j
andauthored
fix(prefer-array-to-sorted): skip non-array iterable spreads (#134)
* fix(prefer-array-to-sorted): bail on non-array iterable spread without type info Avoid unsafe untyped autofixes for spread-copy sorts whose source is known not to be an array. These rewrites can otherwise turn Set/Map/iterator sources into invalid `.toSorted()` calls. Bail on known collection constructors, local identifiers initialized from those constructors, iterator-returning `.entries()`/`.keys()`/`.values()` calls, and TS syntax-only iterable annotations when type services are unavailable. Keep `Object.entries()` and known array sources fixable. Verified with the focused rule test, lint, build, and a consumer probe across representative checked-out projects. * chore: drop casts * chore: remove redundant check Variable IDs are the same as their declarator's name, so no need checking it again. --------- Co-authored-by: James Garbutt <43081j@users.noreply.github.com>
1 parent b689174 commit cb0b86e

2 files changed

Lines changed: 164 additions & 2 deletions

File tree

src/rules/prefer-array-to-sorted.test.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ const ruleTester = new RuleTester({
2525
sourceType: 'module'
2626
}
2727
});
28+
const tsSyntaxRuleTester = new TSRuleTester({
29+
languageOptions: {
30+
parserOptions: {
31+
ecmaVersion: 2022,
32+
sourceType: 'module'
33+
}
34+
}
35+
});
2836

2937
ruleTester.run(
3038
'prefer-array-to-sorted (untyped)',
@@ -40,7 +48,17 @@ ruleTester.run(
4048

4149
// slice with different arguments
4250
'arr.slice(1).sort();',
43-
'arr.slice(0, 5).sort();'
51+
'arr.slice(0, 5).sort();',
52+
53+
'const sorted = [...new Set([1, 2, 3])].sort();',
54+
'const sorted = [...new Set(arr)].sort();',
55+
'const sorted = [...new Map()].sort();',
56+
'const foundVersions = new Set(); const sorted = [...foundVersions].sort();',
57+
'let foundVersions = new Set(); const sorted = [...foundVersions].sort();',
58+
'const entries = new Map(); const sorted = [...entries].sort();',
59+
'const sorted = [...items.entries()].sort();',
60+
'const sorted = [...items.keys()].sort();',
61+
'const sorted = [...items.values()].sort();'
4462
],
4563

4664
invalid: [
@@ -174,6 +192,12 @@ ruleTester.run(
174192
}
175193
]
176194
},
195+
{
196+
code: 'const sorted = [...Object.entries(counts)].sort(([a], [b]) => a.localeCompare(b));',
197+
output:
198+
'const sorted = Object.entries(counts).toSorted(([a], [b]) => a.localeCompare(b));',
199+
errors: [{messageId: 'preferToSorted'}]
200+
},
177201

178202
// Multiple arguments (edge case - sort() typically takes 0 or 1 arg)
179203
{
@@ -244,6 +268,26 @@ ruleTester.run(
244268
}
245269
);
246270

271+
tsSyntaxRuleTester.run(
272+
'prefer-array-to-sorted (typescript syntax)',
273+
preferArrayToSorted,
274+
{
275+
valid: [
276+
'function sortItems<T>(items: Iterable<T>) { return [...items].sort(); }',
277+
'function sortItems(items: Map<string, number>) { return [...items.entries()].sort(); }',
278+
'const items: Set<string> = new Set(); const sorted = [...items].sort();',
279+
'let items: Iterable<string>; const sorted = [...items].sort();'
280+
],
281+
invalid: [
282+
{
283+
code: 'const items: string[] = []; const sorted = [...items].sort();',
284+
output: 'const items: string[] = []; const sorted = items.toSorted();',
285+
errors: [{messageId: 'preferToSorted'}]
286+
}
287+
]
288+
}
289+
);
290+
247291
typedRuleTester.run('prefer-array-to-sorted (typed)', preferArrayToSorted, {
248292
valid: [
249293
// Set spread - Set doesn't have toSorted()

src/rules/prefer-array-to-sorted.ts

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,116 @@ import {isArrayType} from '../utils/typescript.js';
99

1010
type MessageIds = 'preferToSorted';
1111

12+
const NON_ARRAY_COPY_SOURCE_CTORS = new Set(['Set', 'Map']);
13+
const NON_ARRAY_COPY_SOURCE_TYPES = new Set([
14+
'Iterable',
15+
'IterableIterator',
16+
'Iterator',
17+
'Map',
18+
'ReadonlyMap',
19+
'ReadonlySet',
20+
'Set',
21+
'URLSearchParams'
22+
]);
23+
const ITERATOR_RETURNING_METHODS = new Set(['entries', 'keys', 'values']);
24+
25+
function getTypeReferenceName(node: TSESTree.Node): string | null {
26+
if (node.type !== 'TSTypeReference') {
27+
return null;
28+
}
29+
30+
const {typeName} = node;
31+
if (typeName.type === 'Identifier') {
32+
return typeName.name;
33+
}
34+
if (typeName.type === 'TSQualifiedName') {
35+
return typeName.right.name;
36+
}
37+
return null;
38+
}
39+
40+
function hasKnownNonArrayTypeAnnotation(node: TSESTree.BindingName): boolean {
41+
const typeAnnotation = node.typeAnnotation;
42+
if (typeAnnotation === undefined) {
43+
return false;
44+
}
45+
46+
const annotation = typeAnnotation.typeAnnotation;
47+
if (annotation.type === 'TSUnionType') {
48+
return annotation.types.some((typeNode) => hasKnownNonArrayType(typeNode));
49+
}
50+
return hasKnownNonArrayType(annotation);
51+
}
52+
53+
function hasKnownNonArrayType(node: TSESTree.Node): boolean {
54+
const typeName = getTypeReferenceName(node);
55+
return typeName !== null && NON_ARRAY_COPY_SOURCE_TYPES.has(typeName);
56+
}
57+
58+
function isIteratorReturningCall(node: TSESTree.Node): boolean {
59+
return (
60+
node.type === 'CallExpression' &&
61+
node.callee.type === 'MemberExpression' &&
62+
node.callee.property.type === 'Identifier' &&
63+
ITERATOR_RETURNING_METHODS.has(node.callee.property.name) &&
64+
!(
65+
node.callee.object.type === 'Identifier' &&
66+
node.callee.object.name === 'Object'
67+
)
68+
);
69+
}
70+
71+
function isKnownNonArrayCopySource(node: TSESTree.Node): boolean {
72+
return (
73+
isIteratorReturningCall(node) ||
74+
(node.type === 'NewExpression' &&
75+
node.callee.type === 'Identifier' &&
76+
NON_ARRAY_COPY_SOURCE_CTORS.has(node.callee.name))
77+
);
78+
}
79+
80+
function resolvesToKnownNonArrayCopySource(
81+
node: TSESTree.Node,
82+
context: TSESLint.RuleContext<MessageIds, []>
83+
): boolean {
84+
if (isKnownNonArrayCopySource(node)) {
85+
return true;
86+
}
87+
88+
if (node.type !== 'Identifier') {
89+
return false;
90+
}
91+
92+
const variable = context.sourceCode
93+
.getScope(node)
94+
.references.find((ref) => ref.identifier === node)?.resolved;
95+
96+
if (!variable || variable.defs.length !== 1) {
97+
return false;
98+
}
99+
100+
const def = variable.defs[0];
101+
if (!def || (def.type !== 'Variable' && def.type !== 'Parameter')) {
102+
return false;
103+
}
104+
105+
if (hasKnownNonArrayTypeAnnotation(def.name)) {
106+
return true;
107+
}
108+
109+
if (def.type !== 'Variable' || def.node.type !== 'VariableDeclarator') {
110+
return false;
111+
}
112+
113+
const init = def.node.init;
114+
115+
return (
116+
init !== null &&
117+
isKnownNonArrayCopySource(init) &&
118+
variable.references.every((ref) => !ref.isWrite() || ref.writeExpr === init)
119+
);
120+
}
121+
12122
export const preferArrayToSorted: TSESLint.RuleModule<MessageIds, []> = {
13123
meta: {
14124
type: 'suggestion',
@@ -40,7 +150,15 @@ export const preferArrayToSorted: TSESLint.RuleModule<MessageIds, []> = {
40150
const arrayNode = getArrayFromCopyPattern(sortCallee);
41151

42152
if (arrayNode) {
43-
if (isArrayType(arrayNode, context) === false) {
153+
const arrayType = isArrayType(arrayNode, context);
154+
if (arrayType === false) {
155+
return;
156+
}
157+
158+
if (
159+
arrayType !== true &&
160+
resolvesToKnownNonArrayCopySource(arrayNode, context)
161+
) {
44162
return;
45163
}
46164

0 commit comments

Comments
 (0)