Skip to content

Commit 03a93f4

Browse files
ota-meshiota-meshi
authored
fix: preventing infinite loops (#1344)
1 parent c977540 commit 03a93f4

21 files changed

+371
-62
lines changed

‎.changeset/shy-countries-carry.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'eslint-plugin-svelte': patch
3+
---
4+
5+
fix: preventing infinite loops in multiple rules

‎packages/eslint-plugin-svelte/eslint.config.mjs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import * as myPlugin from '@ota-meshi/eslint-plugin';
22
import * as tseslint from 'typescript-eslint';
3+
import { createJiti } from 'jiti';
4+
const jiti = createJiti(import.meta.url);
5+
const internal = {
6+
rules: {
7+
'prefer-find-variable-safe': await jiti.import('./internal-rules/prefer-find-variable-safe.ts')
8+
}
9+
};
310

411
/**
512
* @type {import('eslint').Linter.Config[]}
@@ -54,6 +61,9 @@ const config = [
5461
},
5562
{
5663
files: ['src/**'],
64+
plugins: {
65+
internal
66+
},
5767
rules: {
5868
'@typescript-eslint/no-restricted-imports': [
5969
'error',
@@ -80,7 +90,8 @@ const config = [
8090
{ object: 'context', property: 'getCwd', message: 'Use `context.cwd`' },
8191
{ object: 'context', property: 'getScope', message: 'Use src/utils/compat.ts' },
8292
{ object: 'context', property: 'parserServices', message: 'Use src/utils/compat.ts' }
83-
]
93+
],
94+
'internal/prefer-find-variable-safe': 'error'
8495
}
8596
},
8697
...tseslint.config({

‎packages/eslint-plugin-svelte/internal-rules/prefer-find-variable-safe.ts‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
import type { Rule } from 'eslint';
2+
import { ReferenceTracker, findVariable } from '@eslint-community/eslint-utils';
3+
import path from 'node:path';
4+
import type { TSESTree } from '@typescript-eslint/types';
5+
import type { Variable } from '@typescript-eslint/scope-manager';
6+
7+
export default {
8+
meta: {
9+
docs: {
10+
description: 'enforce to use FindVariableContext to avoid infinite recursion',
11+
category: 'Best Practices',
12+
recommended: false,
13+
conflictWithPrettier: false,
14+
url: 'https://github.com/sveltejs/eslint-plugin-svelte/blob/v3.12.3/docs/rules/prefer-find-variable-safe.md'
15+
},
16+
messages: {
17+
preferFindVariableSafe: 'Prefer to use FindVariableContext to avoid infinite recursion.'
18+
},
19+
schema: [],
20+
type: 'suggestion'
21+
},
22+
create(context: Rule.RuleContext): Rule.RuleListener {
23+
const referenceTracker = new ReferenceTracker(
24+
context.sourceCode.scopeManager.globalScope as never
25+
);
26+
let astUtilsPath = path.relative(
27+
path.dirname(context.physicalFilename),
28+
path.join(import.meta.dirname, '..', 'src', 'utils', 'ast-utils')
29+
);
30+
if (!astUtilsPath.startsWith('.')) {
31+
astUtilsPath = `./${astUtilsPath}`;
32+
}
33+
const findVariableCalls = [
34+
...referenceTracker.iterateEsmReferences({
35+
[astUtilsPath]: {
36+
[ReferenceTracker.ESM]: true,
37+
findVariable: {
38+
[ReferenceTracker.CALL]: true
39+
}
40+
},
41+
[`${astUtilsPath}.js`]: {
42+
[ReferenceTracker.ESM]: true,
43+
findVariable: {
44+
[ReferenceTracker.CALL]: true
45+
}
46+
},
47+
[`${astUtilsPath}.ts`]: {
48+
[ReferenceTracker.ESM]: true,
49+
findVariable: {
50+
[ReferenceTracker.CALL]: true
51+
}
52+
}
53+
})
54+
];
55+
type FunctionContext = {
56+
node:
57+
| TSESTree.FunctionDeclaration
58+
| TSESTree.FunctionExpression
59+
| TSESTree.ArrowFunctionExpression;
60+
identifier: TSESTree.Identifier | null;
61+
findVariableCall?: TSESTree.CallExpression;
62+
calls: Set<TSESTree.Identifier>;
63+
upper: FunctionContext | null;
64+
};
65+
let functionStack: FunctionContext | null = null;
66+
const functionContexts: FunctionContext[] = [];
67+
68+
function getFunctionVariableName(
69+
node:
70+
| TSESTree.FunctionDeclaration
71+
| TSESTree.FunctionExpression
72+
| TSESTree.ArrowFunctionExpression
73+
) {
74+
if (node.type === 'FunctionDeclaration') {
75+
return node.id;
76+
}
77+
if (node.parent?.type === 'VariableDeclarator' && node.parent.id.type === 'Identifier') {
78+
return node.parent.id;
79+
}
80+
return null;
81+
}
82+
83+
function* iterateVariables(node: TSESTree.Identifier) {
84+
const visitedNodes = new Set<TSESTree.Identifier>();
85+
let currentNode: TSESTree.Identifier | null = node;
86+
while (currentNode) {
87+
if (visitedNodes.has(currentNode)) break;
88+
const variable = findVariable(
89+
context.sourceCode.getScope(currentNode),
90+
currentNode
91+
) as Variable | null;
92+
if (!variable) break;
93+
yield variable;
94+
const def = variable.defs[0];
95+
if (!def) break;
96+
if (def.type !== 'Variable' || !def.node.init) break;
97+
if (def.node.init.type !== 'Identifier') break;
98+
currentNode = def.node.init;
99+
visitedNodes.add(currentNode);
100+
}
101+
}
102+
103+
/**
104+
* Verify a function context to report if necessary.
105+
* Reports when a function contains a call to findVariable and the function is recursive.
106+
*/
107+
function verifyFunctionContext(functionContext: FunctionContext) {
108+
if (!functionContext.findVariableCall) return;
109+
if (!hasRecursive(functionContext)) return;
110+
context.report({
111+
node: functionContext.findVariableCall,
112+
messageId: 'preferFindVariableSafe'
113+
});
114+
}
115+
116+
function hasRecursive(functionContext: FunctionContext) {
117+
const buffer = [functionContext];
118+
const visitedContext = new Set<FunctionContext>();
119+
let current;
120+
while ((current = buffer.shift())) {
121+
if (visitedContext.has(current)) continue;
122+
visitedContext.add(current);
123+
if (!current.identifier) continue;
124+
for (const variable of iterateVariables(current.identifier)) {
125+
for (const { identifier } of variable.references) {
126+
if (identifier.type !== 'Identifier') continue;
127+
if (functionContext.calls.has(identifier)) {
128+
return true;
129+
}
130+
buffer.push(...functionContexts.filter((ctx) => ctx.calls.has(identifier)));
131+
}
132+
}
133+
}
134+
return false;
135+
}
136+
137+
return {
138+
':function'(
139+
node:
140+
| TSESTree.FunctionDeclaration
141+
| TSESTree.FunctionExpression
142+
| TSESTree.ArrowFunctionExpression
143+
) {
144+
functionStack = {
145+
node,
146+
identifier: getFunctionVariableName(node),
147+
calls: new Set(),
148+
upper: functionStack
149+
};
150+
functionContexts.push(functionStack);
151+
},
152+
':function:exit'() {
153+
functionStack = functionStack?.upper || null;
154+
},
155+
CallExpression(node) {
156+
if (!functionStack) return;
157+
if (findVariableCalls.some((call) => call.node === node)) {
158+
functionStack.findVariableCall = node;
159+
}
160+
if (node.callee.type === 'Identifier') {
161+
functionStack.calls.add(node.callee);
162+
}
163+
},
164+
'Program:exit'() {
165+
for (const functionContext of functionContexts) {
166+
verifyFunctionContext(functionContext);
167+
}
168+
}
169+
};
170+
}
171+
};

‎packages/eslint-plugin-svelte/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@
9595
"eslint-typegen": "^2.3.0",
9696
"eslint-visitor-keys": "^4.2.1",
9797
"espree": "^10.4.0",
98+
"jiti": "^2.5.1",
9899
"less": "^4.4.1",
99100
"mocha": "~11.7.2",
100101
"postcss-nested": "^7.0.2",

‎packages/eslint-plugin-svelte/src/rules/no-dynamic-slot-name.ts‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { AST } from 'svelte-eslint-parser';
22
import type { TSESTree } from '@typescript-eslint/types';
33
import { createRule } from '../utils/index.js';
44
import {
5-
findVariable,
5+
FindVariableContext,
66
getAttributeValueQuoteAndRange,
77
getStringIfConstant
88
} from '../utils/ast-utils.js';
@@ -67,28 +67,27 @@ export default createRule('no-dynamic-slot-name', {
6767
* Get static text from given expression
6868
*/
6969
function getStaticText(node: TSESTree.Expression) {
70-
const expr = findRootExpression(node);
70+
const expr = findRootExpression(new FindVariableContext(context), node);
7171
return getStringIfConstant(expr);
7272
}
7373

7474
/** Find data expression */
7575
function findRootExpression(
76-
node: TSESTree.Expression,
77-
already = new Set<TSESTree.Identifier>()
76+
ctx: FindVariableContext,
77+
node: TSESTree.Expression
7878
): TSESTree.Expression {
79-
if (node.type !== 'Identifier' || already.has(node)) {
79+
if (node.type !== 'Identifier') {
8080
return node;
8181
}
82-
already.add(node);
83-
const variable = findVariable(context, node);
82+
const variable = ctx.findVariable(node);
8483
if (!variable || variable.defs.length !== 1) {
8584
return node;
8685
}
8786
const def = variable.defs[0];
8887
if (def.type === 'Variable') {
8988
if (def.parent.kind === 'const' && def.node.init) {
9089
const init = def.node.init;
91-
return findRootExpression(init, already);
90+
return findRootExpression(ctx, init);
9291
}
9392
}
9493
return node;

‎packages/eslint-plugin-svelte/src/rules/no-immutable-reactive-statements.ts‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { AST } from 'svelte-eslint-parser';
22
import { createRule } from '../utils/index.js';
33
import type { Scope, Variable, Reference, Definition } from '@typescript-eslint/scope-manager';
44
import type { TSESTree } from '@typescript-eslint/types';
5-
import { findVariable, iterateIdentifiers } from '../utils/ast-utils.js';
5+
import { FindVariableContext, iterateIdentifiers } from '../utils/ast-utils.js';
66

77
export default createRule('no-immutable-reactive-statements', {
88
meta: {
@@ -91,7 +91,7 @@ export default createRule('no-immutable-reactive-statements', {
9191
return true;
9292
}
9393
}
94-
return hasWrite(variable);
94+
return hasWrite(new FindVariableContext(context), variable);
9595
}
9696
return false;
9797
});
@@ -100,7 +100,7 @@ export default createRule('no-immutable-reactive-statements', {
100100
}
101101

102102
/** Checks whether the given variable has a write or reactive store reference or not. */
103-
function hasWrite(variable: Variable) {
103+
function hasWrite(ctx: FindVariableContext, variable: Variable) {
104104
const defIds = variable.defs.map((def: Definition) => def.name);
105105
for (const reference of variable.references) {
106106
if (
@@ -113,7 +113,7 @@ export default createRule('no-immutable-reactive-statements', {
113113
) {
114114
return true;
115115
}
116-
if (hasWriteMember(reference.identifier)) {
116+
if (hasWriteMember(ctx, reference.identifier)) {
117117
return true;
118118
}
119119
}
@@ -122,6 +122,7 @@ export default createRule('no-immutable-reactive-statements', {
122122

123123
/** Checks whether the given expression has writing to a member or not. */
124124
function hasWriteMember(
125+
ctx: FindVariableContext,
125126
expr: TSESTree.Identifier | TSESTree.JSXIdentifier | TSESTree.MemberExpression
126127
): boolean {
127128
if (expr.type === 'JSXIdentifier') return false;
@@ -136,25 +137,30 @@ export default createRule('no-immutable-reactive-statements', {
136137
return parent.operator === 'delete' && parent.argument === expr;
137138
}
138139
if (parent.type === 'MemberExpression') {
139-
return parent.object === expr && hasWriteMember(parent);
140+
return parent.object === expr && hasWriteMember(ctx, parent);
140141
}
141142
if (parent.type === 'SvelteDirective') {
142143
return parent.kind === 'Binding' && parent.expression === expr;
143144
}
144145
if (parent.type === 'SvelteEachBlock') {
145146
return (
146-
parent.context !== null && parent.expression === expr && hasWriteReference(parent.context)
147+
parent.context !== null &&
148+
parent.expression === expr &&
149+
hasWriteReference(ctx, parent.context)
147150
);
148151
}
149152

150153
return false;
151154
}
152155

153156
/** Checks whether the given pattern has writing or not. */
154-
function hasWriteReference(pattern: TSESTree.DestructuringPattern): boolean {
157+
function hasWriteReference(
158+
ctx: FindVariableContext,
159+
pattern: TSESTree.DestructuringPattern
160+
): boolean {
155161
for (const id of iterateIdentifiers(pattern)) {
156-
const variable = findVariable(context, id);
157-
if (variable && hasWrite(variable)) return true;
162+
const variable = ctx.findVariable(id);
163+
if (variable && hasWrite(ctx, variable)) return true;
158164
}
159165

160166
return false;

0 commit comments

Comments
 (0)