diff --git a/.changeset/tricky-windows-brush.md b/.changeset/tricky-windows-brush.md new file mode 100644 index 000000000..2d12d1ed9 --- /dev/null +++ b/.changeset/tricky-windows-brush.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-svelte': minor +--- + +Add svelte/prefer-const rule that excludes reactive variables diff --git a/README.md b/README.md index 9a6c25600..dc1d85ad6 100644 --- a/README.md +++ b/README.md @@ -425,6 +425,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 | :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 7f115da5c..e605cc44b 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -62,6 +62,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 | :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..d14ebffcf --- /dev/null +++ b/docs/rules/prefer-const.md @@ -0,0 +1,22 @@ +--- +pageClass: 'rule-details' +sidebarDepth: 0 +title: 'svelte/prefer-const' +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 + +- :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 + +Based on https://eslint.org/docs/latest/rules/prefer-const but skips reactive variables created by runes. + +## :mag: Implementation + +- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/src/rules/prefer-const.ts) +- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/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 77d3c4ca4..c25acf99d 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -249,6 +249,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/ @@ -460,6 +465,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..63ea21d8a --- /dev/null +++ b/packages/eslint-plugin-svelte/src/rules/prefer-const.ts @@ -0,0 +1,386 @@ +import type { TSESTree } from '@typescript-eslint/types'; +import { createRule } from '../utils'; +import type { RuleFixer, SourceCode } from '../types'; +import type { AST } from 'svelte-eslint-parser'; + +/** + * Skip let statements when they are reactive values created by runes. + */ +function skipReactiveValues(indentifier: TSESTree.Identifier) { + const declarator = indentifier.parent; + if (declarator.type === 'VariableDeclarator' && declarator.init?.type === 'CallExpression') { + const callee = declarator.init.callee; + if (callee.type === 'Identifier' && ['$state', '$props', '$derived'].includes(callee.name)) { + return false; + } + if (callee.type === 'MemberExpression' && callee.object.type === 'Identifier') { + 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 | AST.Range; + + public constructor(fixer: RuleFixer, sourceCode: SourceCode) { + this.fixer = fixer; + this.sourceCode = sourceCode; + this.retainedRange = null; + } + + public retainRange(range: AST.Range) { + this.retainedRange = range; + return this; + } + + public replaceTextRange(range: AST.Range, text: string) { + let actualRange: AST.Range; + if (this.retainedRange) { + actualRange = [ + Math.min(this.retainedRange[0], range[0]), + Math.max(this.retainedRange[1], range[1]) + ]; + } else { + actualRange = range; + } + return this.fixer.replaceTextRange( + actualRange, + this.sourceCode.text.slice(actualRange[0], range[0]) + + text + + this.sourceCode.text.slice(range[1], actualRange[1]) + ); + } +} +/* eslint @typescript-eslint/no-explicit-any: off -- Thoroughly tested by ESLint, but sadly they don't use TypeScript */ +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 options = context.options[0] || {}; + const sourceCode = (context as any).sourceCode; + + const shouldMatchAnyDestructuredVariable = options.destructuring !== 'all'; + const ignoreReadBeforeAssign = options.ignoreReadBeforeAssign === true; + const variables: any[] = []; + let reportCount = 0; + let checkedId: any = null; + let checkedName = ''; + + function checkGroup(nodes: any) { + const nodesToReport = nodes.filter(Boolean); + if ( + nodes.length && + (shouldMatchAnyDestructuredVariable || nodesToReport.length === nodes.length) + ) { + const varDeclParent = findUp(nodes[0], 'VariableDeclaration', (parentNode: any) => + parentNode.type.endsWith('Statement') + ); + const isVarDecParentNull = varDeclParent === null; + if (!isVarDecParentNull && varDeclParent.declarations.length > 0) { + const firstDeclaration = varDeclParent.declarations[0]; + if (firstDeclaration.init) { + const firstDecParent = firstDeclaration.init.parent; + if (firstDecParent.type === 'VariableDeclarator') { + if (firstDecParent.id.name !== checkedName) { + checkedName = firstDecParent.id.name; + reportCount = 0; + } + if (firstDecParent.id.type === 'ObjectPattern') { + if (firstDecParent.init.name !== checkedName) { + checkedName = firstDecParent.init.name; + reportCount = 0; + } + } + if (firstDecParent.id !== checkedId) { + checkedId = firstDecParent.id; + reportCount = 0; + } + } + } + } + let shouldFix = + varDeclParent && + (varDeclParent.parent.type === 'ForInStatement' || + varDeclParent.parent.type === 'ForOfStatement' || + varDeclParent.declarations.every((declaration: any) => declaration.init)) && + nodesToReport.length === nodes.length; + if ( + !isVarDecParentNull && + varDeclParent.declarations && + varDeclParent.declarations.length !== 1 + ) { + if ( + varDeclParent && + varDeclParent.declarations && + varDeclParent.declarations.length >= 1 + ) { + reportCount += nodesToReport.length; + let totalDeclarationsCount = 0; + varDeclParent.declarations.forEach((declaration: any) => { + if (declaration.id.type === 'ObjectPattern') { + totalDeclarationsCount += declaration.id.properties.length; + } else if (declaration.id.type === 'ArrayPattern') { + totalDeclarationsCount += declaration.id.elements.length; + } else { + totalDeclarationsCount += 1; + } + }); + shouldFix = shouldFix && reportCount === totalDeclarationsCount; + } + } + nodesToReport.filter(skipReactiveValues).forEach((node: any) => { + context.report({ + node, + messageId: 'useConst', + data: node, + fix: shouldFix + ? (fixer) => { + const letKeywordToken = sourceCode.getFirstToken( + varDeclParent, + (t: any) => t.value === varDeclParent.kind + ); + return new FixTracker(fixer, sourceCode) + .retainRange(varDeclParent.range) + .replaceTextRange(letKeywordToken.range, 'const'); + } + : null + }); + }); + } + } + + return { + 'Program:exit'() { + groupByDestructuring(variables, ignoreReadBeforeAssign).forEach(checkGroup); + }, + VariableDeclaration(node) { + if (node.kind === 'let' && !isInitOfForStatement(node)) { + variables.push(...sourceCode.getDeclaredVariables(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; + +function isInitOfForStatement(node: any) { + return node.parent.type === 'ForStatement' && node.parent.init === node; +} + +function canBecomeVariableDeclaration(identifier: any) { + let node = identifier.parent; + while (PATTERN_TYPE.test(node.type)) { + node = node.parent; + } + return ( + node.type === 'VariableDeclarator' || + (node.type === 'AssignmentExpression' && + node.parent.type === 'ExpressionStatement' && + DECLARATION_HOST_TYPE.test(node.parent.parent.type)) + ); +} + +function isOuterVariableInDestructing(name: any, initScope: any) { + if (initScope.through.some((ref: any) => ref.resolved && ref.resolved.name === name)) { + return true; + } + const variable = astUtilsGetVariableByName(initScope, name); + if (variable !== null) { + return variable.defs.some((def: any) => def.type === 'Parameter'); + } + return false; +} + +function getDestructuringHost(reference: any) { + if (!reference.isWrite()) { + return null; + } + let node = reference.identifier.parent; + while (PATTERN_TYPE.test(node.type)) { + node = node.parent; + } + if (!DESTRUCTURING_HOST_TYPE.test(node.type)) { + return null; + } + return node; +} + +function hasMemberExpressionAssignment(node: any) { + switch (node.type) { + case 'ObjectPattern': + return node.properties.some((prop: any) => { + if (prop) { + return hasMemberExpressionAssignment(prop.argument || prop.value); + } + return false; + }); + case 'ArrayPattern': + return node.elements.some((element: any) => { + if (element) { + return hasMemberExpressionAssignment(element); + } + return false; + }); + case 'AssignmentPattern': + return hasMemberExpressionAssignment(node.left); + case 'MemberExpression': + return true; + // no default + } + return false; +} + +function getIdentifierIfShouldBeConst(variable: any, ignoreReadBeforeAssign: any) { + 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); + if (destructuringHost !== null && destructuringHost.left !== undefined) { + const leftNode = destructuringHost.left; + let hasOuterVariables = false; + let hasNonIdentifiers = false; + if (leftNode.type === 'ObjectPattern') { + const properties = leftNode.properties; + hasOuterVariables = properties + .filter((prop: any) => prop.value) + .map((prop: any) => prop.value.name) + .some((name: any) => isOuterVariableInDestructing(name, variable.scope)); + hasNonIdentifiers = hasMemberExpressionAssignment(leftNode); + } else if (leftNode.type === 'ArrayPattern') { + const elements = leftNode.elements; + hasOuterVariables = elements + .map((element: any) => element && element.name) + .some((name: any) => isOuterVariableInDestructing(name, variable.scope)); + hasNonIdentifiers = hasMemberExpressionAssignment(leftNode); + } + if (hasOuterVariables || hasNonIdentifiers) { + 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; +} + +function groupByDestructuring(variables: any, ignoreReadBeforeAssign: any) { + 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 = null; + for (let j = 0; j < references.length; ++j) { + const reference = references[j]; + const id = reference.identifier; + if (id === prevId) { + continue; + } + prevId = id; + const group = getDestructuringHost(reference); + if (group) { + if (identifierMap.has(group)) { + identifierMap.get(group).push(identifier); + } else { + identifierMap.set(group, [identifier]); + } + } + } + } + return identifierMap; +} + +function findUp(node: any, type: any, shouldStop: any) { + if (!node || shouldStop(node)) { + return null; + } + if (node.type === type) { + return node; + } + return findUp(node.parent, type, shouldStop); +} + +function astUtilsGetVariableByName(initScope: any, name: any) { + let scope = 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/utils/rules.ts b/packages/eslint-plugin-svelte/src/utils/rules.ts index af0dd20e6..9b51e2704 100644 --- a/packages/eslint-plugin-svelte/src/utils/rules.ts +++ b/packages/eslint-plugin-svelte/src/utils/rules.ts @@ -49,6 +49,7 @@ import noUnusedClassName from '../rules/no-unused-class-name'; import noUnusedSvelteIgnore from '../rules/no-unused-svelte-ignore'; import noUselessMustaches from '../rules/no-useless-mustaches'; import preferClassDirective from '../rules/prefer-class-directive'; +import preferConst from '../rules/prefer-const'; import preferDestructuredStoreProps from '../rules/prefer-destructured-store-props'; import preferStyleDirective from '../rules/prefer-style-directive'; import requireEachKey from '../rules/require-each-key'; @@ -114,6 +115,7 @@ export const rules = [ noUnusedSvelteIgnore, noUselessMustaches, preferClassDirective, + preferConst, preferDestructuredStoreProps, preferStyleDirective, requireEachKey, 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..62a293b62 --- /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: 2 + column: 6 + suggestions: null +- message: "'doubled' is never reassigned. Use 'const' instead." + line: 5 + column: 6 + suggestions: null +- message: "'calculated' is never reassigned. Use 'const' instead." + line: 7 + 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..7efe16f1b --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-input.svelte @@ -0,0 +1,9 @@ + 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..53c4c67ca --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/invalid/test01-output.svelte @@ -0,0 +1,9 @@ + 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..53c4c67ca --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/prefer-const/valid/test01-input.svelte @@ -0,0 +1,9 @@ + 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'));