Skip to content

Commit 70bbe35

Browse files
ota-meshiota-meshi
committed
Replace with FindVariableContext
1 parent 1ea5b5e commit 70bbe35

File tree

10 files changed

+144
-88
lines changed

10 files changed

+144
-88
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import type { Variable } from '@typescript-eslint/scope-manager';
77
export default {
88
meta: {
99
docs: {
10-
description: 'enforce to use findVariableSafe() to avoid infinite recursion',
10+
description: 'enforce to use FindVariableContext to avoid infinite recursion',
1111
category: 'Best Practices',
1212
recommended: false,
1313
conflictWithPrettier: false,
1414
url: 'https://github.com/sveltejs/eslint-plugin-svelte/blob/v3.12.3/docs/rules/prefer-find-variable-safe.md'
1515
},
1616
messages: {
17-
preferFindVariableSafe: 'Prefer to use findVariableSafe() to avoid infinite recursion.'
17+
preferFindVariableSafe: 'Prefer to use FindVariableContext to avoid infinite recursion.'
1818
},
1919
schema: [],
2020
type: 'suggestion'

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

Lines changed: 8 additions & 5 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-
findVariableSafe,
5+
FindVariableContext,
66
getAttributeValueQuoteAndRange,
77
getStringIfConstant
88
} from '../utils/ast-utils.js';
@@ -67,24 +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 */
75-
function findRootExpression(node: TSESTree.Expression): TSESTree.Expression {
75+
function findRootExpression(
76+
ctx: FindVariableContext,
77+
node: TSESTree.Expression
78+
): TSESTree.Expression {
7679
if (node.type !== 'Identifier') {
7780
return node;
7881
}
79-
const variable = findVariableSafe(findRootExpression, context, node);
82+
const variable = ctx.findVariable(node);
8083
if (!variable || variable.defs.length !== 1) {
8184
return node;
8285
}
8386
const def = variable.defs[0];
8487
if (def.type === 'Variable') {
8588
if (def.parent.kind === 'const' && def.node.init) {
8689
const init = def.node.init;
87-
return findRootExpression(init);
90+
return findRootExpression(ctx, init);
8891
}
8992
}
9093
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 { findVariableSafe, 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 = findVariableSafe(hasWriteReference, 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;

‎packages/eslint-plugin-svelte/src/rules/no-navigation-without-resolve.ts‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import type { TSESTree } from '@typescript-eslint/types';
22
import { createRule } from '../utils/index.js';
33
import { ReferenceTracker } from '@eslint-community/eslint-utils';
4-
import { findVariableSafe } from '../utils/ast-utils.js';
4+
import { FindVariableContext } from '../utils/ast-utils.js';
5+
import { findVariable } from '../utils/ast-utils.js';
56
import type { RuleContext } from '../types.js';
67
import type { AST } from 'svelte-eslint-parser';
78

@@ -101,7 +102,11 @@ export default createRule('no-navigation-without-resolve', {
101102
(node.value[0].type === 'SvelteMustacheTag' &&
102103
!expressionIsAbsolute(node.value[0].expression) &&
103104
!expressionIsFragment(node.value[0].expression) &&
104-
!isResolveCall(context, node.value[0].expression, resolveReferences))
105+
!isResolveCall(
106+
new FindVariableContext(context),
107+
node.value[0].expression,
108+
resolveReferences
109+
))
105110
) {
106111
context.report({ loc: node.value[0].loc, messageId: 'linkWithoutResolve' });
107112
}
@@ -126,7 +131,7 @@ function extractResolveReferences(
126131
}
127132
})) {
128133
if (node.type === 'ImportSpecifier') {
129-
const variable = findVariableSafe(extractResolveReferences, context, node.local);
134+
const variable = findVariable(context, node.local);
130135
if (variable === null) {
131136
continue;
132137
}
@@ -191,7 +196,7 @@ function checkGotoCall(
191196
return;
192197
}
193198
const url = call.arguments[0];
194-
if (!isResolveCall(context, url, resolveReferences)) {
199+
if (!isResolveCall(new FindVariableContext(context), url, resolveReferences)) {
195200
context.report({ loc: url.loc, messageId: 'gotoWithoutResolve' });
196201
}
197202
}
@@ -206,15 +211,18 @@ function checkShallowNavigationCall(
206211
return;
207212
}
208213
const url = call.arguments[0];
209-
if (!expressionIsEmpty(url) && !isResolveCall(context, url, resolveReferences)) {
214+
if (
215+
!expressionIsEmpty(url) &&
216+
!isResolveCall(new FindVariableContext(context), url, resolveReferences)
217+
) {
210218
context.report({ loc: url.loc, messageId });
211219
}
212220
}
213221

214222
// Helper functions
215223

216224
function isResolveCall(
217-
context: RuleContext,
225+
ctx: FindVariableContext,
218226
node: TSESTree.CallExpressionArgument,
219227
resolveReferences: Set<TSESTree.Identifier>
220228
): boolean {
@@ -228,13 +236,13 @@ function isResolveCall(
228236
return true;
229237
}
230238
if (node.type === 'Identifier') {
231-
const variable = findVariableSafe(isResolveCall, context, node);
239+
const variable = ctx.findVariable(node);
232240
if (
233241
variable !== null &&
234242
variable.identifiers.length > 0 &&
235243
variable.identifiers[0].parent.type === 'VariableDeclarator' &&
236244
variable.identifiers[0].parent.init !== null &&
237-
isResolveCall(context, variable.identifiers[0].parent.init, resolveReferences)
245+
isResolveCall(ctx, variable.identifiers[0].parent.init, resolveReferences)
238246
) {
239247
return true;
240248
}

‎packages/eslint-plugin-svelte/src/rules/no-not-function-handler.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import type { AST } from 'svelte-eslint-parser';
22
import type { TSESTree } from '@typescript-eslint/types';
33
import { createRule } from '../utils/index.js';
4-
import { findVariableSafe } from '../utils/ast-utils.js';
54
import { EVENT_NAMES } from '../utils/events.js';
5+
import { FindVariableContext } from '../utils/ast-utils.js';
66

77
const PHRASES = {
88
ObjectExpression: 'object',
@@ -37,19 +37,22 @@ export default createRule('no-not-function-handler', {
3737
},
3838
create(context) {
3939
/** Find data expression */
40-
function findRootExpression(node: TSESTree.Expression): TSESTree.Expression {
40+
function findRootExpression(
41+
ctx: FindVariableContext,
42+
node: TSESTree.Expression
43+
): TSESTree.Expression {
4144
if (node.type !== 'Identifier') {
4245
return node;
4346
}
44-
const variable = findVariableSafe(findRootExpression, context, node);
47+
const variable = ctx.findVariable(node);
4548
if (!variable || variable.defs.length !== 1) {
4649
return node;
4750
}
4851
const def = variable.defs[0];
4952
if (def.type === 'Variable') {
5053
if (def.parent.kind === 'const' && def.node.init) {
5154
const init = def.node.init;
52-
return findRootExpression(init);
55+
return findRootExpression(ctx, init);
5356
}
5457
}
5558
return node;
@@ -60,7 +63,7 @@ export default createRule('no-not-function-handler', {
6063
if (!node) {
6164
return;
6265
}
63-
const expression = findRootExpression(node);
66+
const expression = findRootExpression(new FindVariableContext(context), node);
6467

6568
if (
6669
expression.type !== 'ObjectExpression' &&

‎packages/eslint-plugin-svelte/src/utils/ast-utils.ts‎

Lines changed: 17 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -231,40 +231,26 @@ export function findVariable(context: RuleContext, node: TSESTree.Identifier): V
231231
return eslintUtils.findVariable(initialScope, node.name.slice(1));
232232
}
233233

234-
const findVariableSafeVisited = new WeakMap<
235-
RuleContext,
236-
// eslint-disable-next-line @typescript-eslint/no-unsafe-function-type -- ignore
237-
WeakMap<Function, Set<TSESTree.Identifier>>
238-
>();
239-
240234
/**
241-
* Find the variable of a given name safely, avoiding infinite recursion.
242-
* This should be used when the caller function may be called recursively.
243-
* @param caller The caller function. This is used to track recursion.
244-
* @param context The rule context.
245-
* @param node The identifier node to find.
235+
* Context for safely finding variables, avoiding infinite recursion.
236+
* This should be used when the caller function may be called recursively, instead of `findVariable()`.
237+
*
238+
* Create an instance of this class at the top of the call stack where you want to start searching for identifiers,
239+
* and then use it within the recursion.
246240
*/
247-
export function findVariableSafe(
248-
// eslint-disable-next-line @typescript-eslint/no-unsafe-function-type -- ignore
249-
caller: Function,
250-
context: RuleContext,
251-
node: TSESTree.Identifier
252-
): Variable | null {
253-
let visited = findVariableSafeVisited.get(context);
254-
if (!visited) {
255-
visited = new WeakMap();
256-
findVariableSafeVisited.set(context, visited);
257-
}
258-
let visitedNodes = visited.get(caller);
259-
if (!visitedNodes) {
260-
visitedNodes = new Set();
261-
visited.set(caller, visitedNodes);
262-
}
263-
if (visitedNodes.has(node)) {
264-
return null;
241+
export class FindVariableContext {
242+
public readonly findVariable: (node: TSESTree.Identifier) => Variable | null;
243+
244+
public constructor(context: RuleContext) {
245+
const visited = new Set<TSESTree.Identifier>();
246+
this.findVariable = (node: TSESTree.Identifier) => {
247+
if (visited.has(node)) {
248+
return null;
249+
}
250+
visited.add(node);
251+
return findVariable(context, node);
252+
};
265253
}
266-
visitedNodes.add(node);
267-
return findVariable(context, node);
268254
}
269255

270256
/**

0 commit comments

Comments
 (0)