Skip to content

Commit a63e055

Browse files
committed
refactor(prefer-const): remove class in favour of function
1 parent 10e7f7e commit a63e055

File tree

2 files changed

+103
-66
lines changed

2 files changed

+103
-66
lines changed

packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts

+96-63
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,24 @@ import type { RuleContext } from '../../types';
66

77
type ASTNode = TSESTree.Node;
88
type CheckedNode = NonNullable<ASTNode & { parent: NonNullable<ASTNode> }>;
9+
type VariableDeclaration =
10+
| TSESTree.LetOrConstOrVarDeclaration
11+
| TSESTree.UsingInForOfDeclaration
12+
| TSESTree.UsingInNormalContextDeclaration;
13+
type VariableDeclarator =
14+
| TSESTree.LetOrConstOrVarDeclarator
15+
| TSESTree.UsingInForOfDeclarator
16+
| TSESTree.UsingInNomalConextDeclarator;
917

1018
const PATTERN_TYPE =
1119
/^(?:.+?Pattern|RestElement|SpreadProperty|ExperimentalRestProperty|Property)$/u;
1220
const DECLARATION_HOST_TYPE = /^(?:Program|BlockStatement|StaticBlock|SwitchCase)$/u;
1321
const DESTRUCTURING_HOST_TYPE = /^(?:VariableDeclarator|AssignmentExpression)$/u;
1422

23+
/**
24+
* Finds the callee of a `CallExpression` if the given node is part of a
25+
* `VariableDeclarator` or an ObjectPattern within a `VariableDeclarator`.
26+
*/
1527
function findIdentifierCallee(node: CheckedNode) {
1628
const { parent } = node;
1729
if (parent.type === 'VariableDeclarator' && parent.init?.type === 'CallExpression') {
@@ -70,7 +82,8 @@ function skipReactiveValues(identifier: ASTNode | null) {
7082
}
7183

7284
/**
73-
* Checks whether a given Identifier node becomes a VariableDeclaration or not.
85+
* Checks whether a given `Identifier` node becomes a `VariableDeclaration` or
86+
* not.
7487
*/
7588
function canBecomeVariableDeclaration(identifier: ASTNode) {
7689
let node = identifier.parent;
@@ -91,8 +104,8 @@ function canBecomeVariableDeclaration(identifier: ASTNode) {
91104
}
92105

93106
/**
94-
* Checks if an property or element is from outer scope or export function parameters
95-
* in destructing pattern.
107+
* Checks if an property or element is from outer scope or export function
108+
* parameters in destructing pattern.
96109
*/
97110
function isOuterVariableInDestructing(name: string, initScope: Scope) {
98111
if (initScope.through.some((ref) => ref.resolved && ref.resolved.name === name)) {
@@ -104,10 +117,9 @@ function isOuterVariableInDestructing(name: string, initScope: Scope) {
104117
}
105118

106119
/**
107-
* Gets the VariableDeclarator/AssignmentExpression node that a given reference
108-
* belongs to.
109-
* This is used to detect a mix of reassigned and never reassigned in a
110-
* destructuring.
120+
* Gets the `VariableDeclarator/AssignmentExpression` node that a given
121+
* reference belongs to. It is used to detect a mix of reassigned and never
122+
* reassigned in a destructuring.
111123
*/
112124
function getDestructuringHost(reference: Reference) {
113125
if (!reference.isWrite()) {
@@ -127,10 +139,10 @@ function getDestructuringHost(reference: Reference) {
127139
}
128140

129141
/**
130-
* Determines if a destructuring assignment node contains
131-
* any MemberExpression nodes. This is used to determine if a
132-
* variable that is only written once using destructuring can be
133-
* safely converted into a const declaration.
142+
* Determines if a destructuring assignment node contains any
143+
* `MemberExpression` nodes. is used to determine if a variable that is only
144+
* written once using destructuring can be safely converted into a const
145+
* declaration.
134146
*/
135147
function hasMemberExpressionAssignment(node: ASTNode): boolean {
136148
if (node.type === 'MemberExpression') {
@@ -153,6 +165,10 @@ function hasMemberExpressionAssignment(node: ASTNode): boolean {
153165
return false;
154166
}
155167

168+
/**
169+
* Validates an object pattern to determine if it contains outer variables or
170+
* non-identifier assignments.
171+
*/
156172
function validateObjectPattern(node: TSESTree.ObjectPattern, variable: Variable) {
157173
const properties = node.properties;
158174
const hasOuterVariables = properties
@@ -170,6 +186,10 @@ function validateObjectPattern(node: TSESTree.ObjectPattern, variable: Variable)
170186
return hasOuterVariables || hasNonIdentifiers;
171187
}
172188

189+
/**
190+
* Validates an array pattern to determine if it contains outer variables or
191+
* non-identifier elements.
192+
*/
173193
function validateArrayPattern(node: TSESTree.ArrayPattern, variable: Variable): boolean {
174194
const elements = node.elements;
175195
const hasOuterVariables = elements
@@ -187,15 +207,15 @@ function validateArrayPattern(node: TSESTree.ArrayPattern, variable: Variable):
187207
* the first assignment, the identifier node is the node of the declaration.
188208
* Otherwise, the identifier node is the node of the first assignment.
189209
*
190-
* If the variable should not change to const, this export function returns null.
210+
* If the variable should not change to const, export function returns null.
191211
* - If the variable is reassigned.
192212
* - If the variable is never initialized nor assigned.
193213
* - If the variable is initialized in a different scope from the declaration.
194214
* - If the unique assignment of the variable cannot change to a declaration.
195215
* e.g. `if (a) b = 1` / `return (b = 1)`
196216
* - If the variable is declared in the global scope and `eslintUsed` is `true`.
197-
* `/*exported foo` directive comment makes such variables. This rule does not
198-
* warn such variables because this rule cannot distinguish whether the
217+
* `/*exported foo` directive comment makes such variables. rule does not
218+
* warn such variables because rule cannot distinguish whether the
199219
* exported variables are reassigned or not.
200220
*/
201221
function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign: boolean) {
@@ -247,6 +267,7 @@ function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign
247267
if (!shouldBeConst) {
248268
return null;
249269
}
270+
250271
if (isReadBeforeInit) {
251272
return variable.defs[0].name;
252273
}
@@ -295,7 +316,7 @@ export function isInitOfForStatement(node: ASTNode): boolean {
295316
/**
296317
* Groups by the VariableDeclarator/AssignmentExpression node that each
297318
* reference of given variables belongs to.
298-
* This is used to detect a mix of reassigned and never reassigned in a
319+
* is used to detect a mix of reassigned and never reassigned in a
299320
* destructuring.
300321
*/
301322
export function groupByDestructuring(
@@ -336,6 +357,13 @@ export function groupByDestructuring(
336357
return identifierMap;
337358
}
338359

360+
/**
361+
* Calculates the retained text range by comparing the given range with an
362+
* optional retained range. If a retained range is provided, it merges the two
363+
* ranges to form an actual range. It then returns two ranges: one from the
364+
* start of the actual range to the start of the given range, and another from
365+
* the end of the given range to the end of the actual range.
366+
*/
339367
function calculateRetainedTextRange(
340368
range: TSESTree.Range,
341369
retainedRange: TSESTree.Range | null
@@ -350,33 +378,32 @@ function calculateRetainedTextRange(
350378
];
351379
}
352380

353-
type CheckOptions = { destructuring: 'any' | 'all' };
354-
type VariableDeclaration =
355-
| TSESTree.LetOrConstOrVarDeclaration
356-
| TSESTree.UsingInForOfDeclaration
357-
| TSESTree.UsingInNormalContextDeclaration;
358-
type VariableDeclarator =
359-
| TSESTree.LetOrConstOrVarDeclarator
360-
| TSESTree.UsingInForOfDeclarator
361-
| TSESTree.UsingInNomalConextDeclarator;
362-
export class GroupChecker {
363-
private reportCount = 0;
364-
365-
private checkedId: TSESTree.BindingName | null = null;
366-
367-
private checkedName = '';
368-
369-
private readonly shouldMatchAnyDestructuredVariable: boolean;
381+
type NodeReporterOptions = { destructuring: 'any' | 'all' };
382+
type NodeReporter = { report: (nodes: ASTNode[]) => void };
370383

371-
private readonly context: RuleContext;
372-
373-
public constructor(context: RuleContext, { destructuring }: CheckOptions) {
374-
this.context = context;
375-
this.shouldMatchAnyDestructuredVariable = destructuring !== 'all';
376-
}
377-
378-
public checkAndReportNodes(nodes: ASTNode[]): void {
379-
const shouldCheckGroup = nodes.length && this.shouldMatchAnyDestructuredVariable;
384+
/**
385+
* Creates a node reporter function that checks and reports nodes based on the
386+
* provided context and options.
387+
*
388+
* @remarks The `report` function checks and reports nodes that should be
389+
* converted to `const` declarations. It performs various checks to ensure that
390+
* the nodes meet the criteria for reporting and fixing.
391+
*
392+
* The function also handles cases where variables are declared using
393+
* destructuring patterns and ensures that the correct number of nodes are
394+
* reported and fixed.
395+
*/
396+
export function createNodeReporter(
397+
context: RuleContext,
398+
{ destructuring }: NodeReporterOptions
399+
): NodeReporter {
400+
let reportCount = 0;
401+
let checkedId: TSESTree.BindingName | null = null;
402+
let checkedName = '';
403+
const shouldMatchAnyDestructuredVariable = destructuring !== 'all';
404+
405+
function checkAndReportNodes(nodes: ASTNode[]): void {
406+
const shouldCheckGroup = nodes.length && shouldMatchAnyDestructuredVariable;
380407
if (!shouldCheckGroup) {
381408
return;
382409
}
@@ -396,28 +423,28 @@ export class GroupChecker {
396423
}
397424

398425
const dec = variableDeclarationParent.declarations[0];
399-
this.checkDeclarator(dec);
426+
checkDeclarator(dec);
400427

401-
const shouldFix = this.checkShouldFix(variableDeclarationParent, nodes.length);
428+
const shouldFix = checkShouldFix(variableDeclarationParent, nodes.length);
402429
if (!shouldFix) {
403430
return;
404431
}
405432

406-
const sourceCode = getSourceCode(this.context);
433+
const sourceCode = getSourceCode(context);
407434
nodes.filter(skipReactiveValues).forEach((node) => {
408-
this.report(sourceCode, node, variableDeclarationParent);
435+
report(sourceCode, node, variableDeclarationParent);
409436
});
410437
}
411438

412-
private report(
439+
function report(
413440
sourceCode: ReturnType<typeof getSourceCode>,
414441
node: ASTNode,
415442
nodeParent: VariableDeclaration
416443
) {
417-
this.context.report({
444+
context.report({
418445
node,
419446
messageId: 'useConst',
420-
// @ts-expect-error Name will exist at this point
447+
// @ts-expect-error Name will exist at point
421448
data: { name: node.name },
422449
fix: (fixer) => {
423450
const letKeywordToken = sourceCode.getFirstToken(nodeParent, {
@@ -440,23 +467,23 @@ export class GroupChecker {
440467
});
441468
}
442469

443-
private checkShouldFix(declaration: VariableDeclaration, totalNodes: number) {
470+
function checkShouldFix(declaration: VariableDeclaration, totalNodes: number) {
444471
const shouldFix =
445472
declaration &&
446473
(declaration.parent.type === 'ForInStatement' ||
447474
declaration.parent.type === 'ForOfStatement' ||
448475
('declarations' in declaration &&
449476
declaration.declarations.every((declaration) => declaration.init)));
450477

451-
const totalDeclarationCount = this.checkDestructuredDeclaration(declaration, totalNodes);
478+
const totalDeclarationCount = checkDestructuredDeclaration(declaration, totalNodes);
452479
if (totalDeclarationCount === -1) {
453480
return shouldFix;
454481
}
455482

456-
return shouldFix && this.reportCount === totalDeclarationCount;
483+
return shouldFix && reportCount === totalDeclarationCount;
457484
}
458485

459-
private checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) {
486+
function checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) {
460487
const hasMultipleDeclarations =
461488
declaration !== null &&
462489
'declarations' in declaration &&
@@ -471,7 +498,7 @@ export class GroupChecker {
471498
return -1;
472499
}
473500

474-
this.reportCount += totalNodes;
501+
reportCount += totalNodes;
475502

476503
return declaration.declarations.reduce((total, declaration) => {
477504
if (declaration.id.type === 'ObjectPattern') {
@@ -486,7 +513,7 @@ export class GroupChecker {
486513
}, 0);
487514
}
488515

489-
private checkDeclarator(declarator: VariableDeclarator) {
516+
function checkDeclarator(declarator: VariableDeclarator) {
490517
if (!declarator.init) {
491518
return;
492519
}
@@ -497,22 +524,28 @@ export class GroupChecker {
497524
}
498525

499526
const { id } = firstDecParent;
500-
if ('name' in id && id.name !== this.checkedName) {
501-
this.checkedName = id.name;
502-
this.reportCount = 0;
527+
if ('name' in id && id.name !== checkedName) {
528+
checkedName = id.name;
529+
reportCount = 0;
503530
}
504531

505532
if (firstDecParent.id.type === 'ObjectPattern') {
506533
const { init } = firstDecParent;
507-
if (init && 'name' in init && init.name !== this.checkedName) {
508-
this.checkedName = init.name;
509-
this.reportCount = 0;
534+
if (init && 'name' in init && init.name !== checkedName) {
535+
checkedName = init.name;
536+
reportCount = 0;
510537
}
511538
}
512539

513-
if (firstDecParent.id !== this.checkedId) {
514-
this.checkedId = firstDecParent.id;
515-
this.reportCount = 0;
540+
if (firstDecParent.id !== checkedId) {
541+
checkedId = firstDecParent.id;
542+
reportCount = 0;
516543
}
517544
}
545+
546+
return {
547+
report(group) {
548+
checkAndReportNodes(group);
549+
}
550+
};
518551
}

packages/eslint-plugin-svelte/src/rules/prefer-const.ts

+7-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import type { Variable } from '@typescript-eslint/scope-manager';
22
import { createRule } from '../utils';
33
import { getSourceCode } from '../utils/compat';
44

5-
import { GroupChecker, groupByDestructuring, isInitOfForStatement } from './prefer-const-helpers';
5+
import {
6+
createNodeReporter,
7+
groupByDestructuring,
8+
isInitOfForStatement
9+
} from './prefer-const-helpers';
610

711
/**
812
* Rule and helpers are copied from ESLint
@@ -43,11 +47,11 @@ export default createRule('prefer-const', {
4347

4448
return {
4549
'Program:exit'() {
46-
const checker = new GroupChecker(context, {
50+
const nodeReporter = createNodeReporter(context, {
4751
destructuring: options.destructuring
4852
});
4953
groupByDestructuring(variables, ignoreReadBeforeAssign).forEach((group) => {
50-
checker.checkAndReportNodes(group);
54+
nodeReporter.report(group);
5155
});
5256
},
5357
VariableDeclaration(node) {

0 commit comments

Comments
 (0)