Skip to content

Commit 87a6ef9

Browse files
mikededomikededo
committed
refactor(prefer-const): extract logic into class
After having finished the implementation, I've decided to extract the logic into a function, since I strongly believe it'll be easier to understand, as functions are equivalent to classes, if done right. The reason of this commit is to save the current implementation in case I want to revert changes.
1 parent 31ade4d commit 87a6ef9

File tree

4 files changed

+189
-125
lines changed

4 files changed

+189
-125
lines changed

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

Lines changed: 171 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { Reference, Variable, Scope } from '@typescript-eslint/scope-manager';
22
import type { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/types';
33
import { createRule } from '../utils';
4-
import type { RuleFixer, SourceCode } from '../types';
4+
import type { RuleContext, RuleFixer, SourceCode } from '../types';
55
import { getSourceCode } from '../utils/compat';
66

77
type ASTNode = TSESTree.Node;
@@ -112,6 +112,168 @@ class FixTracker {
112112
}
113113
}
114114

115+
type CheckOptions = { destructuring: 'any' | 'all' };
116+
type VariableDeclaration =
117+
| TSESTree.LetOrConstOrVarDeclaration
118+
| TSESTree.UsingInForOfDeclaration
119+
| TSESTree.UsingInNormalContextDeclaration;
120+
type VariableDeclarator =
121+
| TSESTree.LetOrConstOrVarDeclarator
122+
| TSESTree.UsingInForOfDeclarator
123+
| TSESTree.UsingInNomalConextDeclarator;
124+
class GroupChecker {
125+
private reportCount = 0;
126+
127+
private checkedId: TSESTree.BindingName | null = null;
128+
129+
private checkedName = '';
130+
131+
private readonly shouldMatchAnyDestructuredVariable: boolean;
132+
133+
private readonly context: RuleContext;
134+
135+
public constructor(context: RuleContext, { destructuring }: CheckOptions) {
136+
this.context = context;
137+
this.shouldMatchAnyDestructuredVariable = destructuring !== 'all';
138+
}
139+
140+
public checkAndReportNodes(nodes: ASTNode[]) {
141+
const shouldCheckGroup = nodes.length && this.shouldMatchAnyDestructuredVariable;
142+
if (!shouldCheckGroup) {
143+
return;
144+
}
145+
146+
const variableDeclarationParent = findUp(
147+
nodes[0],
148+
'VariableDeclaration',
149+
(parentNode: ASTNode) => parentNode.type.endsWith('Statement')
150+
);
151+
const isVarDecParentNull = variableDeclarationParent === null;
152+
const isValidDecParent =
153+
!isVarDecParentNull &&
154+
'declarations' in variableDeclarationParent &&
155+
variableDeclarationParent.declarations.length > 0;
156+
if (!isValidDecParent) {
157+
return;
158+
}
159+
160+
const dec = variableDeclarationParent.declarations[0];
161+
this.checkDeclarator(dec);
162+
163+
const shouldFix = this.checkShouldFix(variableDeclarationParent, nodes.length);
164+
if (!shouldFix) {
165+
return;
166+
}
167+
168+
const sourceCode = getSourceCode(this.context);
169+
nodes.filter(skipReactiveValues).forEach((node) => {
170+
this.report(sourceCode, node, variableDeclarationParent);
171+
});
172+
}
173+
174+
private report(
175+
sourceCode: ReturnType<typeof getSourceCode>,
176+
node: ASTNode,
177+
nodeParent: VariableDeclaration
178+
) {
179+
this.context.report({
180+
node,
181+
messageId: 'useConst',
182+
// @ts-expect-error Name will exist at this point
183+
data: { name: node.name },
184+
fix: (fixer) => {
185+
const letKeywordToken = sourceCode.getFirstToken(nodeParent, {
186+
includeComments: false,
187+
filter: (t) => 'kind' in nodeParent && t.value === nodeParent.kind
188+
});
189+
if (!letKeywordToken) {
190+
return null;
191+
}
192+
193+
return new FixTracker(fixer, sourceCode)
194+
.retainRange(nodeParent.range)
195+
.replaceTextRange(letKeywordToken.range, 'const');
196+
}
197+
});
198+
}
199+
200+
private checkShouldFix(declaration: VariableDeclaration, totalNodes: number) {
201+
const shouldFix =
202+
declaration &&
203+
(declaration.parent.type === 'ForInStatement' ||
204+
declaration.parent.type === 'ForOfStatement' ||
205+
('declarations' in declaration &&
206+
declaration.declarations.every((declaration) => declaration.init)));
207+
208+
const totalDeclarationCount = this.checkDestructuredDeclaration(declaration, totalNodes);
209+
if (totalDeclarationCount === -1) {
210+
return shouldFix;
211+
}
212+
213+
return shouldFix && this.reportCount === totalDeclarationCount;
214+
}
215+
216+
private checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) {
217+
const hasMultipleDeclarations =
218+
declaration !== null &&
219+
'declarations' in declaration &&
220+
declaration.declarations.length !== 1;
221+
if (!hasMultipleDeclarations) {
222+
return -1;
223+
}
224+
225+
const hasMoreThanOneDeclaration =
226+
declaration && declaration.declarations && declaration.declarations.length >= 1;
227+
if (!hasMoreThanOneDeclaration) {
228+
return -1;
229+
}
230+
231+
this.reportCount += totalNodes;
232+
233+
return declaration.declarations.reduce((total, declaration) => {
234+
if (declaration.id.type === 'ObjectPattern') {
235+
return total + declaration.id.properties.length;
236+
}
237+
238+
if (declaration.id.type === 'ArrayPattern') {
239+
return total + declaration.id.elements.length;
240+
}
241+
242+
return total + 1;
243+
}, 0);
244+
}
245+
246+
private checkDeclarator(declarator: VariableDeclarator) {
247+
if (!declarator.init) {
248+
return;
249+
}
250+
251+
const firstDecParent = declarator.init.parent;
252+
if (firstDecParent.type !== 'VariableDeclarator') {
253+
return;
254+
}
255+
256+
const { id } = firstDecParent;
257+
if ('name' in id && id.name !== this.checkedName) {
258+
this.checkedName = id.name;
259+
this.reportCount = 0;
260+
}
261+
262+
if (firstDecParent.id.type === 'ObjectPattern') {
263+
const { init } = firstDecParent;
264+
if (init && 'name' in init && init.name !== this.checkedName) {
265+
this.checkedName = init.name;
266+
this.reportCount = 0;
267+
}
268+
}
269+
270+
if (firstDecParent.id !== this.checkedId) {
271+
this.checkedId = firstDecParent.id;
272+
this.reportCount = 0;
273+
}
274+
}
275+
}
276+
115277
export default createRule('prefer-const', {
116278
meta: {
117279
type: 'suggestion',
@@ -140,135 +302,18 @@ export default createRule('prefer-const', {
140302
const sourceCode = getSourceCode(context);
141303

142304
const options = context.options[0] || {};
143-
const shouldMatchAnyDestructuredVariable = options.destructuring !== 'all';
144305
const ignoreReadBeforeAssign = options.ignoreReadBeforeAssign === true;
145306

146307
const variables: Variable[] = [];
147-
let reportCount = 0;
148-
let checkedId: TSESTree.BindingName | null = null;
149-
let checkedName = '';
150-
151-
function checkGroup(nodes: (ASTNode | null)[]) {
152-
const nodesToReport = nodes.filter(Boolean);
153-
if (
154-
nodes.length &&
155-
(shouldMatchAnyDestructuredVariable || nodesToReport.length === nodes.length) &&
156-
nodes[0] !== null
157-
) {
158-
const varDeclParent = findUp(nodes[0], 'VariableDeclaration', (parentNode: ASTNode) =>
159-
parentNode.type.endsWith('Statement')
160-
);
161-
162-
const isVarDecParentNull = varDeclParent === null;
163-
const isValidDecParent =
164-
!isVarDecParentNull &&
165-
'declarations' in varDeclParent &&
166-
varDeclParent.declarations.length > 0;
167-
168-
if (isValidDecParent) {
169-
const firstDeclaration = varDeclParent.declarations[0];
170-
171-
if (firstDeclaration.init) {
172-
const firstDecParent = firstDeclaration.init.parent;
173-
174-
if (firstDecParent.type === 'VariableDeclarator') {
175-
const { id } = firstDecParent;
176-
if ('name' in id && id.name !== checkedName) {
177-
checkedName = id.name;
178-
reportCount = 0;
179-
}
180-
181-
if (firstDecParent.id.type === 'ObjectPattern') {
182-
const { init } = firstDecParent;
183-
if (init && 'name' in init && init.name !== checkedName) {
184-
checkedName = init.name;
185-
reportCount = 0;
186-
}
187-
}
188-
189-
if (firstDecParent.id !== checkedId) {
190-
checkedId = firstDecParent.id;
191-
reportCount = 0;
192-
}
193-
}
194-
}
195-
}
196-
197-
let shouldFix =
198-
varDeclParent &&
199-
(varDeclParent.parent.type === 'ForInStatement' ||
200-
varDeclParent.parent.type === 'ForOfStatement' ||
201-
('declarations' in varDeclParent &&
202-
varDeclParent.declarations.every((declaration) => declaration.init))) &&
203-
nodesToReport.length === nodes.length;
204-
205-
if (
206-
!isVarDecParentNull &&
207-
'declarations' in varDeclParent &&
208-
varDeclParent.declarations &&
209-
varDeclParent.declarations.length !== 1
210-
) {
211-
if (
212-
varDeclParent &&
213-
varDeclParent.declarations &&
214-
varDeclParent.declarations.length >= 1
215-
) {
216-
reportCount += nodesToReport.length;
217-
let totalDeclarationsCount = 0;
218-
219-
varDeclParent.declarations.forEach((declaration) => {
220-
if (declaration.id.type === 'ObjectPattern') {
221-
totalDeclarationsCount += declaration.id.properties.length;
222-
return;
223-
}
224-
225-
if (declaration.id.type === 'ArrayPattern') {
226-
totalDeclarationsCount += declaration.id.elements.length;
227-
return;
228-
}
229-
230-
totalDeclarationsCount += 1;
231-
});
232-
shouldFix = shouldFix && reportCount === totalDeclarationsCount;
233-
}
234-
}
235-
236-
if (!shouldFix) {
237-
return;
238-
}
239-
240-
nodesToReport.filter(skipReactiveValues).forEach((node) => {
241-
if (!node || !varDeclParent) {
242-
// TS check
243-
return;
244-
}
245-
246-
context.report({
247-
node,
248-
messageId: 'useConst',
249-
// @ts-expect-error Name will exist at this point
250-
data: { name: node.name },
251-
fix: (fixer) => {
252-
const letKeywordToken = sourceCode.getFirstToken(varDeclParent, {
253-
includeComments: false,
254-
filter: (t) => 'kind' in varDeclParent && t.value === varDeclParent.kind
255-
});
256-
if (!letKeywordToken) {
257-
return null;
258-
}
259-
260-
return new FixTracker(fixer, sourceCode)
261-
.retainRange(varDeclParent.range)
262-
.replaceTextRange(letKeywordToken.range, 'const');
263-
}
264-
});
265-
});
266-
}
267-
}
268308

269309
return {
270310
'Program:exit'() {
271-
groupByDestructuring(variables, ignoreReadBeforeAssign).forEach(checkGroup);
311+
const checker = new GroupChecker(context, {
312+
destructuring: options.destructuring
313+
});
314+
groupByDestructuring(variables, ignoreReadBeforeAssign).forEach((group) => {
315+
checker.checkAndReportNodes(group);
316+
});
272317
},
273318
VariableDeclaration(node) {
274319
if (node.kind === 'let' && !isInitOfForStatement(node)) {
@@ -419,6 +464,7 @@ function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign
419464
if (variable.eslintUsed && variable.scope.type === 'global') {
420465
return null;
421466
}
467+
422468
let writer = null;
423469
let isReadBeforeInit = false;
424470
const references = variable.references;
@@ -487,7 +533,7 @@ function groupByDestructuring(
487533
const references = variable.references;
488534
const identifier = getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign);
489535
if (!identifier) {
490-
return identifierMap;
536+
continue;
491537
}
492538

493539
let prevId: TSESTree.Identifier | TSESTree.JSXIdentifier | null = null;

‎packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-errors.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
- message: "'fn' is never reassigned. Use 'const' instead."
2+
line: 4
3+
column: 6
4+
suggestions: null

‎packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-input.svelte‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<script>
2+
let value = 1;
3+
4+
let fn = () => {
5+
value += 1;
6+
};
7+
</script>

‎packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-output.svelte‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<script>
2+
let value = 1;
3+
4+
const fn = () => {
5+
value += 1;
6+
};
7+
</script>

0 commit comments

Comments
 (0)