Skip to content

Commit a87c296

Browse files
yeonjuanJoshuaKGoldberg
yeonjuan
and
JoshuaKGoldberg
authored
fix(eslint-plugin): [no-loop-func] sync from upstream base rule (#10103)
* fix(eslint-plugin): [no-loop-func] sync from upstream base rule * update commit --------- Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
1 parent 656a36e commit a87c296

File tree

2 files changed

+384
-218
lines changed

2 files changed

+384
-218
lines changed

Diff for: ‎packages/eslint-plugin/src/rules/no-loop-func.ts

+189-147
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,168 @@ export default createRule<Options, MessageIds>({
3030
},
3131
defaultOptions: [],
3232
create(context) {
33+
const SKIPPED_IIFE_NODES = new Set<
34+
| TSESTree.ArrowFunctionExpression
35+
| TSESTree.FunctionDeclaration
36+
| TSESTree.FunctionExpression
37+
>();
38+
39+
/**
40+
* Gets the containing loop node of a specified node.
41+
*
42+
* We don't need to check nested functions, so this ignores those.
43+
* `Scope.through` contains references of nested functions.
44+
*
45+
* @param node An AST node to get.
46+
* @returns The containing loop node of the specified node, or `null`.
47+
*/
48+
function getContainingLoopNode(node: TSESTree.Node): TSESTree.Node | null {
49+
for (
50+
let currentNode = node;
51+
currentNode.parent;
52+
currentNode = currentNode.parent
53+
) {
54+
const parent = currentNode.parent;
55+
56+
switch (parent.type) {
57+
case AST_NODE_TYPES.WhileStatement:
58+
case AST_NODE_TYPES.DoWhileStatement:
59+
return parent;
60+
61+
case AST_NODE_TYPES.ForStatement:
62+
// `init` is outside of the loop.
63+
if (parent.init !== currentNode) {
64+
return parent;
65+
}
66+
break;
67+
68+
case AST_NODE_TYPES.ForInStatement:
69+
case AST_NODE_TYPES.ForOfStatement:
70+
// `right` is outside of the loop.
71+
if (parent.right !== currentNode) {
72+
return parent;
73+
}
74+
break;
75+
76+
case AST_NODE_TYPES.ArrowFunctionExpression:
77+
case AST_NODE_TYPES.FunctionExpression:
78+
case AST_NODE_TYPES.FunctionDeclaration:
79+
// We don't need to check nested functions.
80+
81+
// We need to check nested functions only in case of IIFE.
82+
if (SKIPPED_IIFE_NODES.has(parent)) {
83+
break;
84+
}
85+
return null;
86+
87+
default:
88+
break;
89+
}
90+
}
91+
92+
return null;
93+
}
94+
95+
/**
96+
* Gets the containing loop node of a given node.
97+
* If the loop was nested, this returns the most outer loop.
98+
* @param node A node to get. This is a loop node.
99+
* @param excludedNode A node that the result node should not include.
100+
* @returns The most outer loop node.
101+
*/
102+
function getTopLoopNode(
103+
node: TSESTree.Node,
104+
excludedNode: TSESTree.Node | null | undefined,
105+
): TSESTree.Node {
106+
const border = excludedNode ? excludedNode.range[1] : 0;
107+
let retv = node;
108+
let containingLoopNode: TSESTree.Node | null = node;
109+
110+
while (containingLoopNode && containingLoopNode.range[0] >= border) {
111+
retv = containingLoopNode;
112+
containingLoopNode = getContainingLoopNode(containingLoopNode);
113+
}
114+
115+
return retv;
116+
}
117+
118+
/**
119+
* Checks whether a given reference which refers to an upper scope's variable is
120+
* safe or not.
121+
* @param loopNode A containing loop node.
122+
* @param reference A reference to check.
123+
* @returns `true` if the reference is safe or not.
124+
*/
125+
function isSafe(
126+
loopNode: TSESTree.Node,
127+
reference: TSESLint.Scope.Reference,
128+
): boolean {
129+
const variable = reference.resolved;
130+
const definition = variable?.defs[0];
131+
const declaration = definition?.parent;
132+
const kind =
133+
declaration?.type === AST_NODE_TYPES.VariableDeclaration
134+
? declaration.kind
135+
: '';
136+
137+
// type references are all safe
138+
// this only really matters for global types that haven't been configured
139+
if (reference.isTypeReference) {
140+
return true;
141+
}
142+
143+
// Variables which are declared by `const` is safe.
144+
if (kind === 'const') {
145+
return true;
146+
}
147+
148+
/*
149+
* Variables which are declared by `let` in the loop is safe.
150+
* It's a different instance from the next loop step's.
151+
*/
152+
if (
153+
kind === 'let' &&
154+
declaration &&
155+
declaration.range[0] > loopNode.range[0] &&
156+
declaration.range[1] < loopNode.range[1]
157+
) {
158+
return true;
159+
}
160+
161+
/*
162+
* WriteReferences which exist after this border are unsafe because those
163+
* can modify the variable.
164+
*/
165+
const border = getTopLoopNode(
166+
loopNode,
167+
kind === 'let' ? declaration : null,
168+
).range[0];
169+
170+
/**
171+
* Checks whether a given reference is safe or not.
172+
* The reference is every reference of the upper scope's variable we are
173+
* looking now.
174+
*
175+
* It's safe if the reference matches one of the following condition.
176+
* - is readonly.
177+
* - doesn't exist inside a local function and after the border.
178+
*
179+
* @param upperRef A reference to check.
180+
* @returns `true` if the reference is safe.
181+
*/
182+
function isSafeReference(upperRef: TSESLint.Scope.Reference): boolean {
183+
const id = upperRef.identifier;
184+
185+
return (
186+
!upperRef.isWrite() ||
187+
(variable?.scope.variableScope === upperRef.from.variableScope &&
188+
id.range[0] < border)
189+
);
190+
}
191+
192+
return variable?.references.every(isSafeReference) ?? false;
193+
}
194+
33195
/**
34196
* Reports functions which match the following condition:
35197
* - has a loop node in ancestors.
@@ -50,8 +212,25 @@ export default createRule<Options, MessageIds>({
50212
}
51213

52214
const references = context.sourceCode.getScope(node).through;
215+
216+
if (!(node.async || node.generator) && isIIFE(node)) {
217+
const isFunctionExpression =
218+
node.type === AST_NODE_TYPES.FunctionExpression;
219+
220+
// Check if the function is referenced elsewhere in the code
221+
const isFunctionReferenced =
222+
isFunctionExpression && node.id
223+
? references.some(r => r.identifier.name === node.id?.name)
224+
: false;
225+
226+
if (!isFunctionReferenced) {
227+
SKIPPED_IIFE_NODES.add(node);
228+
return;
229+
}
230+
}
231+
53232
const unsafeRefs = references
54-
.filter(r => !isSafe(loopNode, r))
233+
.filter(r => r.resolved && !isSafe(loopNode, r))
55234
.map(r => r.identifier.name);
56235

57236
if (unsafeRefs.length > 0) {
@@ -71,151 +250,14 @@ export default createRule<Options, MessageIds>({
71250
},
72251
});
73252

74-
/**
75-
* Gets the containing loop node of a specified node.
76-
*
77-
* We don't need to check nested functions, so this ignores those.
78-
* `Scope.through` contains references of nested functions.
79-
*
80-
* @param node An AST node to get.
81-
* @returns The containing loop node of the specified node, or `null`.
82-
*/
83-
function getContainingLoopNode(node: TSESTree.Node): TSESTree.Node | null {
84-
for (
85-
let currentNode = node;
86-
currentNode.parent;
87-
currentNode = currentNode.parent
88-
) {
89-
const parent = currentNode.parent;
90-
91-
switch (parent.type) {
92-
case AST_NODE_TYPES.WhileStatement:
93-
case AST_NODE_TYPES.DoWhileStatement:
94-
return parent;
95-
96-
case AST_NODE_TYPES.ForStatement:
97-
// `init` is outside of the loop.
98-
if (parent.init !== currentNode) {
99-
return parent;
100-
}
101-
break;
102-
103-
case AST_NODE_TYPES.ForInStatement:
104-
case AST_NODE_TYPES.ForOfStatement:
105-
// `right` is outside of the loop.
106-
if (parent.right !== currentNode) {
107-
return parent;
108-
}
109-
break;
110-
111-
case AST_NODE_TYPES.ArrowFunctionExpression:
112-
case AST_NODE_TYPES.FunctionExpression:
113-
case AST_NODE_TYPES.FunctionDeclaration:
114-
// We don't need to check nested functions.
115-
return null;
116-
117-
default:
118-
break;
119-
}
120-
}
121-
122-
return null;
123-
}
124-
125-
/**
126-
* Gets the containing loop node of a given node.
127-
* If the loop was nested, this returns the most outer loop.
128-
* @param node A node to get. This is a loop node.
129-
* @param excludedNode A node that the result node should not include.
130-
* @returns The most outer loop node.
131-
*/
132-
function getTopLoopNode(
133-
node: TSESTree.Node,
134-
excludedNode: TSESTree.Node | null | undefined,
135-
): TSESTree.Node {
136-
const border = excludedNode ? excludedNode.range[1] : 0;
137-
let retv = node;
138-
let containingLoopNode: TSESTree.Node | null = node;
139-
140-
while (containingLoopNode && containingLoopNode.range[0] >= border) {
141-
retv = containingLoopNode;
142-
containingLoopNode = getContainingLoopNode(containingLoopNode);
143-
}
144-
145-
return retv;
146-
}
147-
148-
/**
149-
* Checks whether a given reference which refers to an upper scope's variable is
150-
* safe or not.
151-
* @param loopNode A containing loop node.
152-
* @param reference A reference to check.
153-
* @returns `true` if the reference is safe or not.
154-
*/
155-
function isSafe(
156-
loopNode: TSESTree.Node,
157-
reference: TSESLint.Scope.Reference,
253+
function isIIFE(
254+
node:
255+
| TSESTree.ArrowFunctionExpression
256+
| TSESTree.FunctionDeclaration
257+
| TSESTree.FunctionExpression,
158258
): boolean {
159-
const variable = reference.resolved;
160-
const definition = variable?.defs[0];
161-
const declaration = definition?.parent;
162-
const kind =
163-
declaration?.type === AST_NODE_TYPES.VariableDeclaration
164-
? declaration.kind
165-
: '';
166-
167-
// type references are all safe
168-
// this only really matters for global types that haven't been configured
169-
if (reference.isTypeReference) {
170-
return true;
171-
}
172-
173-
// Variables which are declared by `const` is safe.
174-
if (kind === 'const') {
175-
return true;
176-
}
177-
178-
/*
179-
* Variables which are declared by `let` in the loop is safe.
180-
* It's a different instance from the next loop step's.
181-
*/
182-
if (
183-
kind === 'let' &&
184-
declaration &&
185-
declaration.range[0] > loopNode.range[0] &&
186-
declaration.range[1] < loopNode.range[1]
187-
) {
188-
return true;
189-
}
190-
191-
/*
192-
* WriteReferences which exist after this border are unsafe because those
193-
* can modify the variable.
194-
*/
195-
const border = getTopLoopNode(loopNode, kind === 'let' ? declaration : null)
196-
.range[0];
197-
198-
/**
199-
* Checks whether a given reference is safe or not.
200-
* The reference is every reference of the upper scope's variable we are
201-
* looking now.
202-
*
203-
* It's safe if the reference matches one of the following condition.
204-
* - is readonly.
205-
* - doesn't exist inside a local function and after the border.
206-
*
207-
* @param upperRef A reference to check.
208-
* @returns `true` if the reference is safe.
209-
*/
210-
function isSafeReference(upperRef: TSESLint.Scope.Reference): boolean {
211-
const id = upperRef.identifier;
212-
213-
return (
214-
!upperRef.isWrite() ||
215-
(variable?.scope.variableScope === upperRef.from.variableScope &&
216-
id.range[0] < border)
217-
);
218-
}
219-
220-
return variable?.references.every(isSafeReference) ?? false;
259+
return (
260+
node.parent.type === AST_NODE_TYPES.CallExpression &&
261+
node.parent.callee === node
262+
);
221263
}

0 commit comments

Comments
 (0)