From fcf2b781d040c3b362b383d592cb0ef552e0fb01 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Thu, 28 Nov 2024 11:04:49 +0100 Subject: [PATCH 01/22] feat(prefer-const): add rule --- README.md | 1 + docs/rules.md | 1 + docs/rules/prefer-const.md | 52 ++ .../eslint-plugin-svelte/src/rule-types.ts | 10 + .../src/rules/prefer-const.ts | 546 ++++++++++++++++++ .../src/types-for-node.ts | 14 +- packages/eslint-plugin-svelte/src/types.ts | 2 + .../prefer-const/invalid/test01-errors.yaml | 12 + .../prefer-const/invalid/test01-input.svelte | 10 + .../prefer-const/invalid/test01-output.svelte | 10 + .../prefer-const/valid/test01-input.svelte | 3 + .../tests/src/rules/prefer-const.ts | 12 + 12 files changed, 665 insertions(+), 8 deletions(-) create mode 100644 docs/rules/prefer-const.md create mode 100644 packages/eslint-plugin-svelte/src/rules/prefer-const.ts create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/valid/test01-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts diff --git a/README.md b/README.md index 2601875d0..2f2dcf6fe 100644 --- a/README.md +++ b/README.md @@ -367,6 +367,7 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-unused-class-name](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-class-name/) | disallow the use of a class in the template without a corresponding style | | | [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: | | [svelte/no-useless-mustaches](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: | +| [svelte/prefer-const](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/) | Require `const` declarations for variables that are never reassigned after declared (excludes reactive values). | :wrench: | | [svelte/prefer-destructured-store-props](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/) | destructure values from object stores for better change tracking & fewer redraws | :bulb: | | [svelte/require-each-key](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/) | require keyed `{#each}` block | | | [svelte/require-event-dispatcher-types](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-event-dispatcher-types/) | require type parameters for `createEventDispatcher` | | diff --git a/docs/rules.md b/docs/rules.md index 29701ff7e..9a8d8978b 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -64,6 +64,7 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-unused-class-name](./rules/no-unused-class-name.md) | disallow the use of a class in the template without a corresponding style | | | [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: | | [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :wrench: | +| [svelte/prefer-const](./rules/prefer-const.md) | Require `const` declarations for variables that are never reassigned after declared (excludes reactive values). | :wrench: | | [svelte/prefer-destructured-store-props](./rules/prefer-destructured-store-props.md) | destructure values from object stores for better change tracking & fewer redraws | :bulb: | | [svelte/require-each-key](./rules/require-each-key.md) | require keyed `{#each}` block | | | [svelte/require-event-dispatcher-types](./rules/require-event-dispatcher-types.md) | require type parameters for `createEventDispatcher` | | diff --git a/docs/rules/prefer-const.md b/docs/rules/prefer-const.md new file mode 100644 index 000000000..862c4b8eb --- /dev/null +++ b/docs/rules/prefer-const.md @@ -0,0 +1,52 @@ +--- +pageClass: 'rule-details' +sidebarDepth: 0 +title: 'svelte/prefer-const' +description: 'Require `const` declarations for variables that are never reassigned after declared (excludes reactive values).' +--- + +# svelte/prefer-const + +> Require `const` declarations for variables that are never reassigned after declared (excludes reactive values). + +- :exclamation: **_This rule has not been released yet._** +- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule. + +## :book: Rule Details + +This rule reports ???. + + + + + +```svelte + + + + + +``` + + + +## :wrench: Options + +```json +{ + "svelte/prefer-const": ["error", {}] +} +``` + +- + +## :books: Further Reading + +- + +## :mag: Implementation + +- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/prefer-const.ts) +- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts) diff --git a/packages/eslint-plugin-svelte/src/rule-types.ts b/packages/eslint-plugin-svelte/src/rule-types.ts index 8b7cbe80c..a617817ea 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -264,6 +264,11 @@ export interface RuleOptions { * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-class-directive/ */ 'svelte/prefer-class-directive'?: Linter.RuleEntry + /** + * Require `const` declarations for variables that are never reassigned after declared (excludes reactive values). + * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/ + */ + 'svelte/prefer-const'?: Linter.RuleEntry /** * destructure values from object stores for better change tracking & fewer redraws * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/ @@ -485,6 +490,11 @@ type SvelteNoUselessMustaches = []|[{ type SveltePreferClassDirective = []|[{ prefer?: ("always" | "empty") }] +// ----- svelte/prefer-const ----- +type SveltePreferConst = []|[{ + destructuring?: ("any" | "all") + ignoreReadBeforeAssign?: boolean +}] // ----- svelte/shorthand-attribute ----- type SvelteShorthandAttribute = []|[{ prefer?: ("always" | "never") diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts new file mode 100644 index 000000000..66401f60a --- /dev/null +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -0,0 +1,546 @@ +import type { Reference, Variable, Scope } from '@typescript-eslint/scope-manager'; +import type { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/types'; +import { createRule } from '../utils'; +import type { RuleFixer, SourceCode } from '../types'; +import { getSourceCode } from '../utils/compat'; + +type ASTNode = TSESTree.Node; + +const PATTERN_TYPE = + /^(?:.+?Pattern|RestElement|SpreadProperty|ExperimentalRestProperty|Property)$/u; +const DECLARATION_HOST_TYPE = /^(?:Program|BlockStatement|StaticBlock|SwitchCase)$/u; +const DESTRUCTURING_HOST_TYPE = /^(?:VariableDeclarator|AssignmentExpression)$/u; + +type CheckedNode = NonNullable }>; + +function findIdentifierCallee(node: CheckedNode) { + const { parent } = node; + if (parent.type === 'VariableDeclarator' && parent.init?.type === 'CallExpression') { + return parent.init.callee; + } + + if ( + parent.type === 'Property' && + parent.parent.type === 'ObjectPattern' && + parent.parent.parent.type === 'VariableDeclarator' && + parent.parent.parent.init?.type === 'CallExpression' + ) { + return parent.parent.parent.init.callee; + } + + return null; +} + +/** + * Skip let statements when they are reactive values created by runes. + */ +function skipReactiveValues(identifier: ASTNode | null) { + if (!identifier || !identifier.parent) { + return false; + } + + const callee = findIdentifierCallee(identifier); + if (!callee) { + return true; + } + + if (callee.type === 'Identifier' && ['$state', '$props', '$derived'].includes(callee.name)) { + return false; + } + + if (callee.type !== 'MemberExpression' || callee.object.type !== 'Identifier') { + return true; + } + + if ( + callee.object.name === '$derived' && + callee.property.type === 'Identifier' && + callee.property.name === 'by' + ) { + return false; + } + if ( + callee.object.name === '$state' && + callee.property.type === 'Identifier' && + callee.property.name === 'frozen' + ) { + return false; + } + + return true; +} + +/** + * Rule and helpers are copied from ESLint + * https://github.com/eslint/eslint/blob/main/lib/rules/prefer-const.js + */ + +class FixTracker { + public fixer: RuleFixer; + + public sourceCode: SourceCode; + + public retainedRange: null | [number, number]; + + public constructor(fixer: RuleFixer, sourceCode: SourceCode) { + this.fixer = fixer; + this.sourceCode = sourceCode; + this.retainedRange = null; + } + + private getRetainedRange(range: [number, number]): [number, number] { + if (!this.retainedRange) { + return range; + } + + return [Math.min(this.retainedRange[0], range[0]), Math.max(this.retainedRange[1], range[1])]; + } + + public retainRange(range: [number, number]) { + this.retainedRange = range; + return this; + } + + public replaceTextRange(range: [number, number], text: string) { + const actualRange = this.getRetainedRange(range); + const actualText = + this.sourceCode.text.slice(actualRange[0], range[0]) + + text + + this.sourceCode.text.slice(range[1], actualRange[1]); + + return this.fixer.replaceTextRange(actualRange, actualText); + } +} + +export default createRule('prefer-const', { + meta: { + type: 'suggestion', + docs: { + description: + 'Require `const` declarations for variables that are never reassigned after declared (excludes reactive values).', + category: 'Best Practices', + recommended: false + }, + fixable: 'code', + schema: [ + { + type: 'object', + properties: { + destructuring: { enum: ['any', 'all'], default: 'any' }, + ignoreReadBeforeAssign: { type: 'boolean', default: false } + }, + additionalProperties: false + } + ], + messages: { + useConst: "'{{name}}' is never reassigned. Use 'const' instead." + } + }, + create(context) { + const sourceCode = getSourceCode(context); + + const options = context.options[0] || {}; + const shouldMatchAnyDestructuredVariable = options.destructuring !== 'all'; + const ignoreReadBeforeAssign = options.ignoreReadBeforeAssign === true; + + const variables: Variable[] = []; + let reportCount = 0; + let checkedId: TSESTree.BindingName | null = null; + let checkedName = ''; + + function checkGroup(nodes: (ASTNode | null)[]) { + const nodesToReport = nodes.filter(Boolean); + if ( + nodes.length && + (shouldMatchAnyDestructuredVariable || nodesToReport.length === nodes.length) && + nodes[0] !== null + ) { + const varDeclParent = findUp(nodes[0], 'VariableDeclaration', (parentNode: ASTNode) => + parentNode.type.endsWith('Statement') + ); + + const isVarDecParentNull = varDeclParent === null; + const isValidDecParent = + !isVarDecParentNull && + 'declarations' in varDeclParent && + varDeclParent.declarations.length > 0; + + if (isValidDecParent) { + const firstDeclaration = varDeclParent.declarations[0]; + + if (firstDeclaration.init) { + const firstDecParent = firstDeclaration.init.parent; + + if (firstDecParent.type === 'VariableDeclarator') { + const { id } = firstDecParent; + if ('name' in id && id.name !== checkedName) { + checkedName = id.name; + reportCount = 0; + } + + if (firstDecParent.id.type === 'ObjectPattern') { + const { init } = firstDecParent; + if (init && 'name' in init && init.name !== checkedName) { + checkedName = init.name; + reportCount = 0; + } + } + + if (firstDecParent.id !== checkedId) { + checkedId = firstDecParent.id; + reportCount = 0; + } + } + } + } + + let shouldFix = + varDeclParent && + (varDeclParent.parent.type === 'ForInStatement' || + varDeclParent.parent.type === 'ForOfStatement' || + ('declarations' in varDeclParent && + varDeclParent.declarations.every((declaration) => declaration.init))) && + nodesToReport.length === nodes.length; + + if ( + !isVarDecParentNull && + 'declarations' in varDeclParent && + varDeclParent.declarations && + varDeclParent.declarations.length !== 1 + ) { + if ( + varDeclParent && + varDeclParent.declarations && + varDeclParent.declarations.length >= 1 + ) { + reportCount += nodesToReport.length; + let totalDeclarationsCount = 0; + + varDeclParent.declarations.forEach((declaration) => { + if (declaration.id.type === 'ObjectPattern') { + totalDeclarationsCount += declaration.id.properties.length; + return; + } + + if (declaration.id.type === 'ArrayPattern') { + totalDeclarationsCount += declaration.id.elements.length; + return; + } + + totalDeclarationsCount += 1; + }); + shouldFix = shouldFix && reportCount === totalDeclarationsCount; + } + } + + if (!shouldFix) { + return; + } + + nodesToReport.filter(skipReactiveValues).forEach((node) => { + if (!node || !varDeclParent) { + // TS check + return; + } + + context.report({ + node, + messageId: 'useConst', + // @ts-expect-error Name will exist at this point + data: { name: node.name }, + fix: (fixer) => { + const letKeywordToken = sourceCode.getFirstToken(varDeclParent, { + includeComments: false, + filter: (t) => 'kind' in varDeclParent && t.value === varDeclParent.kind + }); + if (!letKeywordToken) { + return null; + } + + return new FixTracker(fixer, sourceCode) + .retainRange(varDeclParent.range) + .replaceTextRange(letKeywordToken.range, 'const'); + } + }); + }); + } + } + + return { + 'Program:exit'() { + groupByDestructuring(variables, ignoreReadBeforeAssign).forEach(checkGroup); + }, + VariableDeclaration(node) { + if (node.kind === 'let' && !isInitOfForStatement(node)) { + variables.push(...sourceCode.getDeclaredVariables(node)); + } + } + }; + } +}); + +/** + * Checks whether a given node is located at `ForStatement.init` or not. + */ +function isInitOfForStatement(node: ASTNode): boolean { + return Boolean(node.parent && node.parent.type === 'ForStatement' && node.parent.init === node); +} + +/** + * Checks whether a given Identifier node becomes a VariableDeclaration or not. + */ +function canBecomeVariableDeclaration(identifier: ASTNode): boolean { + let node = identifier.parent; + + while (node && PATTERN_TYPE.test(node.type)) { + node = node.parent; + } + if (!node) { + // TS check + return false; + } + + const isValidAssignmentExpr = + node.type === 'AssignmentExpression' && + node.parent.type === 'ExpressionStatement' && + DECLARATION_HOST_TYPE.test(node.parent.parent.type); + return node.type === 'VariableDeclarator' || isValidAssignmentExpr; +} + +/** + * Checks if an property or element is from outer scope or function parameters + * in destructing pattern. + */ +function isOuterVariableInDestructing(name: string, initScope: Scope): boolean { + if (initScope.through.some((ref) => ref.resolved && ref.resolved.name === name)) { + return true; + } + + const variable = getVariableByName(initScope, name); + return variable !== null ? variable.defs.some((def) => def.type === 'Parameter') : false; +} + +/** + * Gets the VariableDeclarator/AssignmentExpression node that a given reference + * belongs to. + * This is used to detect a mix of reassigned and never reassigned in a + * destructuring. + */ +function getDestructuringHost(reference: Reference): ASTNode | null { + if (!reference.isWrite()) { + return null; + } + + let node = reference.identifier.parent; + while (PATTERN_TYPE.test(node.type)) { + if (!node.parent) { + return null; + } + + node = node.parent; + } + + return !DESTRUCTURING_HOST_TYPE.test(node.type) ? null : node; +} + +/** + * Determines if a destructuring assignment node contains + * any MemberExpression nodes. This is used to determine if a + * variable that is only written once using destructuring can be + * safely converted into a const declaration. + */ +function hasMemberExpressionAssignment(node: ASTNode): boolean { + if (node.type === 'MemberExpression') { + return true; + } + if (node.type === 'ObjectPattern') { + return node.properties.some((prop) => + prop ? hasMemberExpressionAssignment('argument' in prop ? prop.argument : prop.value) : false + ); + } + if (node.type === 'ArrayPattern') { + return node.elements.some((element) => + element ? hasMemberExpressionAssignment(element) : false + ); + } + if (node.type === 'AssignmentPattern') { + return hasMemberExpressionAssignment(node.left); + } + + return false; +} + +function validateObjectPattern(node: TSESTree.ObjectPattern, variable: Variable) { + const properties = node.properties; + const hasOuterVariables = properties + .filter((prop) => prop.value) + .map((prop) => + 'value' in prop && prop.value + ? 'name' in prop.value + ? prop.value.name + : undefined + : undefined + ) + .some((name) => name && isOuterVariableInDestructing(name, variable.scope)); + const hasNonIdentifiers = hasMemberExpressionAssignment(node); + + return hasOuterVariables || hasNonIdentifiers; +} + +function validateArrayPattern(node: TSESTree.ArrayPattern, variable: Variable) { + const elements = node.elements; + const hasOuterVariables = elements + .map((element) => (element && 'name' in element ? element.name : undefined)) + .some((name) => name && isOuterVariableInDestructing(name, variable.scope)); + const hasNonIdentifiers = hasMemberExpressionAssignment(node); + + return hasOuterVariables || hasNonIdentifiers; +} + +/** + * Gets an identifier node of a given variable. + * + * If the initialization exists or one or more reading references exist before + * the first assignment, the identifier node is the node of the declaration. + * Otherwise, the identifier node is the node of the first assignment. + * + * If the variable should not change to const, this function returns null. + * - If the variable is reassigned. + * - If the variable is never initialized nor assigned. + * - If the variable is initialized in a different scope from the declaration. + * - If the unique assignment of the variable cannot change to a declaration. + * e.g. `if (a) b = 1` / `return (b = 1)` + * - If the variable is declared in the global scope and `eslintUsed` is `true`. + * `/*exported foo` directive comment makes such variables. This rule does not + * warn such variables because this rule cannot distinguish whether the + * exported variables are reassigned or not. + */ +function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign: boolean) { + if (variable.eslintUsed && variable.scope.type === 'global') { + return null; + } + let writer = null; + let isReadBeforeInit = false; + const references = variable.references; + for (let i = 0; i < references.length; ++i) { + const reference = references[i]; + + if (reference.isWrite()) { + const isReassigned = writer !== null && writer.identifier !== reference.identifier; + if (isReassigned) { + return null; + } + + const destructuringHost = getDestructuringHost(reference); + const isValidDestructuringHost = + destructuringHost !== null && + 'left' in destructuringHost && + destructuringHost.left !== undefined; + + if (isValidDestructuringHost) { + const leftNode = destructuringHost.left; + + if (leftNode.type === 'ObjectPattern' && validateObjectPattern(leftNode, variable)) { + return null; + } else if (leftNode.type === 'ArrayPattern' && validateArrayPattern(leftNode, variable)) { + return null; + } + } + + writer = reference; + } else if (reference.isRead() && writer === null) { + if (ignoreReadBeforeAssign) { + return null; + } + isReadBeforeInit = true; + } + } + + const shouldBeConst = + writer !== null && + writer.from === variable.scope && + canBecomeVariableDeclaration(writer.identifier); + if (!shouldBeConst) { + return null; + } + if (isReadBeforeInit) { + return variable.defs[0].name; + } + + return writer?.identifier; +} + +/** + * Groups by the VariableDeclarator/AssignmentExpression node that each + * reference of given variables belongs to. + * This is used to detect a mix of reassigned and never reassigned in a + * destructuring. + */ +function groupByDestructuring( + variables: Variable[], + ignoreReadBeforeAssign: boolean +): Map { + const identifierMap = new Map(); + + for (let i = 0; i < variables.length; ++i) { + const variable = variables[i]; + const references = variable.references; + const identifier = getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign); + if (!identifier) { + return identifierMap; + } + + let prevId: TSESTree.Identifier | TSESTree.JSXIdentifier | null = null; + references.forEach((reference) => { + const id = reference.identifier; + if (id === prevId) { + return; + } + + prevId = id; + const group = getDestructuringHost(reference); + if (!group) { + return; + } + + if (identifierMap.has(group)) { + identifierMap.get(group)!.push(identifier); + } else { + identifierMap.set(group, [identifier]); + } + }); + } + + return identifierMap; +} + +/** + * Finds the nearest parent of node with a given type. + */ +function findUp(node: ASTNode, type: `${AST_NODE_TYPES}`, shouldStop: (node: ASTNode) => boolean) { + if (!node || shouldStop(node) || !node.parent) { + return null; + } + if (node.type === type) { + return node; + } + + return findUp(node.parent, type, shouldStop); +} + +/** + * Finds the variable by a given name in a given scope and its upper scopes. + */ +function getVariableByName(initScope: Scope, name: string): Variable | null { + let scope: Scope | null = initScope; + + while (scope) { + const variable = scope.set.get(name); + if (variable) { + return variable; + } + scope = scope.upper; + } + + return null; +} diff --git a/packages/eslint-plugin-svelte/src/types-for-node.ts b/packages/eslint-plugin-svelte/src/types-for-node.ts index a1bf991a5..866ea6dab 100644 --- a/packages/eslint-plugin-svelte/src/types-for-node.ts +++ b/packages/eslint-plugin-svelte/src/types-for-node.ts @@ -135,7 +135,6 @@ export type ASTNodeListener = { TSEmptyBodyFunctionExpression?: ( node: TSESTree.TSEmptyBodyFunctionExpression & ASTNodeWithParent ) => void; - TSEnumBody?: (node: TSESTree.TSEnumBody & ASTNodeWithParent) => void; TSEnumDeclaration?: (node: TSESTree.TSEnumDeclaration & ASTNodeWithParent) => void; TSEnumMember?: (node: TSESTree.TSEnumMember & ASTNodeWithParent) => void; TSExportAssignment?: (node: TSESTree.TSExportAssignment & ASTNodeWithParent) => void; @@ -144,6 +143,9 @@ export type ASTNodeListener = { node: TSESTree.TSExternalModuleReference & ASTNodeWithParent ) => void; TSFunctionType?: (node: TSESTree.TSFunctionType & ASTNodeWithParent) => void; + TSInstantiationExpression?: ( + node: TSESTree.TSInstantiationExpression & ASTNodeWithParent + ) => void; TSImportEqualsDeclaration?: ( node: TSESTree.TSImportEqualsDeclaration & ASTNodeWithParent ) => void; @@ -151,9 +153,6 @@ export type ASTNodeListener = { TSIndexedAccessType?: (node: TSESTree.TSIndexedAccessType & ASTNodeWithParent) => void; TSIndexSignature?: (node: TSESTree.TSIndexSignature & ASTNodeWithParent) => void; TSInferType?: (node: TSESTree.TSInferType & ASTNodeWithParent) => void; - TSInstantiationExpression?: ( - node: TSESTree.TSInstantiationExpression & ASTNodeWithParent - ) => void; TSInterfaceBody?: (node: TSESTree.TSInterfaceBody & ASTNodeWithParent) => void; TSInterfaceDeclaration?: (node: TSESTree.TSInterfaceDeclaration & ASTNodeWithParent) => void; TSInterfaceHeritage?: (node: TSESTree.TSInterfaceHeritage & ASTNodeWithParent) => void; @@ -355,7 +354,6 @@ export type TSNodeListener = { TSEmptyBodyFunctionExpression?: ( node: TSESTree.TSEmptyBodyFunctionExpression & ASTNodeWithParent ) => void; - TSEnumBody?: (node: TSESTree.TSEnumBody & ASTNodeWithParent) => void; TSEnumDeclaration?: (node: TSESTree.TSEnumDeclaration & ASTNodeWithParent) => void; TSEnumMember?: (node: TSESTree.TSEnumMember & ASTNodeWithParent) => void; TSExportAssignment?: (node: TSESTree.TSExportAssignment & ASTNodeWithParent) => void; @@ -364,6 +362,9 @@ export type TSNodeListener = { node: TSESTree.TSExternalModuleReference & ASTNodeWithParent ) => void; TSFunctionType?: (node: TSESTree.TSFunctionType & ASTNodeWithParent) => void; + TSInstantiationExpression?: ( + node: TSESTree.TSInstantiationExpression & ASTNodeWithParent + ) => void; TSImportEqualsDeclaration?: ( node: TSESTree.TSImportEqualsDeclaration & ASTNodeWithParent ) => void; @@ -371,9 +372,6 @@ export type TSNodeListener = { TSIndexedAccessType?: (node: TSESTree.TSIndexedAccessType & ASTNodeWithParent) => void; TSIndexSignature?: (node: TSESTree.TSIndexSignature & ASTNodeWithParent) => void; TSInferType?: (node: TSESTree.TSInferType & ASTNodeWithParent) => void; - TSInstantiationExpression?: ( - node: TSESTree.TSInstantiationExpression & ASTNodeWithParent - ) => void; TSInterfaceBody?: (node: TSESTree.TSInterfaceBody & ASTNodeWithParent) => void; TSInterfaceDeclaration?: (node: TSESTree.TSInterfaceDeclaration & ASTNodeWithParent) => void; TSInterfaceHeritage?: (node: TSESTree.TSInterfaceHeritage & ASTNodeWithParent) => void; diff --git a/packages/eslint-plugin-svelte/src/types.ts b/packages/eslint-plugin-svelte/src/types.ts index 7b6452b52..96a6abd9b 100644 --- a/packages/eslint-plugin-svelte/src/types.ts +++ b/packages/eslint-plugin-svelte/src/types.ts @@ -232,6 +232,8 @@ export interface SourceCode { getLines(): string[]; + getDeclaredVariables(node: TSESTree.Node): Variable[]; + getAllComments(): AST.Comment[]; getComments(node: NodeOrToken): { diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml new file mode 100644 index 000000000..5e2a52173 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml @@ -0,0 +1,12 @@ +- message: "'zero' is never reassigned. Use 'const' instead." + line: 3 + column: 6 + suggestions: null +- message: "'doubled' is never reassigned. Use 'const' instead." + line: 6 + column: 6 + suggestions: null +- message: "'calculated' is never reassigned. Use 'const' instead." + line: 8 + column: 6 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-input.svelte new file mode 100644 index 000000000..0f5f5e54f --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-input.svelte @@ -0,0 +1,10 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte new file mode 100644 index 000000000..3138619fd --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte @@ -0,0 +1,10 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/valid/test01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/valid/test01-input.svelte new file mode 100644 index 000000000..5c768eb34 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/valid/test01-input.svelte @@ -0,0 +1,3 @@ + diff --git a/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts new file mode 100644 index 000000000..ed8e50445 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts @@ -0,0 +1,12 @@ +import { RuleTester } from '../../utils/eslint-compat'; +import rule from '../../../src/rules/prefer-const'; +import { loadTestCases } from '../../utils/utils'; + +const tester = new RuleTester({ + languageOptions: { + ecmaVersion: 2020, + sourceType: 'module' + } +}); + +tester.run('prefer-const', rule as any, loadTestCases('prefer-const')); From abaff12653f10d269e213543808786c3916cd06d Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Thu, 28 Nov 2024 11:05:35 +0100 Subject: [PATCH 02/22] chore: add changeset --- .changeset/green-squids-compete.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/green-squids-compete.md diff --git a/.changeset/green-squids-compete.md b/.changeset/green-squids-compete.md new file mode 100644 index 000000000..7666a2ad1 --- /dev/null +++ b/.changeset/green-squids-compete.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-svelte': minor +--- + +Add `prefer-const` rule From dac38c4b020ab2c84e01e49f6ff7987133e59c4e Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Thu, 28 Nov 2024 11:27:44 +0100 Subject: [PATCH 03/22] docs(prefer-const): update docs --- README.md | 2 +- docs/rules.md | 2 +- docs/rules/prefer-const.md | 4 ++-- packages/eslint-plugin-svelte/src/rule-types.ts | 2 +- packages/eslint-plugin-svelte/src/rules/prefer-const.ts | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 2f2dcf6fe..b7807529e 100644 --- a/README.md +++ b/README.md @@ -367,7 +367,7 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-unused-class-name](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-class-name/) | disallow the use of a class in the template without a corresponding style | | | [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: | | [svelte/no-useless-mustaches](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: | -| [svelte/prefer-const](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/) | Require `const` declarations for variables that are never reassigned after declared (excludes reactive values). | :wrench: | +| [svelte/prefer-const](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/) | Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values). | :wrench: | | [svelte/prefer-destructured-store-props](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/) | destructure values from object stores for better change tracking & fewer redraws | :bulb: | | [svelte/require-each-key](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/) | require keyed `{#each}` block | | | [svelte/require-event-dispatcher-types](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-event-dispatcher-types/) | require type parameters for `createEventDispatcher` | | diff --git a/docs/rules.md b/docs/rules.md index 9a8d8978b..2f2870cc8 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -64,7 +64,7 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-unused-class-name](./rules/no-unused-class-name.md) | disallow the use of a class in the template without a corresponding style | | | [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: | | [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :wrench: | -| [svelte/prefer-const](./rules/prefer-const.md) | Require `const` declarations for variables that are never reassigned after declared (excludes reactive values). | :wrench: | +| [svelte/prefer-const](./rules/prefer-const.md) | Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values). | :wrench: | | [svelte/prefer-destructured-store-props](./rules/prefer-destructured-store-props.md) | destructure values from object stores for better change tracking & fewer redraws | :bulb: | | [svelte/require-each-key](./rules/require-each-key.md) | require keyed `{#each}` block | | | [svelte/require-event-dispatcher-types](./rules/require-event-dispatcher-types.md) | require type parameters for `createEventDispatcher` | | diff --git a/docs/rules/prefer-const.md b/docs/rules/prefer-const.md index 862c4b8eb..1522a760f 100644 --- a/docs/rules/prefer-const.md +++ b/docs/rules/prefer-const.md @@ -2,12 +2,12 @@ pageClass: 'rule-details' sidebarDepth: 0 title: 'svelte/prefer-const' -description: 'Require `const` declarations for variables that are never reassigned after declared (excludes reactive values).' +description: 'Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values).' --- # svelte/prefer-const -> Require `const` declarations for variables that are never reassigned after declared (excludes reactive values). +> Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values). - :exclamation: **_This rule has not been released yet._** - :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule. diff --git a/packages/eslint-plugin-svelte/src/rule-types.ts b/packages/eslint-plugin-svelte/src/rule-types.ts index a617817ea..0403bb4ae 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -265,7 +265,7 @@ export interface RuleOptions { */ 'svelte/prefer-class-directive'?: Linter.RuleEntry /** - * Require `const` declarations for variables that are never reassigned after declared (excludes reactive values). + * Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values). * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/ */ 'svelte/prefer-const'?: Linter.RuleEntry diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index 66401f60a..f79a4f702 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -117,7 +117,7 @@ export default createRule('prefer-const', { type: 'suggestion', docs: { description: - 'Require `const` declarations for variables that are never reassigned after declared (excludes reactive values).', + 'Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values).', category: 'Best Practices', recommended: false }, From efef6ba803f2c7a724e54c3833346fe1293ea507 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Fri, 29 Nov 2024 09:50:41 +0100 Subject: [PATCH 04/22] docs(prefer-const): improve rule docs --- docs/rules/prefer-const.md | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/docs/rules/prefer-const.md b/docs/rules/prefer-const.md index 1522a760f..18eb96cc8 100644 --- a/docs/rules/prefer-const.md +++ b/docs/rules/prefer-const.md @@ -14,7 +14,7 @@ description: 'Require `const` declarations for variables that are never reassign ## :book: Rule Details -This rule reports ???. +This rule reports the same as the base ESLint `prefer-const` rule, except that ignores Svelte reactive values such as `$state`, `$derived` and `$props`. If this rule is active, make sure to disable the base `prefer-const` rule, as it will conflict with this rule. @@ -23,11 +23,16 @@ This rule reports ???. ```svelte - + + const { a, b } = $props(); + let { a, b } = $state(); + - + + // Imagine obj is not re-assigned, therefore it should be constant + let obj = { a, b }; + ``` @@ -36,15 +41,24 @@ This rule reports ???. ```json { - "svelte/prefer-const": ["error", {}] + "svelte/prefer-const": [ + "error", + { + "destructuring": "any", + "ignoreReadonly": true + } + ] } ``` -- +- `destructuring`: The kind of the way to address variables in destructuring. There are 2 values: + - `any` (default): if any variables in destructuring should be const, this rule warns for those variables. + - `all`: if all variables in destructuring should be const, this rule warns the variables. Otherwise, ignores them. +- `ignoreReadonly`: If `true`, this rule will ignore variables that are read between the declaration and the _first_ assignment. ## :books: Further Reading -- +- See [ESLint `prefer-const` rule](https://eslint.org/docs/latest/rules/prefer-const) for more information about the base rule. ## :mag: Implementation From 629e1b1a3188438389b231a15bc4a226ad3fceb3 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Fri, 29 Nov 2024 09:50:53 +0100 Subject: [PATCH 05/22] 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. --- .../src/rules/prefer-const.ts | 296 ++++++++++-------- .../prefer-const/invalid/test02-errors.yaml | 4 + .../prefer-const/invalid/test02-input.svelte | 7 + .../prefer-const/invalid/test02-output.svelte | 7 + 4 files changed, 189 insertions(+), 125 deletions(-) create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-output.svelte diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index f79a4f702..14055df26 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -1,7 +1,7 @@ import type { Reference, Variable, Scope } from '@typescript-eslint/scope-manager'; import type { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/types'; import { createRule } from '../utils'; -import type { RuleFixer, SourceCode } from '../types'; +import type { RuleContext, RuleFixer, SourceCode } from '../types'; import { getSourceCode } from '../utils/compat'; type ASTNode = TSESTree.Node; @@ -112,6 +112,168 @@ class FixTracker { } } +type CheckOptions = { destructuring: 'any' | 'all' }; +type VariableDeclaration = + | TSESTree.LetOrConstOrVarDeclaration + | TSESTree.UsingInForOfDeclaration + | TSESTree.UsingInNormalContextDeclaration; +type VariableDeclarator = + | TSESTree.LetOrConstOrVarDeclarator + | TSESTree.UsingInForOfDeclarator + | TSESTree.UsingInNomalConextDeclarator; +class GroupChecker { + private reportCount = 0; + + private checkedId: TSESTree.BindingName | null = null; + + private checkedName = ''; + + private readonly shouldMatchAnyDestructuredVariable: boolean; + + private readonly context: RuleContext; + + public constructor(context: RuleContext, { destructuring }: CheckOptions) { + this.context = context; + this.shouldMatchAnyDestructuredVariable = destructuring !== 'all'; + } + + public checkAndReportNodes(nodes: ASTNode[]) { + const shouldCheckGroup = nodes.length && this.shouldMatchAnyDestructuredVariable; + if (!shouldCheckGroup) { + return; + } + + const variableDeclarationParent = findUp( + nodes[0], + 'VariableDeclaration', + (parentNode: ASTNode) => parentNode.type.endsWith('Statement') + ); + const isVarDecParentNull = variableDeclarationParent === null; + const isValidDecParent = + !isVarDecParentNull && + 'declarations' in variableDeclarationParent && + variableDeclarationParent.declarations.length > 0; + if (!isValidDecParent) { + return; + } + + const dec = variableDeclarationParent.declarations[0]; + this.checkDeclarator(dec); + + const shouldFix = this.checkShouldFix(variableDeclarationParent, nodes.length); + if (!shouldFix) { + return; + } + + const sourceCode = getSourceCode(this.context); + nodes.filter(skipReactiveValues).forEach((node) => { + this.report(sourceCode, node, variableDeclarationParent); + }); + } + + private report( + sourceCode: ReturnType, + node: ASTNode, + nodeParent: VariableDeclaration + ) { + this.context.report({ + node, + messageId: 'useConst', + // @ts-expect-error Name will exist at this point + data: { name: node.name }, + fix: (fixer) => { + const letKeywordToken = sourceCode.getFirstToken(nodeParent, { + includeComments: false, + filter: (t) => 'kind' in nodeParent && t.value === nodeParent.kind + }); + if (!letKeywordToken) { + return null; + } + + return new FixTracker(fixer, sourceCode) + .retainRange(nodeParent.range) + .replaceTextRange(letKeywordToken.range, 'const'); + } + }); + } + + private checkShouldFix(declaration: VariableDeclaration, totalNodes: number) { + const shouldFix = + declaration && + (declaration.parent.type === 'ForInStatement' || + declaration.parent.type === 'ForOfStatement' || + ('declarations' in declaration && + declaration.declarations.every((declaration) => declaration.init))); + + const totalDeclarationCount = this.checkDestructuredDeclaration(declaration, totalNodes); + if (totalDeclarationCount === -1) { + return shouldFix; + } + + return shouldFix && this.reportCount === totalDeclarationCount; + } + + private checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) { + const hasMultipleDeclarations = + declaration !== null && + 'declarations' in declaration && + declaration.declarations.length !== 1; + if (!hasMultipleDeclarations) { + return -1; + } + + const hasMoreThanOneDeclaration = + declaration && declaration.declarations && declaration.declarations.length >= 1; + if (!hasMoreThanOneDeclaration) { + return -1; + } + + this.reportCount += totalNodes; + + return declaration.declarations.reduce((total, declaration) => { + if (declaration.id.type === 'ObjectPattern') { + return total + declaration.id.properties.length; + } + + if (declaration.id.type === 'ArrayPattern') { + return total + declaration.id.elements.length; + } + + return total + 1; + }, 0); + } + + private checkDeclarator(declarator: VariableDeclarator) { + if (!declarator.init) { + return; + } + + const firstDecParent = declarator.init.parent; + if (firstDecParent.type !== 'VariableDeclarator') { + return; + } + + const { id } = firstDecParent; + if ('name' in id && id.name !== this.checkedName) { + this.checkedName = id.name; + this.reportCount = 0; + } + + if (firstDecParent.id.type === 'ObjectPattern') { + const { init } = firstDecParent; + if (init && 'name' in init && init.name !== this.checkedName) { + this.checkedName = init.name; + this.reportCount = 0; + } + } + + if (firstDecParent.id !== this.checkedId) { + this.checkedId = firstDecParent.id; + this.reportCount = 0; + } + } +} + export default createRule('prefer-const', { meta: { type: 'suggestion', @@ -140,135 +302,18 @@ export default createRule('prefer-const', { const sourceCode = getSourceCode(context); const options = context.options[0] || {}; - const shouldMatchAnyDestructuredVariable = options.destructuring !== 'all'; const ignoreReadBeforeAssign = options.ignoreReadBeforeAssign === true; const variables: Variable[] = []; - let reportCount = 0; - let checkedId: TSESTree.BindingName | null = null; - let checkedName = ''; - - function checkGroup(nodes: (ASTNode | null)[]) { - const nodesToReport = nodes.filter(Boolean); - if ( - nodes.length && - (shouldMatchAnyDestructuredVariable || nodesToReport.length === nodes.length) && - nodes[0] !== null - ) { - const varDeclParent = findUp(nodes[0], 'VariableDeclaration', (parentNode: ASTNode) => - parentNode.type.endsWith('Statement') - ); - - const isVarDecParentNull = varDeclParent === null; - const isValidDecParent = - !isVarDecParentNull && - 'declarations' in varDeclParent && - varDeclParent.declarations.length > 0; - - if (isValidDecParent) { - const firstDeclaration = varDeclParent.declarations[0]; - - if (firstDeclaration.init) { - const firstDecParent = firstDeclaration.init.parent; - - if (firstDecParent.type === 'VariableDeclarator') { - const { id } = firstDecParent; - if ('name' in id && id.name !== checkedName) { - checkedName = id.name; - reportCount = 0; - } - - if (firstDecParent.id.type === 'ObjectPattern') { - const { init } = firstDecParent; - if (init && 'name' in init && init.name !== checkedName) { - checkedName = init.name; - reportCount = 0; - } - } - - if (firstDecParent.id !== checkedId) { - checkedId = firstDecParent.id; - reportCount = 0; - } - } - } - } - - let shouldFix = - varDeclParent && - (varDeclParent.parent.type === 'ForInStatement' || - varDeclParent.parent.type === 'ForOfStatement' || - ('declarations' in varDeclParent && - varDeclParent.declarations.every((declaration) => declaration.init))) && - nodesToReport.length === nodes.length; - - if ( - !isVarDecParentNull && - 'declarations' in varDeclParent && - varDeclParent.declarations && - varDeclParent.declarations.length !== 1 - ) { - if ( - varDeclParent && - varDeclParent.declarations && - varDeclParent.declarations.length >= 1 - ) { - reportCount += nodesToReport.length; - let totalDeclarationsCount = 0; - - varDeclParent.declarations.forEach((declaration) => { - if (declaration.id.type === 'ObjectPattern') { - totalDeclarationsCount += declaration.id.properties.length; - return; - } - - if (declaration.id.type === 'ArrayPattern') { - totalDeclarationsCount += declaration.id.elements.length; - return; - } - - totalDeclarationsCount += 1; - }); - shouldFix = shouldFix && reportCount === totalDeclarationsCount; - } - } - - if (!shouldFix) { - return; - } - - nodesToReport.filter(skipReactiveValues).forEach((node) => { - if (!node || !varDeclParent) { - // TS check - return; - } - - context.report({ - node, - messageId: 'useConst', - // @ts-expect-error Name will exist at this point - data: { name: node.name }, - fix: (fixer) => { - const letKeywordToken = sourceCode.getFirstToken(varDeclParent, { - includeComments: false, - filter: (t) => 'kind' in varDeclParent && t.value === varDeclParent.kind - }); - if (!letKeywordToken) { - return null; - } - - return new FixTracker(fixer, sourceCode) - .retainRange(varDeclParent.range) - .replaceTextRange(letKeywordToken.range, 'const'); - } - }); - }); - } - } return { 'Program:exit'() { - groupByDestructuring(variables, ignoreReadBeforeAssign).forEach(checkGroup); + const checker = new GroupChecker(context, { + destructuring: options.destructuring + }); + groupByDestructuring(variables, ignoreReadBeforeAssign).forEach((group) => { + checker.checkAndReportNodes(group); + }); }, VariableDeclaration(node) { if (node.kind === 'let' && !isInitOfForStatement(node)) { @@ -419,6 +464,7 @@ function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign if (variable.eslintUsed && variable.scope.type === 'global') { return null; } + let writer = null; let isReadBeforeInit = false; const references = variable.references; @@ -487,7 +533,7 @@ function groupByDestructuring( const references = variable.references; const identifier = getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign); if (!identifier) { - return identifierMap; + continue; } let prevId: TSESTree.Identifier | TSESTree.JSXIdentifier | null = null; diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-errors.yaml new file mode 100644 index 000000000..0c4b62dd9 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-errors.yaml @@ -0,0 +1,4 @@ +- message: "'fn' is never reassigned. Use 'const' instead." + line: 4 + column: 6 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-input.svelte new file mode 100644 index 000000000..82fdb483b --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-input.svelte @@ -0,0 +1,7 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-output.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-output.svelte new file mode 100644 index 000000000..da091dd5a --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-output.svelte @@ -0,0 +1,7 @@ + From 915ce7dcd86e39f7a31c1110bb47d1c2407f87c4 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Fri, 29 Nov 2024 10:51:38 +0100 Subject: [PATCH 06/22] fix(prefer-const): remove unnecessary `FixTracker` Code has been moved into `prefer-const-helpers` folder, so the rule file is simpler. --- .../src/rules/prefer-const-helpers/index.ts | 518 +++++++++++++++++ .../src/rules/prefer-const.ts | 536 +----------------- 2 files changed, 520 insertions(+), 534 deletions(-) create mode 100644 packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts new file mode 100644 index 000000000..f62b2d694 --- /dev/null +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts @@ -0,0 +1,518 @@ +import type { Reference, Variable, Scope } from '@typescript-eslint/scope-manager'; +import type { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/types'; + +import { getSourceCode } from '../../utils/compat'; +import type { RuleContext } from '../../types'; + +type ASTNode = TSESTree.Node; +type CheckedNode = NonNullable }>; + +const PATTERN_TYPE = + /^(?:.+?Pattern|RestElement|SpreadProperty|ExperimentalRestProperty|Property)$/u; +const DECLARATION_HOST_TYPE = /^(?:Program|BlockStatement|StaticBlock|SwitchCase)$/u; +const DESTRUCTURING_HOST_TYPE = /^(?:VariableDeclarator|AssignmentExpression)$/u; + +function findIdentifierCallee(node: CheckedNode) { + const { parent } = node; + if (parent.type === 'VariableDeclarator' && parent.init?.type === 'CallExpression') { + return parent.init.callee; + } + + if ( + parent.type === 'Property' && + parent.parent.type === 'ObjectPattern' && + parent.parent.parent.type === 'VariableDeclarator' && + parent.parent.parent.init?.type === 'CallExpression' + ) { + return parent.parent.parent.init.callee; + } + + return null; +} + +/** + * Skip let statements when they are reactive values created by runes. + */ +function skipReactiveValues(identifier: ASTNode | null) { + if (!identifier || !identifier.parent) { + return false; + } + + const callee = findIdentifierCallee(identifier); + if (!callee) { + return true; + } + + if (callee.type === 'Identifier' && ['$state', '$props', '$derived'].includes(callee.name)) { + return false; + } + + if (callee.type !== 'MemberExpression' || callee.object.type !== 'Identifier') { + return true; + } + + if ( + callee.object.name === '$derived' && + callee.property.type === 'Identifier' && + callee.property.name === 'by' + ) { + return false; + } + if ( + callee.object.name === '$state' && + callee.property.type === 'Identifier' && + callee.property.name === 'frozen' + ) { + return false; + } + + return true; +} + +/** + * Checks whether a given Identifier node becomes a VariableDeclaration or not. + */ +function canBecomeVariableDeclaration(identifier: ASTNode) { + let node = identifier.parent; + + while (node && PATTERN_TYPE.test(node.type)) { + node = node.parent; + } + if (!node) { + // TS check + return false; + } + + const isValidAssignmentExpr = + node.type === 'AssignmentExpression' && + node.parent.type === 'ExpressionStatement' && + DECLARATION_HOST_TYPE.test(node.parent.parent.type); + return node.type === 'VariableDeclarator' || isValidAssignmentExpr; +} + +/** + * Checks if an property or element is from outer scope or export function parameters + * in destructing pattern. + */ +function isOuterVariableInDestructing(name: string, initScope: Scope) { + if (initScope.through.some((ref) => ref.resolved && ref.resolved.name === name)) { + return true; + } + + const variable = getVariableByName(initScope, name); + return variable !== null ? variable.defs.some((def) => def.type === 'Parameter') : false; +} + +/** + * Gets the VariableDeclarator/AssignmentExpression node that a given reference + * belongs to. + * This is used to detect a mix of reassigned and never reassigned in a + * destructuring. + */ +function getDestructuringHost(reference: Reference) { + if (!reference.isWrite()) { + return null; + } + + let node = reference.identifier.parent; + while (PATTERN_TYPE.test(node.type)) { + if (!node.parent) { + return null; + } + + node = node.parent; + } + + return !DESTRUCTURING_HOST_TYPE.test(node.type) ? null : node; +} + +/** + * Determines if a destructuring assignment node contains + * any MemberExpression nodes. This is used to determine if a + * variable that is only written once using destructuring can be + * safely converted into a const declaration. + */ +function hasMemberExpressionAssignment(node: ASTNode): boolean { + if (node.type === 'MemberExpression') { + return true; + } + if (node.type === 'ObjectPattern') { + return node.properties.some((prop) => + prop ? hasMemberExpressionAssignment('argument' in prop ? prop.argument : prop.value) : false + ); + } + if (node.type === 'ArrayPattern') { + return node.elements.some((element) => + element ? hasMemberExpressionAssignment(element) : false + ); + } + if (node.type === 'AssignmentPattern') { + return hasMemberExpressionAssignment(node.left); + } + + return false; +} + +function validateObjectPattern(node: TSESTree.ObjectPattern, variable: Variable) { + const properties = node.properties; + const hasOuterVariables = properties + .filter((prop) => prop.value) + .map((prop) => + 'value' in prop && prop.value + ? 'name' in prop.value + ? prop.value.name + : undefined + : undefined + ) + .some((name) => name && isOuterVariableInDestructing(name, variable.scope)); + const hasNonIdentifiers = hasMemberExpressionAssignment(node); + + return hasOuterVariables || hasNonIdentifiers; +} + +function validateArrayPattern(node: TSESTree.ArrayPattern, variable: Variable): boolean { + const elements = node.elements; + const hasOuterVariables = elements + .map((element) => (element && 'name' in element ? element.name : undefined)) + .some((name) => name && isOuterVariableInDestructing(name, variable.scope)); + const hasNonIdentifiers = hasMemberExpressionAssignment(node); + + return hasOuterVariables || hasNonIdentifiers; +} + +/** + * Gets an identifier node of a given variable. + * + * If the initialization exists or one or more reading references exist before + * the first assignment, the identifier node is the node of the declaration. + * Otherwise, the identifier node is the node of the first assignment. + * + * If the variable should not change to const, this export function returns null. + * - If the variable is reassigned. + * - If the variable is never initialized nor assigned. + * - If the variable is initialized in a different scope from the declaration. + * - If the unique assignment of the variable cannot change to a declaration. + * e.g. `if (a) b = 1` / `return (b = 1)` + * - If the variable is declared in the global scope and `eslintUsed` is `true`. + * `/*exported foo` directive comment makes such variables. This rule does not + * warn such variables because this rule cannot distinguish whether the + * exported variables are reassigned or not. + */ +function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign: boolean) { + if (variable.eslintUsed && variable.scope.type === 'global') { + return null; + } + + let writer = null; + let isReadBeforeInit = false; + const references = variable.references; + for (let i = 0; i < references.length; ++i) { + const reference = references[i]; + + if (reference.isWrite()) { + const isReassigned = writer !== null && writer.identifier !== reference.identifier; + if (isReassigned) { + return null; + } + + const destructuringHost = getDestructuringHost(reference); + const isValidDestructuringHost = + destructuringHost !== null && + 'left' in destructuringHost && + destructuringHost.left !== undefined; + + if (isValidDestructuringHost) { + const leftNode = destructuringHost.left; + + if (leftNode.type === 'ObjectPattern' && validateObjectPattern(leftNode, variable)) { + return null; + } else if (leftNode.type === 'ArrayPattern' && validateArrayPattern(leftNode, variable)) { + return null; + } + } + + writer = reference; + } else if (reference.isRead() && writer === null) { + if (ignoreReadBeforeAssign) { + return null; + } + isReadBeforeInit = true; + } + } + + const shouldBeConst = + writer !== null && + writer.from === variable.scope && + canBecomeVariableDeclaration(writer.identifier); + if (!shouldBeConst) { + return null; + } + if (isReadBeforeInit) { + return variable.defs[0].name; + } + + return writer?.identifier; +} + +/** + * Finds the nearest parent of node with a given type. + */ +function findUp(node: ASTNode, type: `${AST_NODE_TYPES}`, shouldStop: (node: ASTNode) => boolean) { + if (!node || shouldStop(node) || !node.parent) { + return null; + } + if (node.type === type) { + return node; + } + + return findUp(node.parent, type, shouldStop); +} + +/** + * Finds the variable by a given name in a given scope and its upper scopes. + */ +function getVariableByName(initScope: Scope, name: string): Variable | null { + let scope: Scope | null = initScope; + + while (scope) { + const variable = scope.set.get(name); + if (variable) { + return variable; + } + scope = scope.upper; + } + + return null; +} + +/** + * Checks whether a given node is located at `ForStatement.init` or not. + */ +export function isInitOfForStatement(node: ASTNode): boolean { + return Boolean(node.parent && node.parent.type === 'ForStatement' && node.parent.init === node); +} + +/** + * Groups by the VariableDeclarator/AssignmentExpression node that each + * reference of given variables belongs to. + * This is used to detect a mix of reassigned and never reassigned in a + * destructuring. + */ +export function groupByDestructuring( + variables: Variable[], + ignoreReadBeforeAssign: boolean +): Map { + const identifierMap = new Map(); + + for (let i = 0; i < variables.length; ++i) { + const variable = variables[i]; + const references = variable.references; + const identifier = getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign); + if (!identifier) { + continue; + } + + let prevId: TSESTree.Identifier | TSESTree.JSXIdentifier | null = null; + references.forEach((reference) => { + const id = reference.identifier; + if (id === prevId) { + return; + } + + prevId = id; + const group = getDestructuringHost(reference); + if (!group) { + return; + } + + if (identifierMap.has(group)) { + identifierMap.get(group)!.push(identifier); + } else { + identifierMap.set(group, [identifier]); + } + }); + } + + return identifierMap; +} + +function calculateRetainedTextRange( + range: TSESTree.Range, + retainedRange: TSESTree.Range | null +): [TSESTree.Range, TSESTree.Range] { + const actualRange = retainedRange + ? [Math.min(retainedRange[0], range[0]), Math.max(retainedRange[1], range[1])] + : range; + + return [ + [actualRange[0], range[0]], + [range[1], actualRange[1]] + ]; +} + +type CheckOptions = { destructuring: 'any' | 'all' }; +type VariableDeclaration = + | TSESTree.LetOrConstOrVarDeclaration + | TSESTree.UsingInForOfDeclaration + | TSESTree.UsingInNormalContextDeclaration; +type VariableDeclarator = + | TSESTree.LetOrConstOrVarDeclarator + | TSESTree.UsingInForOfDeclarator + | TSESTree.UsingInNomalConextDeclarator; +export class GroupChecker { + private reportCount = 0; + + private checkedId: TSESTree.BindingName | null = null; + + private checkedName = ''; + + private readonly shouldMatchAnyDestructuredVariable: boolean; + + private readonly context: RuleContext; + + public constructor(context: RuleContext, { destructuring }: CheckOptions) { + this.context = context; + this.shouldMatchAnyDestructuredVariable = destructuring !== 'all'; + } + + public checkAndReportNodes(nodes: ASTNode[]): void { + const shouldCheckGroup = nodes.length && this.shouldMatchAnyDestructuredVariable; + if (!shouldCheckGroup) { + return; + } + + const variableDeclarationParent = findUp( + nodes[0], + 'VariableDeclaration', + (parentNode: ASTNode) => parentNode.type.endsWith('Statement') + ); + const isVarDecParentNull = variableDeclarationParent === null; + const isValidDecParent = + !isVarDecParentNull && + 'declarations' in variableDeclarationParent && + variableDeclarationParent.declarations.length > 0; + if (!isValidDecParent) { + return; + } + + const dec = variableDeclarationParent.declarations[0]; + this.checkDeclarator(dec); + + const shouldFix = this.checkShouldFix(variableDeclarationParent, nodes.length); + if (!shouldFix) { + return; + } + + const sourceCode = getSourceCode(this.context); + nodes.filter(skipReactiveValues).forEach((node) => { + this.report(sourceCode, node, variableDeclarationParent); + }); + } + + private report( + sourceCode: ReturnType, + node: ASTNode, + nodeParent: VariableDeclaration + ) { + this.context.report({ + node, + messageId: 'useConst', + // @ts-expect-error Name will exist at this point + data: { name: node.name }, + fix: (fixer) => { + const letKeywordToken = sourceCode.getFirstToken(nodeParent, { + includeComments: false, + filter: (t) => 'kind' in nodeParent && t.value === nodeParent.kind + }); + if (!letKeywordToken) { + return null; + } + + const [initialRange, finalRange] = calculateRetainedTextRange( + letKeywordToken.range, + nodeParent.range + ); + const prefix = sourceCode.text.slice(...initialRange); + const suffix = sourceCode.text.slice(...finalRange); + + return fixer.replaceTextRange([initialRange[0], finalRange[1]], `${prefix}const${suffix}`); + } + }); + } + + private checkShouldFix(declaration: VariableDeclaration, totalNodes: number) { + const shouldFix = + declaration && + (declaration.parent.type === 'ForInStatement' || + declaration.parent.type === 'ForOfStatement' || + ('declarations' in declaration && + declaration.declarations.every((declaration) => declaration.init))); + + const totalDeclarationCount = this.checkDestructuredDeclaration(declaration, totalNodes); + if (totalDeclarationCount === -1) { + return shouldFix; + } + + return shouldFix && this.reportCount === totalDeclarationCount; + } + + private checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) { + const hasMultipleDeclarations = + declaration !== null && + 'declarations' in declaration && + declaration.declarations.length !== 1; + if (!hasMultipleDeclarations) { + return -1; + } + + const hasMoreThanOneDeclaration = + declaration && declaration.declarations && declaration.declarations.length >= 1; + if (!hasMoreThanOneDeclaration) { + return -1; + } + + this.reportCount += totalNodes; + + return declaration.declarations.reduce((total, declaration) => { + if (declaration.id.type === 'ObjectPattern') { + return total + declaration.id.properties.length; + } + + if (declaration.id.type === 'ArrayPattern') { + return total + declaration.id.elements.length; + } + + return total + 1; + }, 0); + } + + private checkDeclarator(declarator: VariableDeclarator) { + if (!declarator.init) { + return; + } + + const firstDecParent = declarator.init.parent; + if (firstDecParent.type !== 'VariableDeclarator') { + return; + } + + const { id } = firstDecParent; + if ('name' in id && id.name !== this.checkedName) { + this.checkedName = id.name; + this.reportCount = 0; + } + + if (firstDecParent.id.type === 'ObjectPattern') { + const { init } = firstDecParent; + if (init && 'name' in init && init.name !== this.checkedName) { + this.checkedName = init.name; + this.reportCount = 0; + } + } + + if (firstDecParent.id !== this.checkedId) { + this.checkedId = firstDecParent.id; + this.reportCount = 0; + } + } +} diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index 14055df26..4072aa9f6 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -1,279 +1,14 @@ -import type { Reference, Variable, Scope } from '@typescript-eslint/scope-manager'; -import type { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/types'; +import type { Variable } from '@typescript-eslint/scope-manager'; import { createRule } from '../utils'; -import type { RuleContext, RuleFixer, SourceCode } from '../types'; import { getSourceCode } from '../utils/compat'; -type ASTNode = TSESTree.Node; - -const PATTERN_TYPE = - /^(?:.+?Pattern|RestElement|SpreadProperty|ExperimentalRestProperty|Property)$/u; -const DECLARATION_HOST_TYPE = /^(?:Program|BlockStatement|StaticBlock|SwitchCase)$/u; -const DESTRUCTURING_HOST_TYPE = /^(?:VariableDeclarator|AssignmentExpression)$/u; - -type CheckedNode = NonNullable }>; - -function findIdentifierCallee(node: CheckedNode) { - const { parent } = node; - if (parent.type === 'VariableDeclarator' && parent.init?.type === 'CallExpression') { - return parent.init.callee; - } - - if ( - parent.type === 'Property' && - parent.parent.type === 'ObjectPattern' && - parent.parent.parent.type === 'VariableDeclarator' && - parent.parent.parent.init?.type === 'CallExpression' - ) { - return parent.parent.parent.init.callee; - } - - return null; -} - -/** - * Skip let statements when they are reactive values created by runes. - */ -function skipReactiveValues(identifier: ASTNode | null) { - if (!identifier || !identifier.parent) { - return false; - } - - const callee = findIdentifierCallee(identifier); - if (!callee) { - return true; - } - - if (callee.type === 'Identifier' && ['$state', '$props', '$derived'].includes(callee.name)) { - return false; - } - - if (callee.type !== 'MemberExpression' || callee.object.type !== 'Identifier') { - return true; - } - - if ( - callee.object.name === '$derived' && - callee.property.type === 'Identifier' && - callee.property.name === 'by' - ) { - return false; - } - if ( - callee.object.name === '$state' && - callee.property.type === 'Identifier' && - callee.property.name === 'frozen' - ) { - return false; - } - - return true; -} +import { GroupChecker, groupByDestructuring, isInitOfForStatement } from './prefer-const-helpers'; /** * Rule and helpers are copied from ESLint * https://github.com/eslint/eslint/blob/main/lib/rules/prefer-const.js */ -class FixTracker { - public fixer: RuleFixer; - - public sourceCode: SourceCode; - - public retainedRange: null | [number, number]; - - public constructor(fixer: RuleFixer, sourceCode: SourceCode) { - this.fixer = fixer; - this.sourceCode = sourceCode; - this.retainedRange = null; - } - - private getRetainedRange(range: [number, number]): [number, number] { - if (!this.retainedRange) { - return range; - } - - return [Math.min(this.retainedRange[0], range[0]), Math.max(this.retainedRange[1], range[1])]; - } - - public retainRange(range: [number, number]) { - this.retainedRange = range; - return this; - } - - public replaceTextRange(range: [number, number], text: string) { - const actualRange = this.getRetainedRange(range); - const actualText = - this.sourceCode.text.slice(actualRange[0], range[0]) + - text + - this.sourceCode.text.slice(range[1], actualRange[1]); - - return this.fixer.replaceTextRange(actualRange, actualText); - } -} - -type CheckOptions = { destructuring: 'any' | 'all' }; -type VariableDeclaration = - | TSESTree.LetOrConstOrVarDeclaration - | TSESTree.UsingInForOfDeclaration - | TSESTree.UsingInNormalContextDeclaration; -type VariableDeclarator = - | TSESTree.LetOrConstOrVarDeclarator - | TSESTree.UsingInForOfDeclarator - | TSESTree.UsingInNomalConextDeclarator; -class GroupChecker { - private reportCount = 0; - - private checkedId: TSESTree.BindingName | null = null; - - private checkedName = ''; - - private readonly shouldMatchAnyDestructuredVariable: boolean; - - private readonly context: RuleContext; - - public constructor(context: RuleContext, { destructuring }: CheckOptions) { - this.context = context; - this.shouldMatchAnyDestructuredVariable = destructuring !== 'all'; - } - - public checkAndReportNodes(nodes: ASTNode[]) { - const shouldCheckGroup = nodes.length && this.shouldMatchAnyDestructuredVariable; - if (!shouldCheckGroup) { - return; - } - - const variableDeclarationParent = findUp( - nodes[0], - 'VariableDeclaration', - (parentNode: ASTNode) => parentNode.type.endsWith('Statement') - ); - const isVarDecParentNull = variableDeclarationParent === null; - const isValidDecParent = - !isVarDecParentNull && - 'declarations' in variableDeclarationParent && - variableDeclarationParent.declarations.length > 0; - if (!isValidDecParent) { - return; - } - - const dec = variableDeclarationParent.declarations[0]; - this.checkDeclarator(dec); - - const shouldFix = this.checkShouldFix(variableDeclarationParent, nodes.length); - if (!shouldFix) { - return; - } - - const sourceCode = getSourceCode(this.context); - nodes.filter(skipReactiveValues).forEach((node) => { - this.report(sourceCode, node, variableDeclarationParent); - }); - } - - private report( - sourceCode: ReturnType, - node: ASTNode, - nodeParent: VariableDeclaration - ) { - this.context.report({ - node, - messageId: 'useConst', - // @ts-expect-error Name will exist at this point - data: { name: node.name }, - fix: (fixer) => { - const letKeywordToken = sourceCode.getFirstToken(nodeParent, { - includeComments: false, - filter: (t) => 'kind' in nodeParent && t.value === nodeParent.kind - }); - if (!letKeywordToken) { - return null; - } - - return new FixTracker(fixer, sourceCode) - .retainRange(nodeParent.range) - .replaceTextRange(letKeywordToken.range, 'const'); - } - }); - } - - private checkShouldFix(declaration: VariableDeclaration, totalNodes: number) { - const shouldFix = - declaration && - (declaration.parent.type === 'ForInStatement' || - declaration.parent.type === 'ForOfStatement' || - ('declarations' in declaration && - declaration.declarations.every((declaration) => declaration.init))); - - const totalDeclarationCount = this.checkDestructuredDeclaration(declaration, totalNodes); - if (totalDeclarationCount === -1) { - return shouldFix; - } - - return shouldFix && this.reportCount === totalDeclarationCount; - } - - private checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) { - const hasMultipleDeclarations = - declaration !== null && - 'declarations' in declaration && - declaration.declarations.length !== 1; - if (!hasMultipleDeclarations) { - return -1; - } - - const hasMoreThanOneDeclaration = - declaration && declaration.declarations && declaration.declarations.length >= 1; - if (!hasMoreThanOneDeclaration) { - return -1; - } - - this.reportCount += totalNodes; - - return declaration.declarations.reduce((total, declaration) => { - if (declaration.id.type === 'ObjectPattern') { - return total + declaration.id.properties.length; - } - - if (declaration.id.type === 'ArrayPattern') { - return total + declaration.id.elements.length; - } - - return total + 1; - }, 0); - } - - private checkDeclarator(declarator: VariableDeclarator) { - if (!declarator.init) { - return; - } - - const firstDecParent = declarator.init.parent; - if (firstDecParent.type !== 'VariableDeclarator') { - return; - } - - const { id } = firstDecParent; - if ('name' in id && id.name !== this.checkedName) { - this.checkedName = id.name; - this.reportCount = 0; - } - - if (firstDecParent.id.type === 'ObjectPattern') { - const { init } = firstDecParent; - if (init && 'name' in init && init.name !== this.checkedName) { - this.checkedName = init.name; - this.reportCount = 0; - } - } - - if (firstDecParent.id !== this.checkedId) { - this.checkedId = firstDecParent.id; - this.reportCount = 0; - } - } -} - export default createRule('prefer-const', { meta: { type: 'suggestion', @@ -323,270 +58,3 @@ export default createRule('prefer-const', { }; } }); - -/** - * Checks whether a given node is located at `ForStatement.init` or not. - */ -function isInitOfForStatement(node: ASTNode): boolean { - return Boolean(node.parent && node.parent.type === 'ForStatement' && node.parent.init === node); -} - -/** - * Checks whether a given Identifier node becomes a VariableDeclaration or not. - */ -function canBecomeVariableDeclaration(identifier: ASTNode): boolean { - let node = identifier.parent; - - while (node && PATTERN_TYPE.test(node.type)) { - node = node.parent; - } - if (!node) { - // TS check - return false; - } - - const isValidAssignmentExpr = - node.type === 'AssignmentExpression' && - node.parent.type === 'ExpressionStatement' && - DECLARATION_HOST_TYPE.test(node.parent.parent.type); - return node.type === 'VariableDeclarator' || isValidAssignmentExpr; -} - -/** - * Checks if an property or element is from outer scope or function parameters - * in destructing pattern. - */ -function isOuterVariableInDestructing(name: string, initScope: Scope): boolean { - if (initScope.through.some((ref) => ref.resolved && ref.resolved.name === name)) { - return true; - } - - const variable = getVariableByName(initScope, name); - return variable !== null ? variable.defs.some((def) => def.type === 'Parameter') : false; -} - -/** - * Gets the VariableDeclarator/AssignmentExpression node that a given reference - * belongs to. - * This is used to detect a mix of reassigned and never reassigned in a - * destructuring. - */ -function getDestructuringHost(reference: Reference): ASTNode | null { - if (!reference.isWrite()) { - return null; - } - - let node = reference.identifier.parent; - while (PATTERN_TYPE.test(node.type)) { - if (!node.parent) { - return null; - } - - node = node.parent; - } - - return !DESTRUCTURING_HOST_TYPE.test(node.type) ? null : node; -} - -/** - * Determines if a destructuring assignment node contains - * any MemberExpression nodes. This is used to determine if a - * variable that is only written once using destructuring can be - * safely converted into a const declaration. - */ -function hasMemberExpressionAssignment(node: ASTNode): boolean { - if (node.type === 'MemberExpression') { - return true; - } - if (node.type === 'ObjectPattern') { - return node.properties.some((prop) => - prop ? hasMemberExpressionAssignment('argument' in prop ? prop.argument : prop.value) : false - ); - } - if (node.type === 'ArrayPattern') { - return node.elements.some((element) => - element ? hasMemberExpressionAssignment(element) : false - ); - } - if (node.type === 'AssignmentPattern') { - return hasMemberExpressionAssignment(node.left); - } - - return false; -} - -function validateObjectPattern(node: TSESTree.ObjectPattern, variable: Variable) { - const properties = node.properties; - const hasOuterVariables = properties - .filter((prop) => prop.value) - .map((prop) => - 'value' in prop && prop.value - ? 'name' in prop.value - ? prop.value.name - : undefined - : undefined - ) - .some((name) => name && isOuterVariableInDestructing(name, variable.scope)); - const hasNonIdentifiers = hasMemberExpressionAssignment(node); - - return hasOuterVariables || hasNonIdentifiers; -} - -function validateArrayPattern(node: TSESTree.ArrayPattern, variable: Variable) { - const elements = node.elements; - const hasOuterVariables = elements - .map((element) => (element && 'name' in element ? element.name : undefined)) - .some((name) => name && isOuterVariableInDestructing(name, variable.scope)); - const hasNonIdentifiers = hasMemberExpressionAssignment(node); - - return hasOuterVariables || hasNonIdentifiers; -} - -/** - * Gets an identifier node of a given variable. - * - * If the initialization exists or one or more reading references exist before - * the first assignment, the identifier node is the node of the declaration. - * Otherwise, the identifier node is the node of the first assignment. - * - * If the variable should not change to const, this function returns null. - * - If the variable is reassigned. - * - If the variable is never initialized nor assigned. - * - If the variable is initialized in a different scope from the declaration. - * - If the unique assignment of the variable cannot change to a declaration. - * e.g. `if (a) b = 1` / `return (b = 1)` - * - If the variable is declared in the global scope and `eslintUsed` is `true`. - * `/*exported foo` directive comment makes such variables. This rule does not - * warn such variables because this rule cannot distinguish whether the - * exported variables are reassigned or not. - */ -function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign: boolean) { - if (variable.eslintUsed && variable.scope.type === 'global') { - return null; - } - - let writer = null; - let isReadBeforeInit = false; - const references = variable.references; - for (let i = 0; i < references.length; ++i) { - const reference = references[i]; - - if (reference.isWrite()) { - const isReassigned = writer !== null && writer.identifier !== reference.identifier; - if (isReassigned) { - return null; - } - - const destructuringHost = getDestructuringHost(reference); - const isValidDestructuringHost = - destructuringHost !== null && - 'left' in destructuringHost && - destructuringHost.left !== undefined; - - if (isValidDestructuringHost) { - const leftNode = destructuringHost.left; - - if (leftNode.type === 'ObjectPattern' && validateObjectPattern(leftNode, variable)) { - return null; - } else if (leftNode.type === 'ArrayPattern' && validateArrayPattern(leftNode, variable)) { - return null; - } - } - - writer = reference; - } else if (reference.isRead() && writer === null) { - if (ignoreReadBeforeAssign) { - return null; - } - isReadBeforeInit = true; - } - } - - const shouldBeConst = - writer !== null && - writer.from === variable.scope && - canBecomeVariableDeclaration(writer.identifier); - if (!shouldBeConst) { - return null; - } - if (isReadBeforeInit) { - return variable.defs[0].name; - } - - return writer?.identifier; -} - -/** - * Groups by the VariableDeclarator/AssignmentExpression node that each - * reference of given variables belongs to. - * This is used to detect a mix of reassigned and never reassigned in a - * destructuring. - */ -function groupByDestructuring( - variables: Variable[], - ignoreReadBeforeAssign: boolean -): Map { - const identifierMap = new Map(); - - for (let i = 0; i < variables.length; ++i) { - const variable = variables[i]; - const references = variable.references; - const identifier = getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign); - if (!identifier) { - continue; - } - - let prevId: TSESTree.Identifier | TSESTree.JSXIdentifier | null = null; - references.forEach((reference) => { - const id = reference.identifier; - if (id === prevId) { - return; - } - - prevId = id; - const group = getDestructuringHost(reference); - if (!group) { - return; - } - - if (identifierMap.has(group)) { - identifierMap.get(group)!.push(identifier); - } else { - identifierMap.set(group, [identifier]); - } - }); - } - - return identifierMap; -} - -/** - * Finds the nearest parent of node with a given type. - */ -function findUp(node: ASTNode, type: `${AST_NODE_TYPES}`, shouldStop: (node: ASTNode) => boolean) { - if (!node || shouldStop(node) || !node.parent) { - return null; - } - if (node.type === type) { - return node; - } - - return findUp(node.parent, type, shouldStop); -} - -/** - * Finds the variable by a given name in a given scope and its upper scopes. - */ -function getVariableByName(initScope: Scope, name: string): Variable | null { - let scope: Scope | null = initScope; - - while (scope) { - const variable = scope.set.get(name); - if (variable) { - return variable; - } - scope = scope.upper; - } - - return null; -} From 7d18e4e127625ce066502f27f278f696dfbfb1f6 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Fri, 29 Nov 2024 11:07:21 +0100 Subject: [PATCH 07/22] refactor(prefer-const): remove class in favour of function --- .../src/rules/prefer-const-helpers/index.ts | 159 +++++++++++------- .../src/rules/prefer-const.ts | 10 +- 2 files changed, 103 insertions(+), 66 deletions(-) diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts index f62b2d694..96fbc9858 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts @@ -6,12 +6,24 @@ import type { RuleContext } from '../../types'; type ASTNode = TSESTree.Node; type CheckedNode = NonNullable }>; +type VariableDeclaration = + | TSESTree.LetOrConstOrVarDeclaration + | TSESTree.UsingInForOfDeclaration + | TSESTree.UsingInNormalContextDeclaration; +type VariableDeclarator = + | TSESTree.LetOrConstOrVarDeclarator + | TSESTree.UsingInForOfDeclarator + | TSESTree.UsingInNomalConextDeclarator; const PATTERN_TYPE = /^(?:.+?Pattern|RestElement|SpreadProperty|ExperimentalRestProperty|Property)$/u; const DECLARATION_HOST_TYPE = /^(?:Program|BlockStatement|StaticBlock|SwitchCase)$/u; const DESTRUCTURING_HOST_TYPE = /^(?:VariableDeclarator|AssignmentExpression)$/u; +/** + * Finds the callee of a `CallExpression` if the given node is part of a + * `VariableDeclarator` or an ObjectPattern within a `VariableDeclarator`. + */ function findIdentifierCallee(node: CheckedNode) { const { parent } = node; if (parent.type === 'VariableDeclarator' && parent.init?.type === 'CallExpression') { @@ -70,7 +82,8 @@ function skipReactiveValues(identifier: ASTNode | null) { } /** - * Checks whether a given Identifier node becomes a VariableDeclaration or not. + * Checks whether a given `Identifier` node becomes a `VariableDeclaration` or + * not. */ function canBecomeVariableDeclaration(identifier: ASTNode) { let node = identifier.parent; @@ -91,8 +104,8 @@ function canBecomeVariableDeclaration(identifier: ASTNode) { } /** - * Checks if an property or element is from outer scope or export function parameters - * in destructing pattern. + * Checks if an property or element is from outer scope or export function + * parameters in destructing pattern. */ function isOuterVariableInDestructing(name: string, initScope: Scope) { if (initScope.through.some((ref) => ref.resolved && ref.resolved.name === name)) { @@ -104,10 +117,9 @@ function isOuterVariableInDestructing(name: string, initScope: Scope) { } /** - * Gets the VariableDeclarator/AssignmentExpression node that a given reference - * belongs to. - * This is used to detect a mix of reassigned and never reassigned in a - * destructuring. + * Gets the `VariableDeclarator/AssignmentExpression` node that a given + * reference belongs to. It is used to detect a mix of reassigned and never + * reassigned in a destructuring. */ function getDestructuringHost(reference: Reference) { if (!reference.isWrite()) { @@ -127,10 +139,10 @@ function getDestructuringHost(reference: Reference) { } /** - * Determines if a destructuring assignment node contains - * any MemberExpression nodes. This is used to determine if a - * variable that is only written once using destructuring can be - * safely converted into a const declaration. + * Determines if a destructuring assignment node contains any + * `MemberExpression` nodes. is used to determine if a variable that is only + * written once using destructuring can be safely converted into a const + * declaration. */ function hasMemberExpressionAssignment(node: ASTNode): boolean { if (node.type === 'MemberExpression') { @@ -153,6 +165,10 @@ function hasMemberExpressionAssignment(node: ASTNode): boolean { return false; } +/** + * Validates an object pattern to determine if it contains outer variables or + * non-identifier assignments. + */ function validateObjectPattern(node: TSESTree.ObjectPattern, variable: Variable) { const properties = node.properties; const hasOuterVariables = properties @@ -170,6 +186,10 @@ function validateObjectPattern(node: TSESTree.ObjectPattern, variable: Variable) return hasOuterVariables || hasNonIdentifiers; } +/** + * Validates an array pattern to determine if it contains outer variables or + * non-identifier elements. + */ function validateArrayPattern(node: TSESTree.ArrayPattern, variable: Variable): boolean { const elements = node.elements; const hasOuterVariables = elements @@ -187,15 +207,15 @@ function validateArrayPattern(node: TSESTree.ArrayPattern, variable: Variable): * the first assignment, the identifier node is the node of the declaration. * Otherwise, the identifier node is the node of the first assignment. * - * If the variable should not change to const, this export function returns null. + * If the variable should not change to const, export function returns null. * - If the variable is reassigned. * - If the variable is never initialized nor assigned. * - If the variable is initialized in a different scope from the declaration. * - If the unique assignment of the variable cannot change to a declaration. * e.g. `if (a) b = 1` / `return (b = 1)` * - If the variable is declared in the global scope and `eslintUsed` is `true`. - * `/*exported foo` directive comment makes such variables. This rule does not - * warn such variables because this rule cannot distinguish whether the + * `/*exported foo` directive comment makes such variables. rule does not + * warn such variables because rule cannot distinguish whether the * exported variables are reassigned or not. */ function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign: boolean) { @@ -247,6 +267,7 @@ function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign if (!shouldBeConst) { return null; } + if (isReadBeforeInit) { return variable.defs[0].name; } @@ -295,7 +316,7 @@ export function isInitOfForStatement(node: ASTNode): boolean { /** * Groups by the VariableDeclarator/AssignmentExpression node that each * reference of given variables belongs to. - * This is used to detect a mix of reassigned and never reassigned in a + * is used to detect a mix of reassigned and never reassigned in a * destructuring. */ export function groupByDestructuring( @@ -336,6 +357,13 @@ export function groupByDestructuring( return identifierMap; } +/** + * Calculates the retained text range by comparing the given range with an + * optional retained range. If a retained range is provided, it merges the two + * ranges to form an actual range. It then returns two ranges: one from the + * start of the actual range to the start of the given range, and another from + * the end of the given range to the end of the actual range. + */ function calculateRetainedTextRange( range: TSESTree.Range, retainedRange: TSESTree.Range | null @@ -350,33 +378,32 @@ function calculateRetainedTextRange( ]; } -type CheckOptions = { destructuring: 'any' | 'all' }; -type VariableDeclaration = - | TSESTree.LetOrConstOrVarDeclaration - | TSESTree.UsingInForOfDeclaration - | TSESTree.UsingInNormalContextDeclaration; -type VariableDeclarator = - | TSESTree.LetOrConstOrVarDeclarator - | TSESTree.UsingInForOfDeclarator - | TSESTree.UsingInNomalConextDeclarator; -export class GroupChecker { - private reportCount = 0; - - private checkedId: TSESTree.BindingName | null = null; - - private checkedName = ''; - - private readonly shouldMatchAnyDestructuredVariable: boolean; +type NodeReporterOptions = { destructuring: 'any' | 'all' }; +type NodeReporter = { report: (nodes: ASTNode[]) => void }; - private readonly context: RuleContext; - - public constructor(context: RuleContext, { destructuring }: CheckOptions) { - this.context = context; - this.shouldMatchAnyDestructuredVariable = destructuring !== 'all'; - } - - public checkAndReportNodes(nodes: ASTNode[]): void { - const shouldCheckGroup = nodes.length && this.shouldMatchAnyDestructuredVariable; +/** + * Creates a node reporter function that checks and reports nodes based on the + * provided context and options. + * + * @remarks The `report` function checks and reports nodes that should be + * converted to `const` declarations. It performs various checks to ensure that + * the nodes meet the criteria for reporting and fixing. + * + * The function also handles cases where variables are declared using + * destructuring patterns and ensures that the correct number of nodes are + * reported and fixed. + */ +export function createNodeReporter( + context: RuleContext, + { destructuring }: NodeReporterOptions +): NodeReporter { + let reportCount = 0; + let checkedId: TSESTree.BindingName | null = null; + let checkedName = ''; + const shouldMatchAnyDestructuredVariable = destructuring !== 'all'; + + function checkAndReportNodes(nodes: ASTNode[]): void { + const shouldCheckGroup = nodes.length && shouldMatchAnyDestructuredVariable; if (!shouldCheckGroup) { return; } @@ -396,28 +423,28 @@ export class GroupChecker { } const dec = variableDeclarationParent.declarations[0]; - this.checkDeclarator(dec); + checkDeclarator(dec); - const shouldFix = this.checkShouldFix(variableDeclarationParent, nodes.length); + const shouldFix = checkShouldFix(variableDeclarationParent, nodes.length); if (!shouldFix) { return; } - const sourceCode = getSourceCode(this.context); + const sourceCode = getSourceCode(context); nodes.filter(skipReactiveValues).forEach((node) => { - this.report(sourceCode, node, variableDeclarationParent); + report(sourceCode, node, variableDeclarationParent); }); } - private report( + function report( sourceCode: ReturnType, node: ASTNode, nodeParent: VariableDeclaration ) { - this.context.report({ + context.report({ node, messageId: 'useConst', - // @ts-expect-error Name will exist at this point + // @ts-expect-error Name will exist at point data: { name: node.name }, fix: (fixer) => { const letKeywordToken = sourceCode.getFirstToken(nodeParent, { @@ -440,7 +467,7 @@ export class GroupChecker { }); } - private checkShouldFix(declaration: VariableDeclaration, totalNodes: number) { + function checkShouldFix(declaration: VariableDeclaration, totalNodes: number) { const shouldFix = declaration && (declaration.parent.type === 'ForInStatement' || @@ -448,15 +475,15 @@ export class GroupChecker { ('declarations' in declaration && declaration.declarations.every((declaration) => declaration.init))); - const totalDeclarationCount = this.checkDestructuredDeclaration(declaration, totalNodes); + const totalDeclarationCount = checkDestructuredDeclaration(declaration, totalNodes); if (totalDeclarationCount === -1) { return shouldFix; } - return shouldFix && this.reportCount === totalDeclarationCount; + return shouldFix && reportCount === totalDeclarationCount; } - private checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) { + function checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) { const hasMultipleDeclarations = declaration !== null && 'declarations' in declaration && @@ -471,7 +498,7 @@ export class GroupChecker { return -1; } - this.reportCount += totalNodes; + reportCount += totalNodes; return declaration.declarations.reduce((total, declaration) => { if (declaration.id.type === 'ObjectPattern') { @@ -486,7 +513,7 @@ export class GroupChecker { }, 0); } - private checkDeclarator(declarator: VariableDeclarator) { + function checkDeclarator(declarator: VariableDeclarator) { if (!declarator.init) { return; } @@ -497,22 +524,28 @@ export class GroupChecker { } const { id } = firstDecParent; - if ('name' in id && id.name !== this.checkedName) { - this.checkedName = id.name; - this.reportCount = 0; + if ('name' in id && id.name !== checkedName) { + checkedName = id.name; + reportCount = 0; } if (firstDecParent.id.type === 'ObjectPattern') { const { init } = firstDecParent; - if (init && 'name' in init && init.name !== this.checkedName) { - this.checkedName = init.name; - this.reportCount = 0; + if (init && 'name' in init && init.name !== checkedName) { + checkedName = init.name; + reportCount = 0; } } - if (firstDecParent.id !== this.checkedId) { - this.checkedId = firstDecParent.id; - this.reportCount = 0; + if (firstDecParent.id !== checkedId) { + checkedId = firstDecParent.id; + reportCount = 0; } } + + return { + report(group) { + checkAndReportNodes(group); + } + }; } diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index 4072aa9f6..e355b733c 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -2,7 +2,11 @@ import type { Variable } from '@typescript-eslint/scope-manager'; import { createRule } from '../utils'; import { getSourceCode } from '../utils/compat'; -import { GroupChecker, groupByDestructuring, isInitOfForStatement } from './prefer-const-helpers'; +import { + createNodeReporter, + groupByDestructuring, + isInitOfForStatement +} from './prefer-const-helpers'; /** * Rule and helpers are copied from ESLint @@ -43,11 +47,11 @@ export default createRule('prefer-const', { return { 'Program:exit'() { - const checker = new GroupChecker(context, { + const nodeReporter = createNodeReporter(context, { destructuring: options.destructuring }); groupByDestructuring(variables, ignoreReadBeforeAssign).forEach((group) => { - checker.checkAndReportNodes(group); + nodeReporter.report(group); }); }, VariableDeclaration(node) { From 6b1d01bb22be85f265647edb1ccadfe738a05ed1 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Fri, 29 Nov 2024 11:09:48 +0100 Subject: [PATCH 08/22] docs(prefer-const): update attribution for rule and helpers from ESLint --- .../src/rules/prefer-const-helpers/index.ts | 6 ++++++ packages/eslint-plugin-svelte/src/rules/prefer-const.ts | 5 ----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts index 96fbc9858..a073e2236 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts @@ -1,3 +1,9 @@ +/** + * Atributions: + * Rule and helpers are adapted from ESLint + * https://github.com/eslint/eslint/blob/main/lib/rules/prefer-const.js + */ + import type { Reference, Variable, Scope } from '@typescript-eslint/scope-manager'; import type { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/types'; diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index e355b733c..91944fd11 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -8,11 +8,6 @@ import { isInitOfForStatement } from './prefer-const-helpers'; -/** - * Rule and helpers are copied from ESLint - * https://github.com/eslint/eslint/blob/main/lib/rules/prefer-const.js - */ - export default createRule('prefer-const', { meta: { type: 'suggestion', From 308daba3e7eb8037cccc22a0568c893c4c8a6b8d Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Fri, 29 Nov 2024 11:14:10 +0100 Subject: [PATCH 09/22] chore(prefer-const): fix docs --- docs/rules/prefer-const.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/rules/prefer-const.md b/docs/rules/prefer-const.md index 18eb96cc8..d043f3cf0 100644 --- a/docs/rules/prefer-const.md +++ b/docs/rules/prefer-const.md @@ -24,12 +24,11 @@ This rule reports the same as the base ESLint `prefer-const` rule, except that i From 6c6255d1d719cbc64119ef0101146f38ebf78e04 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Fri, 29 Nov 2024 17:23:52 +0100 Subject: [PATCH 10/22] fix(prefer-const): typo --- .../src/rules/prefer-const-helpers/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts index a073e2236..8d125ccf4 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts @@ -19,7 +19,7 @@ type VariableDeclaration = type VariableDeclarator = | TSESTree.LetOrConstOrVarDeclarator | TSESTree.UsingInForOfDeclarator - | TSESTree.UsingInNomalConextDeclarator; + | TSESTree.UsingInNormalContextDeclarator; const PATTERN_TYPE = /^(?:.+?Pattern|RestElement|SpreadProperty|ExperimentalRestProperty|Property)$/u; From a5e74bdb3b8a4fda347f32338be7d6ed1a9c95c5 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Fri, 29 Nov 2024 17:39:42 +0100 Subject: [PATCH 11/22] chore(prefer-const): run `pnpm run update` --- .../eslint-plugin-svelte/src/types-for-node.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin-svelte/src/types-for-node.ts b/packages/eslint-plugin-svelte/src/types-for-node.ts index 866ea6dab..a1bf991a5 100644 --- a/packages/eslint-plugin-svelte/src/types-for-node.ts +++ b/packages/eslint-plugin-svelte/src/types-for-node.ts @@ -135,6 +135,7 @@ export type ASTNodeListener = { TSEmptyBodyFunctionExpression?: ( node: TSESTree.TSEmptyBodyFunctionExpression & ASTNodeWithParent ) => void; + TSEnumBody?: (node: TSESTree.TSEnumBody & ASTNodeWithParent) => void; TSEnumDeclaration?: (node: TSESTree.TSEnumDeclaration & ASTNodeWithParent) => void; TSEnumMember?: (node: TSESTree.TSEnumMember & ASTNodeWithParent) => void; TSExportAssignment?: (node: TSESTree.TSExportAssignment & ASTNodeWithParent) => void; @@ -143,9 +144,6 @@ export type ASTNodeListener = { node: TSESTree.TSExternalModuleReference & ASTNodeWithParent ) => void; TSFunctionType?: (node: TSESTree.TSFunctionType & ASTNodeWithParent) => void; - TSInstantiationExpression?: ( - node: TSESTree.TSInstantiationExpression & ASTNodeWithParent - ) => void; TSImportEqualsDeclaration?: ( node: TSESTree.TSImportEqualsDeclaration & ASTNodeWithParent ) => void; @@ -153,6 +151,9 @@ export type ASTNodeListener = { TSIndexedAccessType?: (node: TSESTree.TSIndexedAccessType & ASTNodeWithParent) => void; TSIndexSignature?: (node: TSESTree.TSIndexSignature & ASTNodeWithParent) => void; TSInferType?: (node: TSESTree.TSInferType & ASTNodeWithParent) => void; + TSInstantiationExpression?: ( + node: TSESTree.TSInstantiationExpression & ASTNodeWithParent + ) => void; TSInterfaceBody?: (node: TSESTree.TSInterfaceBody & ASTNodeWithParent) => void; TSInterfaceDeclaration?: (node: TSESTree.TSInterfaceDeclaration & ASTNodeWithParent) => void; TSInterfaceHeritage?: (node: TSESTree.TSInterfaceHeritage & ASTNodeWithParent) => void; @@ -354,6 +355,7 @@ export type TSNodeListener = { TSEmptyBodyFunctionExpression?: ( node: TSESTree.TSEmptyBodyFunctionExpression & ASTNodeWithParent ) => void; + TSEnumBody?: (node: TSESTree.TSEnumBody & ASTNodeWithParent) => void; TSEnumDeclaration?: (node: TSESTree.TSEnumDeclaration & ASTNodeWithParent) => void; TSEnumMember?: (node: TSESTree.TSEnumMember & ASTNodeWithParent) => void; TSExportAssignment?: (node: TSESTree.TSExportAssignment & ASTNodeWithParent) => void; @@ -362,9 +364,6 @@ export type TSNodeListener = { node: TSESTree.TSExternalModuleReference & ASTNodeWithParent ) => void; TSFunctionType?: (node: TSESTree.TSFunctionType & ASTNodeWithParent) => void; - TSInstantiationExpression?: ( - node: TSESTree.TSInstantiationExpression & ASTNodeWithParent - ) => void; TSImportEqualsDeclaration?: ( node: TSESTree.TSImportEqualsDeclaration & ASTNodeWithParent ) => void; @@ -372,6 +371,9 @@ export type TSNodeListener = { TSIndexedAccessType?: (node: TSESTree.TSIndexedAccessType & ASTNodeWithParent) => void; TSIndexSignature?: (node: TSESTree.TSIndexSignature & ASTNodeWithParent) => void; TSInferType?: (node: TSESTree.TSInferType & ASTNodeWithParent) => void; + TSInstantiationExpression?: ( + node: TSESTree.TSInstantiationExpression & ASTNodeWithParent + ) => void; TSInterfaceBody?: (node: TSESTree.TSInterfaceBody & ASTNodeWithParent) => void; TSInterfaceDeclaration?: (node: TSESTree.TSInterfaceDeclaration & ASTNodeWithParent) => void; TSInterfaceHeritage?: (node: TSESTree.TSInterfaceHeritage & ASTNodeWithParent) => void; From c69f65663c69edcb6dd2e0f4c780dd6079b235f5 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Sat, 30 Nov 2024 08:42:36 +0100 Subject: [PATCH 12/22] test(prefer-const): add original rule tests & fix code By adding the original rule tests, I was able to identify issues with the rule, that had been caused because of the refactoring. Now, not only I think the code is more readable and maintainable, but also the behavior is as expected, and there are tests to ensure that. There's a tests that has been skipped since it reporting a false positive, which is dues to `@typescript-eslint`. I've reported the issue in https://github.com/typescript-eslint/typescript-eslint/issues/10426 --- .../src/rules/prefer-const-helpers/index.ts | 119 +-- .../tests/src/rules/prefer-const.ts | 731 +++++++++++++++++- 2 files changed, 792 insertions(+), 58 deletions(-) diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts index 8d125ccf4..d979b8b71 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts @@ -135,7 +135,7 @@ function getDestructuringHost(reference: Reference) { let node = reference.identifier.parent; while (PATTERN_TYPE.test(node.type)) { if (!node.parent) { - return null; + break; } node = node.parent; @@ -278,7 +278,7 @@ function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign return variable.defs[0].name; } - return writer?.identifier; + return writer?.identifier ?? null; } /** @@ -328,16 +328,13 @@ export function isInitOfForStatement(node: ASTNode): boolean { export function groupByDestructuring( variables: Variable[], ignoreReadBeforeAssign: boolean -): Map { - const identifierMap = new Map(); +): Map { + const identifierMap = new Map(); for (let i = 0; i < variables.length; ++i) { const variable = variables[i]; const references = variable.references; const identifier = getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign); - if (!identifier) { - continue; - } let prevId: TSESTree.Identifier | TSESTree.JSXIdentifier | null = null; references.forEach((reference) => { @@ -385,7 +382,7 @@ function calculateRetainedTextRange( } type NodeReporterOptions = { destructuring: 'any' | 'all' }; -type NodeReporter = { report: (nodes: ASTNode[]) => void }; +type NodeReporter = { report: (nodes: (ASTNode | null)[]) => void }; /** * Creates a node reporter function that checks and reports nodes based on the @@ -408,8 +405,10 @@ export function createNodeReporter( let checkedName = ''; const shouldMatchAnyDestructuredVariable = destructuring !== 'all'; - function checkAndReportNodes(nodes: ASTNode[]): void { - const shouldCheckGroup = nodes.length && shouldMatchAnyDestructuredVariable; + function checkAndReportNodes(initialNodes: (ASTNode | null)[]): void { + const nodes = initialNodes.filter(Boolean) as ASTNode[]; + const shouldCheckGroup = + nodes.length && (shouldMatchAnyDestructuredVariable || initialNodes.length === nodes.length); if (!shouldCheckGroup) { return; } @@ -424,62 +423,70 @@ export function createNodeReporter( !isVarDecParentNull && 'declarations' in variableDeclarationParent && variableDeclarationParent.declarations.length > 0; - if (!isValidDecParent) { - return; - } - - const dec = variableDeclarationParent.declarations[0]; - checkDeclarator(dec); - - const shouldFix = checkShouldFix(variableDeclarationParent, nodes.length); - if (!shouldFix) { - return; + if (isValidDecParent) { + checkDeclarator(variableDeclarationParent.declarations[0]); } + const shouldFix = + isValidDecParent && + checkShouldFix(variableDeclarationParent, nodes.length, initialNodes.length); const sourceCode = getSourceCode(context); nodes.filter(skipReactiveValues).forEach((node) => { - report(sourceCode, node, variableDeclarationParent); + report(sourceCode, node, variableDeclarationParent!, shouldFix); }); } function report( sourceCode: ReturnType, node: ASTNode, - nodeParent: VariableDeclaration + nodeParent: ASTNode, + shouldFix: boolean ) { context.report({ node, messageId: 'useConst', // @ts-expect-error Name will exist at point data: { name: node.name }, - fix: (fixer) => { - const letKeywordToken = sourceCode.getFirstToken(nodeParent, { - includeComments: false, - filter: (t) => 'kind' in nodeParent && t.value === nodeParent.kind - }); - if (!letKeywordToken) { - return null; - } - - const [initialRange, finalRange] = calculateRetainedTextRange( - letKeywordToken.range, - nodeParent.range - ); - const prefix = sourceCode.text.slice(...initialRange); - const suffix = sourceCode.text.slice(...finalRange); - - return fixer.replaceTextRange([initialRange[0], finalRange[1]], `${prefix}const${suffix}`); - } + fix: !shouldFix + ? null + : (fixer) => { + const letKeywordToken = sourceCode.getFirstToken(nodeParent, { + includeComments: false, + filter: (t) => 'kind' in nodeParent && t.value === nodeParent.kind + }); + if (!letKeywordToken) { + return null; + } + + const [initialRange, finalRange] = calculateRetainedTextRange( + letKeywordToken.range, + nodeParent.range + ); + const prefix = sourceCode.text.slice(...initialRange); + const suffix = sourceCode.text.slice(...finalRange); + + return fixer.replaceTextRange( + [initialRange[0], finalRange[1]], + `${prefix}const${suffix}` + ); + } }); } - function checkShouldFix(declaration: VariableDeclaration, totalNodes: number) { + function checkShouldFix( + declaration: VariableDeclaration, + totalNodes: number, + totalInitialNodes: number + ) { + const isParentForStatement = + declaration?.parent.type === 'ForInStatement' || + declaration?.parent.type === 'ForOfStatement'; + const areAllDeclarationsInitialized = + 'declarations' in declaration && declaration.declarations.every((decl) => decl.init); const shouldFix = + totalNodes === totalInitialNodes && declaration && - (declaration.parent.type === 'ForInStatement' || - declaration.parent.type === 'ForOfStatement' || - ('declarations' in declaration && - declaration.declarations.every((declaration) => declaration.init))); + (isParentForStatement || areAllDeclarationsInitialized); const totalDeclarationCount = checkDestructuredDeclaration(declaration, totalNodes); if (totalDeclarationCount === -1) { @@ -490,16 +497,14 @@ export function createNodeReporter( } function checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) { - const hasMultipleDeclarations = - declaration !== null && - 'declarations' in declaration && - declaration.declarations.length !== 1; + const isValidDeclaration = declaration !== null && 'declarations' in declaration; + const hasMultipleDeclarations = isValidDeclaration && declaration.declarations.length !== 1; if (!hasMultipleDeclarations) { return -1; } const hasMoreThanOneDeclaration = - declaration && declaration.declarations && declaration.declarations.length >= 1; + declaration.declarations && declaration.declarations.length >= 1; if (!hasMoreThanOneDeclaration) { return -1; } @@ -524,27 +529,27 @@ export function createNodeReporter( return; } - const firstDecParent = declarator.init.parent; - if (firstDecParent.type !== 'VariableDeclarator') { + const firstDeclaratorParent = declarator.init.parent; + if (firstDeclaratorParent.type !== 'VariableDeclarator') { return; } - const { id } = firstDecParent; + const { id } = firstDeclaratorParent; if ('name' in id && id.name !== checkedName) { checkedName = id.name; reportCount = 0; } - if (firstDecParent.id.type === 'ObjectPattern') { - const { init } = firstDecParent; + if (firstDeclaratorParent.id.type === 'ObjectPattern') { + const { init } = firstDeclaratorParent; if (init && 'name' in init && init.name !== checkedName) { checkedName = init.name; reportCount = 0; } } - if (firstDecParent.id !== checkedId) { - checkedId = firstDecParent.id; + if (firstDeclaratorParent.id !== checkedId) { + checkedId = firstDeclaratorParent.id; reportCount = 0; } } diff --git a/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts index ed8e50445..d7f62a832 100644 --- a/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts @@ -2,11 +2,740 @@ import { RuleTester } from '../../utils/eslint-compat'; import rule from '../../../src/rules/prefer-const'; import { loadTestCases } from '../../utils/utils'; +type ValidTestCases = ReturnType['valid']; +type InvalidTestCases = ReturnType['invalid']; + const tester = new RuleTester({ languageOptions: { ecmaVersion: 2020, sourceType: 'module' + }, + plugins: { + custom: { + rules: { + 'use-x': { + create(context) { + const sourceCode = context.sourceCode; + + return { + VariableDeclaration(node) { + sourceCode.markVariableAsUsed('x', node); + } + }; + } + } + } + } } }); -tester.run('prefer-const', rule as any, loadTestCases('prefer-const')); +const loadedCases = loadTestCases('prefer-const'); +// This tests cases have been imported/adapted from eslint/prefer-const rule +const ESLINT_RULE_TEST_CASES = { + valid: [ + 'var x = 0;', + 'let x;', + 'let x; { x = 0; } foo(x);', + 'let x = 0; x = 1;', + 'const x = 0;', + 'for (let i = 0, end = 10; i < end; ++i) {}', + 'for (let i in [1,2,3]) { i = 0; }', + 'for (let x of [1,2,3]) { x = 0; }', + '(function() { var x = 0; })();', + '(function() { let x; })();', + '(function() { let x; { x = 0; } foo(x); })();', + '(function() { let x = 0; x = 1; })();', + '(function() { const x = 0; })();', + '(function() { for (let i = 0, end = 10; i < end; ++i) {} })();', + '(function() { for (let i in [1,2,3]) { i = 0; } })();', + '(function() { for (let x of [1,2,3]) { x = 0; } })();', + '(function(x = 0) { })();', + 'let a; while (a = foo());', + 'let a; do {} while (a = foo());', + 'let a; for (; a = foo(); );', + 'let a; for (;; ++a);', + 'let a; for (const {b = ++a} in foo());', + 'let a; for (const {b = ++a} of foo());', + 'let a; for (const x of [1,2,3]) { if (a) {} a = foo(); }', + 'let a; for (const x of [1,2,3]) { a = a || foo(); bar(a); }', + 'let a; for (const x of [1,2,3]) { foo(++a); }', + 'let a; function foo() { if (a) {} a = bar(); }', + 'let a; function foo() { a = a || bar(); baz(a); }', + 'let a; function foo() { bar(++a); }', + [ + 'let id;', + 'function foo() {', + " if (typeof id !== 'undefined') {", + ' return;', + ' }', + ' id = setInterval(() => {}, 250);', + '}', + 'foo();' + ].join('\n'), + '/*exported a*/ let a; function init() { a = foo(); }', + // TODO: Skipping this test until https://github.com/typescript-eslint/typescript-eslint/issues/10426 + // '/*exported a*/ let a = 1', + 'let a; if (true) a = 0; foo(a);', + ` + (function (a) { + let b; + ({ a, b } = obj); + })(); + `, + ` + (function (a) { + let b; + ([ a, b ] = obj); + })(); + `, + 'var a; { var b; ({ a, b } = obj); }', + 'let a; { let b; ({ a, b } = obj); }', + 'var a; { var b; ([ a, b ] = obj); }', + 'let a; { let b; ([ a, b ] = obj); }', + + /* + * The assignment is located in a different scope. + * Those are warned by prefer-smaller-scope. + */ + 'let x; { x = 0; foo(x); }', + '(function() { let x; { x = 0; foo(x); } })();', + 'let x; for (const a of [1,2,3]) { x = foo(); bar(x); }', + '(function() { let x; for (const a of [1,2,3]) { x = foo(); bar(x); } })();', + 'let x; for (x of array) { x; }', + { + code: 'let {a, b} = obj; b = 0;', + options: [{ destructuring: 'all' }] + }, + { + code: 'let a, b; ({a, b} = obj); b++;', + options: [{ destructuring: 'all' }] + }, + // https://github.com/eslint/eslint/issues/8187 + { + code: 'let { name, ...otherStuff } = obj; otherStuff = {};', + options: [{ destructuring: 'all' }], + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let { name, ...otherStuff } = obj; otherStuff = {};', + options: [{ destructuring: 'all' }] + }, + // https://github.com/eslint/eslint/issues/8308 + { + code: 'let predicate; [typeNode.returnType, predicate] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let predicate; [typeNode.returnType, ...predicate] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + // intentionally testing empty slot in destructuring assignment + code: 'let predicate; [typeNode.returnType,, predicate] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let predicate; [typeNode.returnType=5, predicate] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let predicate; [[typeNode.returnType=5], predicate] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let predicate; [[typeNode.returnType, predicate]] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let predicate; [typeNode.returnType, [predicate]] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let predicate; [, [typeNode.returnType, predicate]] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let predicate; [, {foo:typeNode.returnType, predicate}] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let predicate; [, {foo:typeNode.returnType, ...predicate}] = foo();', + languageOptions: { ecmaVersion: 2018 } + }, + { + code: 'let a; const b = {}; ({ a, c: b.c } = func());', + languageOptions: { ecmaVersion: 2018 } + }, + // ignoreReadBeforeAssign + { + code: 'let x; function foo() { bar(x); } x = 0;', + options: [{ ignoreReadBeforeAssign: true }] + }, + // https://github.com/eslint/eslint/issues/10520 + 'const x = [1,2]; let y; [,y] = x; y = 0;', + 'const x = [1,2,3]; let y, z; [y,,z] = x; y = 0; z = 0;', + { + code: 'class C { static { let a = 1; a = 2; } }', + languageOptions: { ecmaVersion: 2022 } + }, + { + code: 'class C { static { let a; a = 1; a = 2; } }', + languageOptions: { ecmaVersion: 2022 } + }, + { + code: 'let a; class C { static { a = 1; } }', + languageOptions: { ecmaVersion: 2022 } + }, + { + code: 'class C { static { let a; if (foo) { a = 1; } } }', + languageOptions: { ecmaVersion: 2022 } + }, + { + code: 'class C { static { let a; if (foo) a = 1; } }', + languageOptions: { ecmaVersion: 2022 } + }, + { + code: 'class C { static { let a, b; if (foo) { ({ a, b } = foo); } } }', + output: null, + languageOptions: { ecmaVersion: 2022 }, + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } + ] + }, + { + code: 'class C { static { let a, b; if (foo) ({ a, b } = foo); } }', + output: null, + languageOptions: { ecmaVersion: 2022 }, + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } + ] + }, + { + code: 'class C { static { a; } } let a = 1; ', + options: [{ ignoreReadBeforeAssign: true }], + languageOptions: { ecmaVersion: 2022 } + }, + { + code: 'class C { static { () => a; let a = 1; } };', + options: [{ ignoreReadBeforeAssign: true }], + languageOptions: { ecmaVersion: 2022 } + } + ] as ValidTestCases, + invalid: [ + { + code: 'let x = 1; foo(x);', + output: 'const x = 1; foo(x);', + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: 'for (let i in [1,2,3]) { foo(i); }', + output: 'for (const i in [1,2,3]) { foo(i); }', + errors: [{ messageId: 'useConst', data: { name: 'i' }, type: 'Identifier' }] + }, + { + code: 'for (let x of [1,2,3]) { foo(x); }', + output: 'for (const x of [1,2,3]) { foo(x); }', + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: 'let [x = -1, y] = [1,2]; y = 0;', + output: null, + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: 'let {a: x = -1, b: y} = {a:1,b:2}; y = 0;', + output: null, + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: '(function() { let x = 1; foo(x); })();', + output: '(function() { const x = 1; foo(x); })();', + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: '(function() { for (let i in [1,2,3]) { foo(i); } })();', + output: '(function() { for (const i in [1,2,3]) { foo(i); } })();', + errors: [{ messageId: 'useConst', data: { name: 'i' }, type: 'Identifier' }] + }, + { + code: '(function() { for (let x of [1,2,3]) { foo(x); } })();', + output: '(function() { for (const x of [1,2,3]) { foo(x); } })();', + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: '(function() { let [x = -1, y] = [1,2]; y = 0; })();', + output: null, + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: 'let f = (function() { let g = x; })(); f = 1;', + output: 'let f = (function() { const g = x; })(); f = 1;', + errors: [{ messageId: 'useConst', data: { name: 'g' }, type: 'Identifier' }] + }, + { + code: '(function() { let {a: x = -1, b: y} = {a:1,b:2}; y = 0; })();', + output: null, + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: 'let x = 0; { let x = 1; foo(x); } x = 0;', + output: 'let x = 0; { const x = 1; foo(x); } x = 0;', + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: 'for (let i = 0; i < 10; ++i) { let x = 1; foo(x); }', + output: 'for (let i = 0; i < 10; ++i) { const x = 1; foo(x); }', + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: 'for (let i in [1,2,3]) { let x = 1; foo(x); }', + output: 'for (const i in [1,2,3]) { const x = 1; foo(x); }', + errors: [ + { messageId: 'useConst', data: { name: 'i' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' } + ] + }, + { + code: [ + 'var foo = function() {', + ' for (const b of c) {', + ' let a;', + ' a = 1;', + ' }', + '};' + ].join('\n'), + output: null, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: [ + 'var foo = function() {', + ' for (const b of c) {', + ' let a;', + ' ({a} = 1);', + ' }', + '};' + ].join('\n'), + output: null, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'let x; x = 0;', + output: null, + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier', column: 8 }] + }, + { + code: 'switch (a) { case 0: let x; x = 0; }', + output: null, + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier', column: 29 }] + }, + { + code: '(function() { let x; x = 1; })();', + output: null, + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier', column: 22 }] + }, + + { + code: 'let {a = 0, b} = obj; b = 0; foo(a, b);', + output: null, + options: [{ destructuring: 'any' }], + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'let {a: {b, c}} = {a: {b: 1, c: 2}}; b = 3;', + output: null, + options: [{ destructuring: 'any' }], + errors: [{ messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' }] + }, + { + code: 'let {a: {b, c}} = {a: {b: 1, c: 2}}', + output: 'const {a: {b, c}} = {a: {b: 1, c: 2}}', + options: [{ destructuring: 'all' }], + errors: [ + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' } + ] + }, + { + code: 'let a, b; ({a = 0, b} = obj); b = 0; foo(a, b);', + output: null, + options: [{ destructuring: 'any' }], + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'let {a = 0, b} = obj; foo(a, b);', + output: 'const {a = 0, b} = obj; foo(a, b);', + options: [{ destructuring: 'all' }], + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } + ] + }, + { + code: 'let [a] = [1]', + output: 'const [a] = [1]', + options: [], + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'let {a} = obj', + output: 'const {a} = obj', + options: [], + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'let a, b; ({a = 0, b} = obj); foo(a, b);', + output: null, + options: [{ destructuring: 'all' }], + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } + ] + }, + { + code: 'let {a = 0, b} = obj, c = a; b = a;', + output: null, + options: [{ destructuring: 'any' }], + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' } + ] + }, + { + code: 'let {a = 0, b} = obj, c = a; b = a;', + output: null, + options: [{ destructuring: 'all' }], + errors: [{ messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' }] + }, + + // https://github.com/eslint/eslint/issues/8187 + { + code: 'let { name, ...otherStuff } = obj; otherStuff = {};', + output: null, + options: [{ destructuring: 'any' }], + languageOptions: { ecmaVersion: 2018 }, + errors: [{ messageId: 'useConst', data: { name: 'name' }, type: 'Identifier', column: 7 }] + }, + { + code: 'let { name, ...otherStuff } = obj; otherStuff = {};', + output: null, + options: [{ destructuring: 'any' }], + errors: [{ messageId: 'useConst', data: { name: 'name' }, type: 'Identifier', column: 7 }] + }, + // Warnings are located at declaration if there are reading references before assignments. + { + code: 'let x; function foo() { bar(x); } x = 0;', + output: null, + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier', column: 5 }] + }, + // https://github.com/eslint/eslint/issues/5837 + { + code: '/*eslint custom/use-x:error*/ let x = 1', + output: '/*eslint custom/use-x:error*/ const x = 1', + languageOptions: { parserOptions: { ecmaFeatures: { globalReturn: true } } }, + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: '/*eslint custom/use-x:error*/ { let x = 1 }', + output: '/*eslint custom/use-x:error*/ { const x = 1 }', + errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] + }, + { + code: 'let { foo, bar } = baz;', + output: 'const { foo, bar } = baz;', + errors: [ + { messageId: 'useConst', data: { name: 'foo' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'bar' }, type: 'Identifier' } + ] + }, + // https://github.com/eslint/eslint/issues/10520 + { + code: 'const x = [1,2]; let [,y] = x;', + output: 'const x = [1,2]; const [,y] = x;', + errors: [{ messageId: 'useConst', data: { name: 'y' }, type: 'Identifier' }] + }, + { + code: 'const x = [1,2,3]; let [y,,z] = x;', + output: 'const x = [1,2,3]; const [y,,z] = x;', + errors: [ + { messageId: 'useConst', data: { name: 'y' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'z' }, type: 'Identifier' } + ] + }, + // https://github.com/eslint/eslint/issues/8308 + { + code: 'let predicate; [, {foo:returnType, predicate}] = foo();', + output: null, + languageOptions: { ecmaVersion: 2018 }, + errors: [ + { message: "'predicate' is never reassigned. Use 'const' instead.", type: 'Identifier' } + ] + }, + { + code: 'let predicate; [, {foo:returnType, predicate}, ...bar ] = foo();', + output: null, + languageOptions: { ecmaVersion: 2018 }, + errors: [ + { message: "'predicate' is never reassigned. Use 'const' instead.", type: 'Identifier' } + ] + }, + { + code: 'let predicate; [, {foo:returnType, ...predicate} ] = foo();', + output: null, + languageOptions: { ecmaVersion: 2018 }, + errors: [ + { message: "'predicate' is never reassigned. Use 'const' instead.", type: 'Identifier' } + ] + }, + { + code: "let x = 'x', y = 'y';", + output: "const x = 'x', y = 'y';", + errors: [ + { message: "'x' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' } + ] + }, + { + code: "let x = 'x', y = 'y'; x = 1", + output: null, + errors: [{ message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' }] + }, + { + code: "let x = 1, y = 'y'; let z = 1;", + output: "const x = 1, y = 'y'; const z = 1;", + errors: [ + { message: "'x' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'z' is never reassigned. Use 'const' instead.", type: 'Identifier' } + ] + }, + { + code: 'let { a, b, c} = obj; let { x, y, z} = anotherObj; x = 2;', + output: 'const { a, b, c} = obj; let { x, y, z} = anotherObj; x = 2;', + errors: [ + { message: "'a' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'b' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'c' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'z' is never reassigned. Use 'const' instead.", type: 'Identifier' } + ] + }, + { + code: "let x = 'x', y = 'y'; function someFunc() { let a = 1, b = 2; foo(a, b) }", + output: "const x = 'x', y = 'y'; function someFunc() { const a = 1, b = 2; foo(a, b) }", + errors: [ + { message: "'x' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'a' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'b' is never reassigned. Use 'const' instead.", type: 'Identifier' } + ] + }, + // The inner `let` will be auto-fixed in the second pass + { + code: 'let someFunc = () => { let a = 1, b = 2; foo(a, b) }', + output: 'const someFunc = () => { let a = 1, b = 2; foo(a, b) }', + errors: [ + { message: "'someFunc' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'a' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'b' is never reassigned. Use 'const' instead.", type: 'Identifier' } + ] + }, + // https://github.com/eslint/eslint/issues/11699 + { + code: 'let {a, b} = c, d;', + output: null, + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } + ] + }, + { + code: 'let {a, b, c} = {}, e, f;', + output: null, + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' } + ] + }, + { + code: [ + 'function a() {', + 'let foo = 0,', + ' bar = 1;', + 'foo = 1;', + '}', + 'function b() {', + 'let foo = 0,', + ' bar = 2;', + 'foo = 2;', + '}' + ].join('\n'), + output: null, + errors: [ + { message: "'bar' is never reassigned. Use 'const' instead.", type: 'Identifier' }, + { message: "'bar' is never reassigned. Use 'const' instead.", type: 'Identifier' } + ] + }, + // https://github.com/eslint/eslint/issues/13899 + { + code: '/*eslint no-undef-init:error*/ let foo = undefined;', + output: '/*eslint no-undef-init:error*/ const foo = undefined;', + errors: 2 + }, + { + code: 'let a = 1; class C { static { a; } }', + output: 'const a = 1; class C { static { a; } }', + languageOptions: { ecmaVersion: 2022 }, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + // this is a TDZ error with either `let` or `const`, but that isn't a concern of this rule + code: 'class C { static { a; } } let a = 1;', + output: 'class C { static { a; } } const a = 1;', + languageOptions: { ecmaVersion: 2022 }, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'class C { static { let a = 1; } }', + output: 'class C { static { const a = 1; } }', + languageOptions: { ecmaVersion: 2022 }, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'class C { static { if (foo) { let a = 1; } } }', + output: 'class C { static { if (foo) { const a = 1; } } }', + languageOptions: { ecmaVersion: 2022 }, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'class C { static { let a = 1; if (foo) { a; } } }', + output: 'class C { static { const a = 1; if (foo) { a; } } }', + languageOptions: { ecmaVersion: 2022 }, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'class C { static { if (foo) { let a; a = 1; } } }', + output: null, + languageOptions: { ecmaVersion: 2022 }, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + { + code: 'class C { static { let a; a = 1; } }', + output: null, + languageOptions: { ecmaVersion: 2022 }, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier', column: 27 }] + }, + { + code: 'class C { static { let { a, b } = foo; } }', + output: 'class C { static { const { a, b } = foo; } }', + languageOptions: { ecmaVersion: 2022 }, + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } + ] + }, + { + code: 'class C { static { let a, b; ({ a, b } = foo); } }', + output: null, + languageOptions: { ecmaVersion: 2022 }, + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } + ] + }, + { + code: 'class C { static { let a; let b; ({ a, b } = foo); } }', + output: null, + languageOptions: { ecmaVersion: 2022 }, + errors: [ + { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } + ] + }, + { + code: 'class C { static { let a; a = 0; console.log(a); } }', + output: null, + languageOptions: { ecmaVersion: 2022 }, + errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] + }, + // https://github.com/eslint/eslint/issues/16266 + { + code: ` + let { itemId, list } = {}, + obj = [], + total = 0; + total = 9; + console.log(itemId, list, obj, total); + `, + output: null, + options: [{ destructuring: 'any', ignoreReadBeforeAssign: true }], + languageOptions: { ecmaVersion: 2022 }, + errors: [ + { messageId: 'useConst', data: { name: 'itemId' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'list' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'obj' }, type: 'Identifier' } + ] + }, + { + code: ` + let { itemId, list } = {}, + obj = []; + console.log(itemId, list, obj); + `, + output: ` + const { itemId, list } = {}, + obj = []; + console.log(itemId, list, obj); + `, + options: [{ destructuring: 'any', ignoreReadBeforeAssign: true }], + languageOptions: { ecmaVersion: 2022 }, + errors: [ + { messageId: 'useConst', data: { name: 'itemId' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'list' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'obj' }, type: 'Identifier' } + ] + }, + { + code: ` + let [ itemId, list ] = [], + total = 0; + total = 9; + console.log(itemId, list, total); + `, + output: null, + options: [{ destructuring: 'any', ignoreReadBeforeAssign: true }], + languageOptions: { ecmaVersion: 2022 }, + errors: [ + { messageId: 'useConst', data: { name: 'itemId' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'list' }, type: 'Identifier' } + ] + }, + { + code: ` + let [ itemId, list ] = [], + obj = []; + console.log(itemId, list, obj); + `, + output: ` + const [ itemId, list ] = [], + obj = []; + console.log(itemId, list, obj); + `, + options: [{ destructuring: 'any', ignoreReadBeforeAssign: true }], + languageOptions: { ecmaVersion: 2022 }, + errors: [ + { messageId: 'useConst', data: { name: 'itemId' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'list' }, type: 'Identifier' }, + { messageId: 'useConst', data: { name: 'obj' }, type: 'Identifier' } + ] + } + ] as InvalidTestCases +}; + +tester.run('prefer-const', rule as any, { + valid: [...ESLINT_RULE_TEST_CASES.valid, ...loadedCases.valid], + invalid: [...ESLINT_RULE_TEST_CASES.invalid, ...loadedCases.invalid] +}); From f204f4d1dc5ce30fabfa384ab5cf94ca94f99868 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Sat, 30 Nov 2024 18:32:53 +0100 Subject: [PATCH 13/22] fix(prefer-const): use core rule helpers Remove the duplicated code implementation in the rule, by using the helpers to get the actual rule :sparkles: --- .../src/rules/prefer-const-helpers/index.ts | 562 ------------------ .../src/rules/prefer-const.ts | 132 ++-- 2 files changed, 86 insertions(+), 608 deletions(-) delete mode 100644 packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts deleted file mode 100644 index d979b8b71..000000000 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const-helpers/index.ts +++ /dev/null @@ -1,562 +0,0 @@ -/** - * Atributions: - * Rule and helpers are adapted from ESLint - * https://github.com/eslint/eslint/blob/main/lib/rules/prefer-const.js - */ - -import type { Reference, Variable, Scope } from '@typescript-eslint/scope-manager'; -import type { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/types'; - -import { getSourceCode } from '../../utils/compat'; -import type { RuleContext } from '../../types'; - -type ASTNode = TSESTree.Node; -type CheckedNode = NonNullable }>; -type VariableDeclaration = - | TSESTree.LetOrConstOrVarDeclaration - | TSESTree.UsingInForOfDeclaration - | TSESTree.UsingInNormalContextDeclaration; -type VariableDeclarator = - | TSESTree.LetOrConstOrVarDeclarator - | TSESTree.UsingInForOfDeclarator - | TSESTree.UsingInNormalContextDeclarator; - -const PATTERN_TYPE = - /^(?:.+?Pattern|RestElement|SpreadProperty|ExperimentalRestProperty|Property)$/u; -const DECLARATION_HOST_TYPE = /^(?:Program|BlockStatement|StaticBlock|SwitchCase)$/u; -const DESTRUCTURING_HOST_TYPE = /^(?:VariableDeclarator|AssignmentExpression)$/u; - -/** - * Finds the callee of a `CallExpression` if the given node is part of a - * `VariableDeclarator` or an ObjectPattern within a `VariableDeclarator`. - */ -function findIdentifierCallee(node: CheckedNode) { - const { parent } = node; - if (parent.type === 'VariableDeclarator' && parent.init?.type === 'CallExpression') { - return parent.init.callee; - } - - if ( - parent.type === 'Property' && - parent.parent.type === 'ObjectPattern' && - parent.parent.parent.type === 'VariableDeclarator' && - parent.parent.parent.init?.type === 'CallExpression' - ) { - return parent.parent.parent.init.callee; - } - - return null; -} - -/** - * Skip let statements when they are reactive values created by runes. - */ -function skipReactiveValues(identifier: ASTNode | null) { - if (!identifier || !identifier.parent) { - return false; - } - - const callee = findIdentifierCallee(identifier); - if (!callee) { - return true; - } - - if (callee.type === 'Identifier' && ['$state', '$props', '$derived'].includes(callee.name)) { - return false; - } - - if (callee.type !== 'MemberExpression' || callee.object.type !== 'Identifier') { - return true; - } - - if ( - callee.object.name === '$derived' && - callee.property.type === 'Identifier' && - callee.property.name === 'by' - ) { - return false; - } - if ( - callee.object.name === '$state' && - callee.property.type === 'Identifier' && - callee.property.name === 'frozen' - ) { - return false; - } - - return true; -} - -/** - * Checks whether a given `Identifier` node becomes a `VariableDeclaration` or - * not. - */ -function canBecomeVariableDeclaration(identifier: ASTNode) { - let node = identifier.parent; - - while (node && PATTERN_TYPE.test(node.type)) { - node = node.parent; - } - if (!node) { - // TS check - return false; - } - - const isValidAssignmentExpr = - node.type === 'AssignmentExpression' && - node.parent.type === 'ExpressionStatement' && - DECLARATION_HOST_TYPE.test(node.parent.parent.type); - return node.type === 'VariableDeclarator' || isValidAssignmentExpr; -} - -/** - * Checks if an property or element is from outer scope or export function - * parameters in destructing pattern. - */ -function isOuterVariableInDestructing(name: string, initScope: Scope) { - if (initScope.through.some((ref) => ref.resolved && ref.resolved.name === name)) { - return true; - } - - const variable = getVariableByName(initScope, name); - return variable !== null ? variable.defs.some((def) => def.type === 'Parameter') : false; -} - -/** - * Gets the `VariableDeclarator/AssignmentExpression` node that a given - * reference belongs to. It is used to detect a mix of reassigned and never - * reassigned in a destructuring. - */ -function getDestructuringHost(reference: Reference) { - if (!reference.isWrite()) { - return null; - } - - let node = reference.identifier.parent; - while (PATTERN_TYPE.test(node.type)) { - if (!node.parent) { - break; - } - - node = node.parent; - } - - return !DESTRUCTURING_HOST_TYPE.test(node.type) ? null : node; -} - -/** - * Determines if a destructuring assignment node contains any - * `MemberExpression` nodes. is used to determine if a variable that is only - * written once using destructuring can be safely converted into a const - * declaration. - */ -function hasMemberExpressionAssignment(node: ASTNode): boolean { - if (node.type === 'MemberExpression') { - return true; - } - if (node.type === 'ObjectPattern') { - return node.properties.some((prop) => - prop ? hasMemberExpressionAssignment('argument' in prop ? prop.argument : prop.value) : false - ); - } - if (node.type === 'ArrayPattern') { - return node.elements.some((element) => - element ? hasMemberExpressionAssignment(element) : false - ); - } - if (node.type === 'AssignmentPattern') { - return hasMemberExpressionAssignment(node.left); - } - - return false; -} - -/** - * Validates an object pattern to determine if it contains outer variables or - * non-identifier assignments. - */ -function validateObjectPattern(node: TSESTree.ObjectPattern, variable: Variable) { - const properties = node.properties; - const hasOuterVariables = properties - .filter((prop) => prop.value) - .map((prop) => - 'value' in prop && prop.value - ? 'name' in prop.value - ? prop.value.name - : undefined - : undefined - ) - .some((name) => name && isOuterVariableInDestructing(name, variable.scope)); - const hasNonIdentifiers = hasMemberExpressionAssignment(node); - - return hasOuterVariables || hasNonIdentifiers; -} - -/** - * Validates an array pattern to determine if it contains outer variables or - * non-identifier elements. - */ -function validateArrayPattern(node: TSESTree.ArrayPattern, variable: Variable): boolean { - const elements = node.elements; - const hasOuterVariables = elements - .map((element) => (element && 'name' in element ? element.name : undefined)) - .some((name) => name && isOuterVariableInDestructing(name, variable.scope)); - const hasNonIdentifiers = hasMemberExpressionAssignment(node); - - return hasOuterVariables || hasNonIdentifiers; -} - -/** - * Gets an identifier node of a given variable. - * - * If the initialization exists or one or more reading references exist before - * the first assignment, the identifier node is the node of the declaration. - * Otherwise, the identifier node is the node of the first assignment. - * - * If the variable should not change to const, export function returns null. - * - If the variable is reassigned. - * - If the variable is never initialized nor assigned. - * - If the variable is initialized in a different scope from the declaration. - * - If the unique assignment of the variable cannot change to a declaration. - * e.g. `if (a) b = 1` / `return (b = 1)` - * - If the variable is declared in the global scope and `eslintUsed` is `true`. - * `/*exported foo` directive comment makes such variables. rule does not - * warn such variables because rule cannot distinguish whether the - * exported variables are reassigned or not. - */ -function getIdentifierIfShouldBeConst(variable: Variable, ignoreReadBeforeAssign: boolean) { - if (variable.eslintUsed && variable.scope.type === 'global') { - return null; - } - - let writer = null; - let isReadBeforeInit = false; - const references = variable.references; - for (let i = 0; i < references.length; ++i) { - const reference = references[i]; - - if (reference.isWrite()) { - const isReassigned = writer !== null && writer.identifier !== reference.identifier; - if (isReassigned) { - return null; - } - - const destructuringHost = getDestructuringHost(reference); - const isValidDestructuringHost = - destructuringHost !== null && - 'left' in destructuringHost && - destructuringHost.left !== undefined; - - if (isValidDestructuringHost) { - const leftNode = destructuringHost.left; - - if (leftNode.type === 'ObjectPattern' && validateObjectPattern(leftNode, variable)) { - return null; - } else if (leftNode.type === 'ArrayPattern' && validateArrayPattern(leftNode, variable)) { - return null; - } - } - - writer = reference; - } else if (reference.isRead() && writer === null) { - if (ignoreReadBeforeAssign) { - return null; - } - isReadBeforeInit = true; - } - } - - const shouldBeConst = - writer !== null && - writer.from === variable.scope && - canBecomeVariableDeclaration(writer.identifier); - if (!shouldBeConst) { - return null; - } - - if (isReadBeforeInit) { - return variable.defs[0].name; - } - - return writer?.identifier ?? null; -} - -/** - * Finds the nearest parent of node with a given type. - */ -function findUp(node: ASTNode, type: `${AST_NODE_TYPES}`, shouldStop: (node: ASTNode) => boolean) { - if (!node || shouldStop(node) || !node.parent) { - return null; - } - if (node.type === type) { - return node; - } - - return findUp(node.parent, type, shouldStop); -} - -/** - * Finds the variable by a given name in a given scope and its upper scopes. - */ -function getVariableByName(initScope: Scope, name: string): Variable | null { - let scope: Scope | null = initScope; - - while (scope) { - const variable = scope.set.get(name); - if (variable) { - return variable; - } - scope = scope.upper; - } - - return null; -} - -/** - * Checks whether a given node is located at `ForStatement.init` or not. - */ -export function isInitOfForStatement(node: ASTNode): boolean { - return Boolean(node.parent && node.parent.type === 'ForStatement' && node.parent.init === node); -} - -/** - * Groups by the VariableDeclarator/AssignmentExpression node that each - * reference of given variables belongs to. - * is used to detect a mix of reassigned and never reassigned in a - * destructuring. - */ -export function groupByDestructuring( - variables: Variable[], - ignoreReadBeforeAssign: boolean -): Map { - const identifierMap = new Map(); - - for (let i = 0; i < variables.length; ++i) { - const variable = variables[i]; - const references = variable.references; - const identifier = getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign); - - let prevId: TSESTree.Identifier | TSESTree.JSXIdentifier | null = null; - references.forEach((reference) => { - const id = reference.identifier; - if (id === prevId) { - return; - } - - prevId = id; - const group = getDestructuringHost(reference); - if (!group) { - return; - } - - if (identifierMap.has(group)) { - identifierMap.get(group)!.push(identifier); - } else { - identifierMap.set(group, [identifier]); - } - }); - } - - return identifierMap; -} - -/** - * Calculates the retained text range by comparing the given range with an - * optional retained range. If a retained range is provided, it merges the two - * ranges to form an actual range. It then returns two ranges: one from the - * start of the actual range to the start of the given range, and another from - * the end of the given range to the end of the actual range. - */ -function calculateRetainedTextRange( - range: TSESTree.Range, - retainedRange: TSESTree.Range | null -): [TSESTree.Range, TSESTree.Range] { - const actualRange = retainedRange - ? [Math.min(retainedRange[0], range[0]), Math.max(retainedRange[1], range[1])] - : range; - - return [ - [actualRange[0], range[0]], - [range[1], actualRange[1]] - ]; -} - -type NodeReporterOptions = { destructuring: 'any' | 'all' }; -type NodeReporter = { report: (nodes: (ASTNode | null)[]) => void }; - -/** - * Creates a node reporter function that checks and reports nodes based on the - * provided context and options. - * - * @remarks The `report` function checks and reports nodes that should be - * converted to `const` declarations. It performs various checks to ensure that - * the nodes meet the criteria for reporting and fixing. - * - * The function also handles cases where variables are declared using - * destructuring patterns and ensures that the correct number of nodes are - * reported and fixed. - */ -export function createNodeReporter( - context: RuleContext, - { destructuring }: NodeReporterOptions -): NodeReporter { - let reportCount = 0; - let checkedId: TSESTree.BindingName | null = null; - let checkedName = ''; - const shouldMatchAnyDestructuredVariable = destructuring !== 'all'; - - function checkAndReportNodes(initialNodes: (ASTNode | null)[]): void { - const nodes = initialNodes.filter(Boolean) as ASTNode[]; - const shouldCheckGroup = - nodes.length && (shouldMatchAnyDestructuredVariable || initialNodes.length === nodes.length); - if (!shouldCheckGroup) { - return; - } - - const variableDeclarationParent = findUp( - nodes[0], - 'VariableDeclaration', - (parentNode: ASTNode) => parentNode.type.endsWith('Statement') - ); - const isVarDecParentNull = variableDeclarationParent === null; - const isValidDecParent = - !isVarDecParentNull && - 'declarations' in variableDeclarationParent && - variableDeclarationParent.declarations.length > 0; - if (isValidDecParent) { - checkDeclarator(variableDeclarationParent.declarations[0]); - } - - const shouldFix = - isValidDecParent && - checkShouldFix(variableDeclarationParent, nodes.length, initialNodes.length); - const sourceCode = getSourceCode(context); - nodes.filter(skipReactiveValues).forEach((node) => { - report(sourceCode, node, variableDeclarationParent!, shouldFix); - }); - } - - function report( - sourceCode: ReturnType, - node: ASTNode, - nodeParent: ASTNode, - shouldFix: boolean - ) { - context.report({ - node, - messageId: 'useConst', - // @ts-expect-error Name will exist at point - data: { name: node.name }, - fix: !shouldFix - ? null - : (fixer) => { - const letKeywordToken = sourceCode.getFirstToken(nodeParent, { - includeComments: false, - filter: (t) => 'kind' in nodeParent && t.value === nodeParent.kind - }); - if (!letKeywordToken) { - return null; - } - - const [initialRange, finalRange] = calculateRetainedTextRange( - letKeywordToken.range, - nodeParent.range - ); - const prefix = sourceCode.text.slice(...initialRange); - const suffix = sourceCode.text.slice(...finalRange); - - return fixer.replaceTextRange( - [initialRange[0], finalRange[1]], - `${prefix}const${suffix}` - ); - } - }); - } - - function checkShouldFix( - declaration: VariableDeclaration, - totalNodes: number, - totalInitialNodes: number - ) { - const isParentForStatement = - declaration?.parent.type === 'ForInStatement' || - declaration?.parent.type === 'ForOfStatement'; - const areAllDeclarationsInitialized = - 'declarations' in declaration && declaration.declarations.every((decl) => decl.init); - const shouldFix = - totalNodes === totalInitialNodes && - declaration && - (isParentForStatement || areAllDeclarationsInitialized); - - const totalDeclarationCount = checkDestructuredDeclaration(declaration, totalNodes); - if (totalDeclarationCount === -1) { - return shouldFix; - } - - return shouldFix && reportCount === totalDeclarationCount; - } - - function checkDestructuredDeclaration(declaration: VariableDeclaration, totalNodes: number) { - const isValidDeclaration = declaration !== null && 'declarations' in declaration; - const hasMultipleDeclarations = isValidDeclaration && declaration.declarations.length !== 1; - if (!hasMultipleDeclarations) { - return -1; - } - - const hasMoreThanOneDeclaration = - declaration.declarations && declaration.declarations.length >= 1; - if (!hasMoreThanOneDeclaration) { - return -1; - } - - reportCount += totalNodes; - - return declaration.declarations.reduce((total, declaration) => { - if (declaration.id.type === 'ObjectPattern') { - return total + declaration.id.properties.length; - } - - if (declaration.id.type === 'ArrayPattern') { - return total + declaration.id.elements.length; - } - - return total + 1; - }, 0); - } - - function checkDeclarator(declarator: VariableDeclarator) { - if (!declarator.init) { - return; - } - - const firstDeclaratorParent = declarator.init.parent; - if (firstDeclaratorParent.type !== 'VariableDeclarator') { - return; - } - - const { id } = firstDeclaratorParent; - if ('name' in id && id.name !== checkedName) { - checkedName = id.name; - reportCount = 0; - } - - if (firstDeclaratorParent.id.type === 'ObjectPattern') { - const { init } = firstDeclaratorParent; - if (init && 'name' in init && init.name !== checkedName) { - checkedName = init.name; - reportCount = 0; - } - } - - if (firstDeclaratorParent.id !== checkedId) { - checkedId = firstDeclaratorParent.id; - reportCount = 0; - } - } - - return { - report(group) { - checkAndReportNodes(group); - } - }; -} diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index 91944fd11..acfbc4d07 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -1,59 +1,99 @@ -import type { Variable } from '@typescript-eslint/scope-manager'; import { createRule } from '../utils'; -import { getSourceCode } from '../utils/compat'; +import type { TSESTree } from '@typescript-eslint/types'; -import { - createNodeReporter, - groupByDestructuring, - isInitOfForStatement -} from './prefer-const-helpers'; +import { defineWrapperListener, getCoreRule } from '../utils/eslint-core'; + +const coreRule = getCoreRule('prefer-const'); + +type Declaration = TSESTree.Expression; + +/** + * Finds and returns the callee of a declaration node within variable declarations or object patterns. + */ +function findDeclarationCallee(node: Declaration) { + const { parent } = node; + if (parent.type === 'VariableDeclarator' && parent.init?.type === 'CallExpression') { + return parent.init.callee; + } + + if ( + parent.type === 'Property' && + parent.parent.type === 'ObjectPattern' && + parent.parent.parent.type === 'VariableDeclarator' && + parent.parent.parent.init?.type === 'CallExpression' + ) { + return parent.parent.parent.init.callee; + } + + return null; +} + +/** + * Determines if a declaration should be skipped in the const preference analysis. + * Specifically checks for Svelte's state management utilities ($state, $props, $derived). + */ +function shouldSkipDeclaration(declaration: Declaration | null) { + if (!declaration || !declaration.parent) { + return false; + } + + const callee = findDeclarationCallee(declaration); + if (!callee) { + return false; + } + + if (callee.type === 'Identifier' && ['$state', '$props', '$derived'].includes(callee.name)) { + return true; + } + + if (callee.type !== 'MemberExpression' || callee.object.type !== 'Identifier') { + return false; + } + + if ( + callee.object.name === '$derived' && + callee.property.type === 'Identifier' && + callee.property.name === 'by' + ) { + return true; + } + if ( + callee.object.name === '$state' && + callee.property.type === 'Identifier' && + callee.property.name === 'frozen' + ) { + return true; + } + + return false; +} export default createRule('prefer-const', { meta: { - type: 'suggestion', + ...coreRule.meta, docs: { - description: - 'Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values).', + description: coreRule.meta.docs.description, category: 'Best Practices', - recommended: false - }, - fixable: 'code', - schema: [ - { - type: 'object', - properties: { - destructuring: { enum: ['any', 'all'], default: 'any' }, - ignoreReadBeforeAssign: { type: 'boolean', default: false } - }, - additionalProperties: false - } - ], - messages: { - useConst: "'{{name}}' is never reassigned. Use 'const' instead." + recommended: false, + extensionRule: 'prefer-const' } }, create(context) { - const sourceCode = getSourceCode(context); - - const options = context.options[0] || {}; - const ignoreReadBeforeAssign = options.ignoreReadBeforeAssign === true; - - const variables: Variable[] = []; - - return { - 'Program:exit'() { - const nodeReporter = createNodeReporter(context, { - destructuring: options.destructuring - }); - groupByDestructuring(variables, ignoreReadBeforeAssign).forEach((group) => { - nodeReporter.report(group); - }); - }, - VariableDeclaration(node) { - if (node.kind === 'let' && !isInitOfForStatement(node)) { - variables.push(...sourceCode.getDeclaredVariables(node)); - } + return defineWrapperListener(coreRule, context, { + createListenerProxy(coreListener) { + return { + ...coreListener, + VariableDeclaration(node) { + for (const decl of node.declarations) { + if (shouldSkipDeclaration(decl.init)) { + return; + } + } + + coreListener.VariableDeclaration?.(node); + } + }; } - }; + }); } }); From 202e3a03340e6a03335c4435bf5daedff6a38bbc Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Sat, 30 Nov 2024 18:35:44 +0100 Subject: [PATCH 14/22] test(prefer-const): correct tests --- .../eslint-plugin-svelte/tests/src/rules/prefer-const.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts index d7f62a832..9f7e79bd0 100644 --- a/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts @@ -8,7 +8,7 @@ type InvalidTestCases = ReturnType['invalid']; const tester = new RuleTester({ languageOptions: { ecmaVersion: 2020, - sourceType: 'module' + sourceType: 'script' }, plugins: { custom: { @@ -73,8 +73,7 @@ const ESLINT_RULE_TEST_CASES = { 'foo();' ].join('\n'), '/*exported a*/ let a; function init() { a = foo(); }', - // TODO: Skipping this test until https://github.com/typescript-eslint/typescript-eslint/issues/10426 - // '/*exported a*/ let a = 1', + '/*exported a*/ let a = 1', 'let a; if (true) a = 0; foo(a);', ` (function (a) { From 7bdf0bdbbc653b5992a4447ec55e2cd2e23aea56 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Sat, 30 Nov 2024 18:37:38 +0100 Subject: [PATCH 15/22] chore(prefer-const): update resources --- README.md | 2 +- docs/rules.md | 2 +- docs/rules/prefer-const.md | 6 ++++-- packages/eslint-plugin-svelte/src/rule-types.ts | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b7807529e..e492f8a76 100644 --- a/README.md +++ b/README.md @@ -367,7 +367,7 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-unused-class-name](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-class-name/) | disallow the use of a class in the template without a corresponding style | | | [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: | | [svelte/no-useless-mustaches](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: | -| [svelte/prefer-const](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/) | Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values). | :wrench: | +| [svelte/prefer-const](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/) | Require `const` declarations for variables that are never reassigned after declared | :wrench: | | [svelte/prefer-destructured-store-props](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/) | destructure values from object stores for better change tracking & fewer redraws | :bulb: | | [svelte/require-each-key](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/) | require keyed `{#each}` block | | | [svelte/require-event-dispatcher-types](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-event-dispatcher-types/) | require type parameters for `createEventDispatcher` | | diff --git a/docs/rules.md b/docs/rules.md index 2f2870cc8..d2dd3d7a7 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -64,7 +64,7 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-unused-class-name](./rules/no-unused-class-name.md) | disallow the use of a class in the template without a corresponding style | | | [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: | | [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :wrench: | -| [svelte/prefer-const](./rules/prefer-const.md) | Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values). | :wrench: | +| [svelte/prefer-const](./rules/prefer-const.md) | Require `const` declarations for variables that are never reassigned after declared | :wrench: | | [svelte/prefer-destructured-store-props](./rules/prefer-destructured-store-props.md) | destructure values from object stores for better change tracking & fewer redraws | :bulb: | | [svelte/require-each-key](./rules/require-each-key.md) | require keyed `{#each}` block | | | [svelte/require-event-dispatcher-types](./rules/require-event-dispatcher-types.md) | require type parameters for `createEventDispatcher` | | diff --git a/docs/rules/prefer-const.md b/docs/rules/prefer-const.md index d043f3cf0..534d44d6e 100644 --- a/docs/rules/prefer-const.md +++ b/docs/rules/prefer-const.md @@ -2,12 +2,12 @@ pageClass: 'rule-details' sidebarDepth: 0 title: 'svelte/prefer-const' -description: 'Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values).' +description: 'Require `const` declarations for variables that are never reassigned after declared' --- # svelte/prefer-const -> Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values). +> Require `const` declarations for variables that are never reassigned after declared - :exclamation: **_This rule has not been released yet._** - :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule. @@ -63,3 +63,5 @@ This rule reports the same as the base ESLint `prefer-const` rule, except that i - [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/prefer-const.ts) - [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts) + +Taken with ❤️ [from ESLint core](https://eslint.org/docs/rules/prefer-const) diff --git a/packages/eslint-plugin-svelte/src/rule-types.ts b/packages/eslint-plugin-svelte/src/rule-types.ts index 0403bb4ae..71eae844b 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -265,7 +265,7 @@ export interface RuleOptions { */ 'svelte/prefer-class-directive'?: Linter.RuleEntry /** - * Require `const` declarations for variables that are never reassigned after declared (excluding Svelte reactive values). + * Require `const` declarations for variables that are never reassigned after declared * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/ */ 'svelte/prefer-const'?: Linter.RuleEntry From 449691fc58747075e3b101dfd2fb2b1a4d2eec7f Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Mon, 2 Dec 2024 08:17:51 +0100 Subject: [PATCH 16/22] fix(prefer-const): corrections - Removed unnecessary if branches - Updated tests cases - Since we are now using the default implementation of the rule, there's no need to keep the original tests, since the default rule is already tested. - Removed unnecessary test cases --- .../src/rules/prefer-const.ts | 19 +- .../prefer-const/invalid/test01-input.svelte | 3 +- .../prefer-const/invalid/test01-output.svelte | 3 +- .../prefer-const/invalid/test02-errors.yaml | 4 - .../prefer-const/invalid/test02-input.svelte | 7 - .../prefer-const/invalid/test02-output.svelte | 7 - .../tests/src/rules/prefer-const.ts | 732 +----------------- 7 files changed, 10 insertions(+), 765 deletions(-) delete mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-errors.yaml delete mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-input.svelte delete mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-output.svelte diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index acfbc4d07..91ff58931 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -5,26 +5,15 @@ import { defineWrapperListener, getCoreRule } from '../utils/eslint-core'; const coreRule = getCoreRule('prefer-const'); -type Declaration = TSESTree.Expression; - /** * Finds and returns the callee of a declaration node within variable declarations or object patterns. */ -function findDeclarationCallee(node: Declaration) { +function findDeclarationCallee(node: TSESTree.Expression) { const { parent } = node; if (parent.type === 'VariableDeclarator' && parent.init?.type === 'CallExpression') { return parent.init.callee; } - if ( - parent.type === 'Property' && - parent.parent.type === 'ObjectPattern' && - parent.parent.parent.type === 'VariableDeclarator' && - parent.parent.parent.init?.type === 'CallExpression' - ) { - return parent.parent.parent.init.callee; - } - return null; } @@ -32,8 +21,8 @@ function findDeclarationCallee(node: Declaration) { * Determines if a declaration should be skipped in the const preference analysis. * Specifically checks for Svelte's state management utilities ($state, $props, $derived). */ -function shouldSkipDeclaration(declaration: Declaration | null) { - if (!declaration || !declaration.parent) { +function shouldSkipDeclaration(declaration: TSESTree.Expression | null) { + if (!declaration) { return false; } @@ -60,7 +49,7 @@ function shouldSkipDeclaration(declaration: Declaration | null) { if ( callee.object.name === '$state' && callee.property.type === 'Identifier' && - callee.property.name === 'frozen' + callee.property.name === 'raw' ) { return true; } diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-input.svelte index 0f5f5e54f..2a9ff2bba 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-input.svelte @@ -2,9 +2,10 @@ let { prop1, prop2 } = $props(); let zero = 0; let state = $state(0); - let frozen = $state.frozen(0); + let raw = $state.raw(0); let doubled = state * 2; let derived = $derived(state * 2); let calculated = calc(); let derivedBy = $derived.by(calc()); + let noInit; diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte index 3138619fd..97d178239 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte @@ -2,9 +2,10 @@ let { prop1, prop2 } = $props(); const zero = 0; let state = $state(0); - let frozen = $state.frozen(0); + let raw = $state.raw(0); const doubled = state * 2; let derived = $derived(state * 2); const calculated = calc(); let derivedBy = $derived.by(calc()); + let noInit; diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-errors.yaml deleted file mode 100644 index 0c4b62dd9..000000000 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-errors.yaml +++ /dev/null @@ -1,4 +0,0 @@ -- message: "'fn' is never reassigned. Use 'const' instead." - line: 4 - column: 6 - suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-input.svelte deleted file mode 100644 index 82fdb483b..000000000 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-input.svelte +++ /dev/null @@ -1,7 +0,0 @@ - diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-output.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-output.svelte deleted file mode 100644 index da091dd5a..000000000 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test02-output.svelte +++ /dev/null @@ -1,7 +0,0 @@ - diff --git a/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts index 9f7e79bd0..ffe176d0a 100644 --- a/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts @@ -2,739 +2,11 @@ import { RuleTester } from '../../utils/eslint-compat'; import rule from '../../../src/rules/prefer-const'; import { loadTestCases } from '../../utils/utils'; -type ValidTestCases = ReturnType['valid']; -type InvalidTestCases = ReturnType['invalid']; - const tester = new RuleTester({ languageOptions: { ecmaVersion: 2020, - sourceType: 'script' + sourceType: 'module' }, - plugins: { - custom: { - rules: { - 'use-x': { - create(context) { - const sourceCode = context.sourceCode; - - return { - VariableDeclaration(node) { - sourceCode.markVariableAsUsed('x', node); - } - }; - } - } - } - } - } }); -const loadedCases = loadTestCases('prefer-const'); -// This tests cases have been imported/adapted from eslint/prefer-const rule -const ESLINT_RULE_TEST_CASES = { - valid: [ - 'var x = 0;', - 'let x;', - 'let x; { x = 0; } foo(x);', - 'let x = 0; x = 1;', - 'const x = 0;', - 'for (let i = 0, end = 10; i < end; ++i) {}', - 'for (let i in [1,2,3]) { i = 0; }', - 'for (let x of [1,2,3]) { x = 0; }', - '(function() { var x = 0; })();', - '(function() { let x; })();', - '(function() { let x; { x = 0; } foo(x); })();', - '(function() { let x = 0; x = 1; })();', - '(function() { const x = 0; })();', - '(function() { for (let i = 0, end = 10; i < end; ++i) {} })();', - '(function() { for (let i in [1,2,3]) { i = 0; } })();', - '(function() { for (let x of [1,2,3]) { x = 0; } })();', - '(function(x = 0) { })();', - 'let a; while (a = foo());', - 'let a; do {} while (a = foo());', - 'let a; for (; a = foo(); );', - 'let a; for (;; ++a);', - 'let a; for (const {b = ++a} in foo());', - 'let a; for (const {b = ++a} of foo());', - 'let a; for (const x of [1,2,3]) { if (a) {} a = foo(); }', - 'let a; for (const x of [1,2,3]) { a = a || foo(); bar(a); }', - 'let a; for (const x of [1,2,3]) { foo(++a); }', - 'let a; function foo() { if (a) {} a = bar(); }', - 'let a; function foo() { a = a || bar(); baz(a); }', - 'let a; function foo() { bar(++a); }', - [ - 'let id;', - 'function foo() {', - " if (typeof id !== 'undefined') {", - ' return;', - ' }', - ' id = setInterval(() => {}, 250);', - '}', - 'foo();' - ].join('\n'), - '/*exported a*/ let a; function init() { a = foo(); }', - '/*exported a*/ let a = 1', - 'let a; if (true) a = 0; foo(a);', - ` - (function (a) { - let b; - ({ a, b } = obj); - })(); - `, - ` - (function (a) { - let b; - ([ a, b ] = obj); - })(); - `, - 'var a; { var b; ({ a, b } = obj); }', - 'let a; { let b; ({ a, b } = obj); }', - 'var a; { var b; ([ a, b ] = obj); }', - 'let a; { let b; ([ a, b ] = obj); }', - - /* - * The assignment is located in a different scope. - * Those are warned by prefer-smaller-scope. - */ - 'let x; { x = 0; foo(x); }', - '(function() { let x; { x = 0; foo(x); } })();', - 'let x; for (const a of [1,2,3]) { x = foo(); bar(x); }', - '(function() { let x; for (const a of [1,2,3]) { x = foo(); bar(x); } })();', - 'let x; for (x of array) { x; }', - { - code: 'let {a, b} = obj; b = 0;', - options: [{ destructuring: 'all' }] - }, - { - code: 'let a, b; ({a, b} = obj); b++;', - options: [{ destructuring: 'all' }] - }, - // https://github.com/eslint/eslint/issues/8187 - { - code: 'let { name, ...otherStuff } = obj; otherStuff = {};', - options: [{ destructuring: 'all' }], - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let { name, ...otherStuff } = obj; otherStuff = {};', - options: [{ destructuring: 'all' }] - }, - // https://github.com/eslint/eslint/issues/8308 - { - code: 'let predicate; [typeNode.returnType, predicate] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let predicate; [typeNode.returnType, ...predicate] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - // intentionally testing empty slot in destructuring assignment - code: 'let predicate; [typeNode.returnType,, predicate] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let predicate; [typeNode.returnType=5, predicate] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let predicate; [[typeNode.returnType=5], predicate] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let predicate; [[typeNode.returnType, predicate]] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let predicate; [typeNode.returnType, [predicate]] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let predicate; [, [typeNode.returnType, predicate]] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let predicate; [, {foo:typeNode.returnType, predicate}] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let predicate; [, {foo:typeNode.returnType, ...predicate}] = foo();', - languageOptions: { ecmaVersion: 2018 } - }, - { - code: 'let a; const b = {}; ({ a, c: b.c } = func());', - languageOptions: { ecmaVersion: 2018 } - }, - // ignoreReadBeforeAssign - { - code: 'let x; function foo() { bar(x); } x = 0;', - options: [{ ignoreReadBeforeAssign: true }] - }, - // https://github.com/eslint/eslint/issues/10520 - 'const x = [1,2]; let y; [,y] = x; y = 0;', - 'const x = [1,2,3]; let y, z; [y,,z] = x; y = 0; z = 0;', - { - code: 'class C { static { let a = 1; a = 2; } }', - languageOptions: { ecmaVersion: 2022 } - }, - { - code: 'class C { static { let a; a = 1; a = 2; } }', - languageOptions: { ecmaVersion: 2022 } - }, - { - code: 'let a; class C { static { a = 1; } }', - languageOptions: { ecmaVersion: 2022 } - }, - { - code: 'class C { static { let a; if (foo) { a = 1; } } }', - languageOptions: { ecmaVersion: 2022 } - }, - { - code: 'class C { static { let a; if (foo) a = 1; } }', - languageOptions: { ecmaVersion: 2022 } - }, - { - code: 'class C { static { let a, b; if (foo) { ({ a, b } = foo); } } }', - output: null, - languageOptions: { ecmaVersion: 2022 }, - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } - ] - }, - { - code: 'class C { static { let a, b; if (foo) ({ a, b } = foo); } }', - output: null, - languageOptions: { ecmaVersion: 2022 }, - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } - ] - }, - { - code: 'class C { static { a; } } let a = 1; ', - options: [{ ignoreReadBeforeAssign: true }], - languageOptions: { ecmaVersion: 2022 } - }, - { - code: 'class C { static { () => a; let a = 1; } };', - options: [{ ignoreReadBeforeAssign: true }], - languageOptions: { ecmaVersion: 2022 } - } - ] as ValidTestCases, - invalid: [ - { - code: 'let x = 1; foo(x);', - output: 'const x = 1; foo(x);', - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: 'for (let i in [1,2,3]) { foo(i); }', - output: 'for (const i in [1,2,3]) { foo(i); }', - errors: [{ messageId: 'useConst', data: { name: 'i' }, type: 'Identifier' }] - }, - { - code: 'for (let x of [1,2,3]) { foo(x); }', - output: 'for (const x of [1,2,3]) { foo(x); }', - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: 'let [x = -1, y] = [1,2]; y = 0;', - output: null, - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: 'let {a: x = -1, b: y} = {a:1,b:2}; y = 0;', - output: null, - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: '(function() { let x = 1; foo(x); })();', - output: '(function() { const x = 1; foo(x); })();', - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: '(function() { for (let i in [1,2,3]) { foo(i); } })();', - output: '(function() { for (const i in [1,2,3]) { foo(i); } })();', - errors: [{ messageId: 'useConst', data: { name: 'i' }, type: 'Identifier' }] - }, - { - code: '(function() { for (let x of [1,2,3]) { foo(x); } })();', - output: '(function() { for (const x of [1,2,3]) { foo(x); } })();', - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: '(function() { let [x = -1, y] = [1,2]; y = 0; })();', - output: null, - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: 'let f = (function() { let g = x; })(); f = 1;', - output: 'let f = (function() { const g = x; })(); f = 1;', - errors: [{ messageId: 'useConst', data: { name: 'g' }, type: 'Identifier' }] - }, - { - code: '(function() { let {a: x = -1, b: y} = {a:1,b:2}; y = 0; })();', - output: null, - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: 'let x = 0; { let x = 1; foo(x); } x = 0;', - output: 'let x = 0; { const x = 1; foo(x); } x = 0;', - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: 'for (let i = 0; i < 10; ++i) { let x = 1; foo(x); }', - output: 'for (let i = 0; i < 10; ++i) { const x = 1; foo(x); }', - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: 'for (let i in [1,2,3]) { let x = 1; foo(x); }', - output: 'for (const i in [1,2,3]) { const x = 1; foo(x); }', - errors: [ - { messageId: 'useConst', data: { name: 'i' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' } - ] - }, - { - code: [ - 'var foo = function() {', - ' for (const b of c) {', - ' let a;', - ' a = 1;', - ' }', - '};' - ].join('\n'), - output: null, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: [ - 'var foo = function() {', - ' for (const b of c) {', - ' let a;', - ' ({a} = 1);', - ' }', - '};' - ].join('\n'), - output: null, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'let x; x = 0;', - output: null, - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier', column: 8 }] - }, - { - code: 'switch (a) { case 0: let x; x = 0; }', - output: null, - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier', column: 29 }] - }, - { - code: '(function() { let x; x = 1; })();', - output: null, - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier', column: 22 }] - }, - - { - code: 'let {a = 0, b} = obj; b = 0; foo(a, b);', - output: null, - options: [{ destructuring: 'any' }], - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'let {a: {b, c}} = {a: {b: 1, c: 2}}; b = 3;', - output: null, - options: [{ destructuring: 'any' }], - errors: [{ messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' }] - }, - { - code: 'let {a: {b, c}} = {a: {b: 1, c: 2}}', - output: 'const {a: {b, c}} = {a: {b: 1, c: 2}}', - options: [{ destructuring: 'all' }], - errors: [ - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' } - ] - }, - { - code: 'let a, b; ({a = 0, b} = obj); b = 0; foo(a, b);', - output: null, - options: [{ destructuring: 'any' }], - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'let {a = 0, b} = obj; foo(a, b);', - output: 'const {a = 0, b} = obj; foo(a, b);', - options: [{ destructuring: 'all' }], - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } - ] - }, - { - code: 'let [a] = [1]', - output: 'const [a] = [1]', - options: [], - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'let {a} = obj', - output: 'const {a} = obj', - options: [], - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'let a, b; ({a = 0, b} = obj); foo(a, b);', - output: null, - options: [{ destructuring: 'all' }], - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } - ] - }, - { - code: 'let {a = 0, b} = obj, c = a; b = a;', - output: null, - options: [{ destructuring: 'any' }], - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' } - ] - }, - { - code: 'let {a = 0, b} = obj, c = a; b = a;', - output: null, - options: [{ destructuring: 'all' }], - errors: [{ messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' }] - }, - - // https://github.com/eslint/eslint/issues/8187 - { - code: 'let { name, ...otherStuff } = obj; otherStuff = {};', - output: null, - options: [{ destructuring: 'any' }], - languageOptions: { ecmaVersion: 2018 }, - errors: [{ messageId: 'useConst', data: { name: 'name' }, type: 'Identifier', column: 7 }] - }, - { - code: 'let { name, ...otherStuff } = obj; otherStuff = {};', - output: null, - options: [{ destructuring: 'any' }], - errors: [{ messageId: 'useConst', data: { name: 'name' }, type: 'Identifier', column: 7 }] - }, - // Warnings are located at declaration if there are reading references before assignments. - { - code: 'let x; function foo() { bar(x); } x = 0;', - output: null, - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier', column: 5 }] - }, - // https://github.com/eslint/eslint/issues/5837 - { - code: '/*eslint custom/use-x:error*/ let x = 1', - output: '/*eslint custom/use-x:error*/ const x = 1', - languageOptions: { parserOptions: { ecmaFeatures: { globalReturn: true } } }, - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: '/*eslint custom/use-x:error*/ { let x = 1 }', - output: '/*eslint custom/use-x:error*/ { const x = 1 }', - errors: [{ messageId: 'useConst', data: { name: 'x' }, type: 'Identifier' }] - }, - { - code: 'let { foo, bar } = baz;', - output: 'const { foo, bar } = baz;', - errors: [ - { messageId: 'useConst', data: { name: 'foo' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'bar' }, type: 'Identifier' } - ] - }, - // https://github.com/eslint/eslint/issues/10520 - { - code: 'const x = [1,2]; let [,y] = x;', - output: 'const x = [1,2]; const [,y] = x;', - errors: [{ messageId: 'useConst', data: { name: 'y' }, type: 'Identifier' }] - }, - { - code: 'const x = [1,2,3]; let [y,,z] = x;', - output: 'const x = [1,2,3]; const [y,,z] = x;', - errors: [ - { messageId: 'useConst', data: { name: 'y' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'z' }, type: 'Identifier' } - ] - }, - // https://github.com/eslint/eslint/issues/8308 - { - code: 'let predicate; [, {foo:returnType, predicate}] = foo();', - output: null, - languageOptions: { ecmaVersion: 2018 }, - errors: [ - { message: "'predicate' is never reassigned. Use 'const' instead.", type: 'Identifier' } - ] - }, - { - code: 'let predicate; [, {foo:returnType, predicate}, ...bar ] = foo();', - output: null, - languageOptions: { ecmaVersion: 2018 }, - errors: [ - { message: "'predicate' is never reassigned. Use 'const' instead.", type: 'Identifier' } - ] - }, - { - code: 'let predicate; [, {foo:returnType, ...predicate} ] = foo();', - output: null, - languageOptions: { ecmaVersion: 2018 }, - errors: [ - { message: "'predicate' is never reassigned. Use 'const' instead.", type: 'Identifier' } - ] - }, - { - code: "let x = 'x', y = 'y';", - output: "const x = 'x', y = 'y';", - errors: [ - { message: "'x' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' } - ] - }, - { - code: "let x = 'x', y = 'y'; x = 1", - output: null, - errors: [{ message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' }] - }, - { - code: "let x = 1, y = 'y'; let z = 1;", - output: "const x = 1, y = 'y'; const z = 1;", - errors: [ - { message: "'x' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'z' is never reassigned. Use 'const' instead.", type: 'Identifier' } - ] - }, - { - code: 'let { a, b, c} = obj; let { x, y, z} = anotherObj; x = 2;', - output: 'const { a, b, c} = obj; let { x, y, z} = anotherObj; x = 2;', - errors: [ - { message: "'a' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'b' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'c' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'z' is never reassigned. Use 'const' instead.", type: 'Identifier' } - ] - }, - { - code: "let x = 'x', y = 'y'; function someFunc() { let a = 1, b = 2; foo(a, b) }", - output: "const x = 'x', y = 'y'; function someFunc() { const a = 1, b = 2; foo(a, b) }", - errors: [ - { message: "'x' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'y' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'a' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'b' is never reassigned. Use 'const' instead.", type: 'Identifier' } - ] - }, - // The inner `let` will be auto-fixed in the second pass - { - code: 'let someFunc = () => { let a = 1, b = 2; foo(a, b) }', - output: 'const someFunc = () => { let a = 1, b = 2; foo(a, b) }', - errors: [ - { message: "'someFunc' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'a' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'b' is never reassigned. Use 'const' instead.", type: 'Identifier' } - ] - }, - // https://github.com/eslint/eslint/issues/11699 - { - code: 'let {a, b} = c, d;', - output: null, - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } - ] - }, - { - code: 'let {a, b, c} = {}, e, f;', - output: null, - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'c' }, type: 'Identifier' } - ] - }, - { - code: [ - 'function a() {', - 'let foo = 0,', - ' bar = 1;', - 'foo = 1;', - '}', - 'function b() {', - 'let foo = 0,', - ' bar = 2;', - 'foo = 2;', - '}' - ].join('\n'), - output: null, - errors: [ - { message: "'bar' is never reassigned. Use 'const' instead.", type: 'Identifier' }, - { message: "'bar' is never reassigned. Use 'const' instead.", type: 'Identifier' } - ] - }, - // https://github.com/eslint/eslint/issues/13899 - { - code: '/*eslint no-undef-init:error*/ let foo = undefined;', - output: '/*eslint no-undef-init:error*/ const foo = undefined;', - errors: 2 - }, - { - code: 'let a = 1; class C { static { a; } }', - output: 'const a = 1; class C { static { a; } }', - languageOptions: { ecmaVersion: 2022 }, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - // this is a TDZ error with either `let` or `const`, but that isn't a concern of this rule - code: 'class C { static { a; } } let a = 1;', - output: 'class C { static { a; } } const a = 1;', - languageOptions: { ecmaVersion: 2022 }, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'class C { static { let a = 1; } }', - output: 'class C { static { const a = 1; } }', - languageOptions: { ecmaVersion: 2022 }, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'class C { static { if (foo) { let a = 1; } } }', - output: 'class C { static { if (foo) { const a = 1; } } }', - languageOptions: { ecmaVersion: 2022 }, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'class C { static { let a = 1; if (foo) { a; } } }', - output: 'class C { static { const a = 1; if (foo) { a; } } }', - languageOptions: { ecmaVersion: 2022 }, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'class C { static { if (foo) { let a; a = 1; } } }', - output: null, - languageOptions: { ecmaVersion: 2022 }, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - { - code: 'class C { static { let a; a = 1; } }', - output: null, - languageOptions: { ecmaVersion: 2022 }, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier', column: 27 }] - }, - { - code: 'class C { static { let { a, b } = foo; } }', - output: 'class C { static { const { a, b } = foo; } }', - languageOptions: { ecmaVersion: 2022 }, - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } - ] - }, - { - code: 'class C { static { let a, b; ({ a, b } = foo); } }', - output: null, - languageOptions: { ecmaVersion: 2022 }, - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } - ] - }, - { - code: 'class C { static { let a; let b; ({ a, b } = foo); } }', - output: null, - languageOptions: { ecmaVersion: 2022 }, - errors: [ - { messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'b' }, type: 'Identifier' } - ] - }, - { - code: 'class C { static { let a; a = 0; console.log(a); } }', - output: null, - languageOptions: { ecmaVersion: 2022 }, - errors: [{ messageId: 'useConst', data: { name: 'a' }, type: 'Identifier' }] - }, - // https://github.com/eslint/eslint/issues/16266 - { - code: ` - let { itemId, list } = {}, - obj = [], - total = 0; - total = 9; - console.log(itemId, list, obj, total); - `, - output: null, - options: [{ destructuring: 'any', ignoreReadBeforeAssign: true }], - languageOptions: { ecmaVersion: 2022 }, - errors: [ - { messageId: 'useConst', data: { name: 'itemId' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'list' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'obj' }, type: 'Identifier' } - ] - }, - { - code: ` - let { itemId, list } = {}, - obj = []; - console.log(itemId, list, obj); - `, - output: ` - const { itemId, list } = {}, - obj = []; - console.log(itemId, list, obj); - `, - options: [{ destructuring: 'any', ignoreReadBeforeAssign: true }], - languageOptions: { ecmaVersion: 2022 }, - errors: [ - { messageId: 'useConst', data: { name: 'itemId' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'list' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'obj' }, type: 'Identifier' } - ] - }, - { - code: ` - let [ itemId, list ] = [], - total = 0; - total = 9; - console.log(itemId, list, total); - `, - output: null, - options: [{ destructuring: 'any', ignoreReadBeforeAssign: true }], - languageOptions: { ecmaVersion: 2022 }, - errors: [ - { messageId: 'useConst', data: { name: 'itemId' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'list' }, type: 'Identifier' } - ] - }, - { - code: ` - let [ itemId, list ] = [], - obj = []; - console.log(itemId, list, obj); - `, - output: ` - const [ itemId, list ] = [], - obj = []; - console.log(itemId, list, obj); - `, - options: [{ destructuring: 'any', ignoreReadBeforeAssign: true }], - languageOptions: { ecmaVersion: 2022 }, - errors: [ - { messageId: 'useConst', data: { name: 'itemId' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'list' }, type: 'Identifier' }, - { messageId: 'useConst', data: { name: 'obj' }, type: 'Identifier' } - ] - } - ] as InvalidTestCases -}; - -tester.run('prefer-const', rule as any, { - valid: [...ESLINT_RULE_TEST_CASES.valid, ...loadedCases.valid], - invalid: [...ESLINT_RULE_TEST_CASES.invalid, ...loadedCases.invalid] -}); +tester.run('prefer-const', rule as any, loadTestCases('prefer-const')); From 3a5c4a12acdbdf404a589bfbe93d0cbf8d9f05db Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Mon, 2 Dec 2024 08:21:16 +0100 Subject: [PATCH 17/22] chore(prefer-const): update resources --- README.md | 1 - docs/rules.md | 1 - packages/eslint-plugin-svelte/src/rule-types.ts | 10 ---------- packages/eslint-plugin-svelte/src/utils/rules.ts | 2 ++ 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index e492f8a76..2601875d0 100644 --- a/README.md +++ b/README.md @@ -367,7 +367,6 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-unused-class-name](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-class-name/) | disallow the use of a class in the template without a corresponding style | | | [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: | | [svelte/no-useless-mustaches](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: | -| [svelte/prefer-const](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/) | Require `const` declarations for variables that are never reassigned after declared | :wrench: | | [svelte/prefer-destructured-store-props](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/) | destructure values from object stores for better change tracking & fewer redraws | :bulb: | | [svelte/require-each-key](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/) | require keyed `{#each}` block | | | [svelte/require-event-dispatcher-types](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-event-dispatcher-types/) | require type parameters for `createEventDispatcher` | | diff --git a/docs/rules.md b/docs/rules.md index d2dd3d7a7..29701ff7e 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -64,7 +64,6 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-unused-class-name](./rules/no-unused-class-name.md) | disallow the use of a class in the template without a corresponding style | | | [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: | | [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :wrench: | -| [svelte/prefer-const](./rules/prefer-const.md) | Require `const` declarations for variables that are never reassigned after declared | :wrench: | | [svelte/prefer-destructured-store-props](./rules/prefer-destructured-store-props.md) | destructure values from object stores for better change tracking & fewer redraws | :bulb: | | [svelte/require-each-key](./rules/require-each-key.md) | require keyed `{#each}` block | | | [svelte/require-event-dispatcher-types](./rules/require-event-dispatcher-types.md) | require type parameters for `createEventDispatcher` | | diff --git a/packages/eslint-plugin-svelte/src/rule-types.ts b/packages/eslint-plugin-svelte/src/rule-types.ts index 71eae844b..8b7cbe80c 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -264,11 +264,6 @@ export interface RuleOptions { * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-class-directive/ */ 'svelte/prefer-class-directive'?: Linter.RuleEntry - /** - * Require `const` declarations for variables that are never reassigned after declared - * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/ - */ - 'svelte/prefer-const'?: Linter.RuleEntry /** * destructure values from object stores for better change tracking & fewer redraws * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/ @@ -490,11 +485,6 @@ type SvelteNoUselessMustaches = []|[{ type SveltePreferClassDirective = []|[{ prefer?: ("always" | "empty") }] -// ----- svelte/prefer-const ----- -type SveltePreferConst = []|[{ - destructuring?: ("any" | "all") - ignoreReadBeforeAssign?: boolean -}] // ----- svelte/shorthand-attribute ----- type SvelteShorthandAttribute = []|[{ prefer?: ("always" | "never") diff --git a/packages/eslint-plugin-svelte/src/utils/rules.ts b/packages/eslint-plugin-svelte/src/utils/rules.ts index 296a7700d..bba9a9975 100644 --- a/packages/eslint-plugin-svelte/src/utils/rules.ts +++ b/packages/eslint-plugin-svelte/src/utils/rules.ts @@ -52,6 +52,7 @@ import noUnusedClassName from '../rules/no-unused-class-name.js'; import noUnusedSvelteIgnore from '../rules/no-unused-svelte-ignore.js'; import noUselessMustaches from '../rules/no-useless-mustaches.js'; import preferClassDirective from '../rules/prefer-class-directive.js'; +import preferConst from '../rules/prefer-const.js'; import preferDestructuredStoreProps from '../rules/prefer-destructured-store-props.js'; import preferStyleDirective from '../rules/prefer-style-directive.js'; import requireEachKey from '../rules/require-each-key.js'; @@ -120,6 +121,7 @@ export const rules = [ noUnusedSvelteIgnore, noUselessMustaches, preferClassDirective, + preferConst, preferDestructuredStoreProps, preferStyleDirective, requireEachKey, From 2c18bb538d3198c5f5080acb0ac1d3ffb36ad937 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Mon, 2 Dec 2024 08:57:48 +0100 Subject: [PATCH 18/22] fix(prefer-const): use `.js` when importing --- packages/eslint-plugin-svelte/src/rules/prefer-const.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index 91ff58931..7fcc7bacf 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -1,7 +1,7 @@ import { createRule } from '../utils'; import type { TSESTree } from '@typescript-eslint/types'; -import { defineWrapperListener, getCoreRule } from '../utils/eslint-core'; +import { defineWrapperListener, getCoreRule } from '../utils/eslint-core.js'; const coreRule = getCoreRule('prefer-const'); From 99852a9129e91dafa4d9e64c8815de949960458c Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Mon, 2 Dec 2024 09:04:51 +0100 Subject: [PATCH 19/22] fix(prefer-const): remove `$state` from excluded runes --- packages/eslint-plugin-svelte/src/rules/prefer-const.ts | 4 ++-- .../fixtures/rules/prefer-const/invalid/test01-errors.yaml | 4 ++++ .../fixtures/rules/prefer-const/invalid/test01-output.svelte | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index 7fcc7bacf..282481e3b 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -1,6 +1,6 @@ -import { createRule } from '../utils'; import type { TSESTree } from '@typescript-eslint/types'; +import { createRule } from '../utils/index.js'; import { defineWrapperListener, getCoreRule } from '../utils/eslint-core.js'; const coreRule = getCoreRule('prefer-const'); @@ -31,7 +31,7 @@ function shouldSkipDeclaration(declaration: TSESTree.Expression | null) { return false; } - if (callee.type === 'Identifier' && ['$state', '$props', '$derived'].includes(callee.name)) { + if (callee.type === 'Identifier' && ['$props', '$derived'].includes(callee.name)) { return true; } diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml index 5e2a52173..4d0f6d413 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml @@ -2,6 +2,10 @@ line: 3 column: 6 suggestions: null +- message: "'state' is never reassigned. Use 'const' instead." + line: 4 + column: 6 + suggestions: null - message: "'doubled' is never reassigned. Use 'const' instead." line: 6 column: 6 diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte index 97d178239..98ff12100 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte @@ -1,7 +1,7 @@ ``` - - ## :wrench: Options ```json diff --git a/packages/eslint-plugin-svelte/src/rule-types.ts b/packages/eslint-plugin-svelte/src/rule-types.ts index 8b7cbe80c..71eae844b 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -264,6 +264,11 @@ export interface RuleOptions { * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-class-directive/ */ 'svelte/prefer-class-directive'?: Linter.RuleEntry + /** + * Require `const` declarations for variables that are never reassigned after declared + * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/ + */ + 'svelte/prefer-const'?: Linter.RuleEntry /** * destructure values from object stores for better change tracking & fewer redraws * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/ @@ -485,6 +490,11 @@ type SvelteNoUselessMustaches = []|[{ type SveltePreferClassDirective = []|[{ prefer?: ("always" | "empty") }] +// ----- svelte/prefer-const ----- +type SveltePreferConst = []|[{ + destructuring?: ("any" | "all") + ignoreReadBeforeAssign?: boolean +}] // ----- svelte/shorthand-attribute ----- type SvelteShorthandAttribute = []|[{ prefer?: ("always" | "never") From b8230a03219ba837d4bbd252524e121626ba6301 Mon Sep 17 00:00:00 2001 From: Miquel de Domingo Date: Sat, 28 Dec 2024 08:25:59 +0100 Subject: [PATCH 21/22] fix(prefer-const): remove `state.raw` check --- packages/eslint-plugin-svelte/src/rules/prefer-const.ts | 7 ------- .../fixtures/rules/prefer-const/invalid/test01-errors.yaml | 4 ++++ .../rules/prefer-const/invalid/test01-output.svelte | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index 282481e3b..8f4e27c6b 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -46,13 +46,6 @@ function shouldSkipDeclaration(declaration: TSESTree.Expression | null) { ) { return true; } - if ( - callee.object.name === '$state' && - callee.property.type === 'Identifier' && - callee.property.name === 'raw' - ) { - return true; - } return false; } diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml index 4d0f6d413..fed423c4a 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-errors.yaml @@ -6,6 +6,10 @@ line: 4 column: 6 suggestions: null +- message: "'raw' is never reassigned. Use 'const' instead." + line: 5 + column: 6 + suggestions: null - message: "'doubled' is never reassigned. Use 'const' instead." line: 6 column: 6 diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte index 98ff12100..fb6da26ca 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte @@ -2,7 +2,7 @@ let { prop1, prop2 } = $props(); const zero = 0; const state = $state(0); - let raw = $state.raw(0); + const raw = $state.raw(0); const doubled = state * 2; let derived = $derived(state * 2); const calculated = calc(); From 54138862b6d50514933bc554ad90942c43aed007 Mon Sep 17 00:00:00 2001 From: baseballyama Date: Mon, 30 Dec 2024 11:58:56 +0900 Subject: [PATCH 22/22] tidy --- docs/rules/prefer-const.md | 12 +++++++++--- .../eslint-plugin-svelte/src/rules/prefer-const.ts | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/rules/prefer-const.md b/docs/rules/prefer-const.md index 2230e342c..f24b70fe2 100644 --- a/docs/rules/prefer-const.md +++ b/docs/rules/prefer-const.md @@ -14,7 +14,7 @@ description: 'Require `const` declarations for variables that are never reassign ## :book: Rule Details -This rule reports the same as the base ESLint `prefer-const` rule, except that ignores Svelte reactive values such as `$state`, `$derived` and `$props`. If this rule is active, make sure to disable the base `prefer-const` rule, as it will conflict with this rule. +This rule reports the same as the base ESLint `prefer-const` rule, except that ignores Svelte reactive values such as `$derived` and `$props`. If this rule is active, make sure to disable the base `prefer-const` rule, as it will conflict with this rule. @@ -24,12 +24,18 @@ This rule reports the same as the base ESLint `prefer-const` rule, except that i // ✓ GOOD const { a, b } = $props(); - let { a, b } = $state(); + let c = $state(''); + let d = $derived(a * 2); + let e = $derived.by(() => b * 2); // ✗ BAD - // Imagine obj is not re-assigned, therefore it should be constant let obj = { a, b }; + let g = $state(0); + let h = $state({ count: 1 }); + + + ``` ## :wrench: Options diff --git a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts index 8f4e27c6b..0d079b1e1 100644 --- a/packages/eslint-plugin-svelte/src/rules/prefer-const.ts +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -19,7 +19,7 @@ function findDeclarationCallee(node: TSESTree.Expression) { /** * Determines if a declaration should be skipped in the const preference analysis. - * Specifically checks for Svelte's state management utilities ($state, $props, $derived). + * Specifically checks for Svelte's state management utilities ($props, $derived). */ function shouldSkipDeclaration(declaration: TSESTree.Expression | null) { if (!declaration) {