From 1640da208990daff7f03971310908c678c603a56 Mon Sep 17 00:00:00 2001 From: baseballyama Date: Sun, 9 Feb 2025 17:22:48 +0900 Subject: [PATCH 1/2] feat: add `no-unnecessary-state-wrap` rule --- .changeset/popular-needles-remain.md | 5 + README.md | 1 + docs/rules.md | 1 + docs/rules/no-unnecessary-state-wrap.md | 96 ++++++++++ .../src/configs/flat/recommended.ts | 1 + .../eslint-plugin-svelte/src/rule-types.ts | 9 + .../src/rules/no-unnecessary-state-wrap.ts | 138 ++++++++++++++ .../eslint-plugin-svelte/src/utils/rules.ts | 2 + .../invalid/_requirements.json | 3 + .../invalid/additional-class-config.json | 7 + .../invalid/additional-class-errors.yaml | 34 ++++ .../invalid/additional-class-input.svelte | 10 + .../invalid/basic-errors.yaml | 174 ++++++++++++++++++ .../invalid/basic-input.svelte | 22 +++ .../invalid/import-alias-errors.yaml | 28 +++ .../invalid/import-alias-input.svelte | 7 + .../valid/_requirements.json | 3 + .../valid/additional-class-config.json | 7 + .../valid/additional-class-input.svelte | 9 + .../valid/basic-input.svelte | 22 +++ .../src/rules/no-unnecessary-state-wrap.ts | 12 ++ 21 files changed, 591 insertions(+) create mode 100644 .changeset/popular-needles-remain.md create mode 100644 docs/rules/no-unnecessary-state-wrap.md create mode 100644 packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/_requirements.json create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-config.json create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/basic-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/basic-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/import-alias-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/import-alias-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/_requirements.json create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/additional-class-config.json create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/additional-class-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/basic-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/src/rules/no-unnecessary-state-wrap.ts diff --git a/.changeset/popular-needles-remain.md b/.changeset/popular-needles-remain.md new file mode 100644 index 000000000..9fc497018 --- /dev/null +++ b/.changeset/popular-needles-remain.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-svelte': minor +--- + +feat: add `no-unnecessary-state-wrap` rule diff --git a/README.md b/README.md index 01eb6bdfa..13f402de5 100644 --- a/README.md +++ b/README.md @@ -361,6 +361,7 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-reactive-functions](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | it's not necessary to define functions in reactive statements | :star::bulb: | | [svelte/no-reactive-literals](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :star::bulb: | | [svelte/no-svelte-internal](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-svelte-internal/) | svelte/internal will be removed in Svelte 6. | :star: | +| [svelte/no-unnecessary-state-wrap](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unnecessary-state-wrap/) | Disallow unnecessary $state wrapping of reactive classes | :star::bulb: | | [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-children-snippet](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-useless-children-snippet/) | disallow explicit children snippet where it's not needed | :star: | diff --git a/docs/rules.md b/docs/rules.md index 5e509b01e..dfdd5c15b 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -58,6 +58,7 @@ These rules relate to better ways of doing things to help you avoid problems: | [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | it's not necessary to define functions in reactive statements | :star::bulb: | | [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :star::bulb: | | [svelte/no-svelte-internal](./rules/no-svelte-internal.md) | svelte/internal will be removed in Svelte 6. | :star: | +| [svelte/no-unnecessary-state-wrap](./rules/no-unnecessary-state-wrap.md) | Disallow unnecessary $state wrapping of reactive classes | :star::bulb: | | [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-children-snippet](./rules/no-useless-children-snippet.md) | disallow explicit children snippet where it's not needed | :star: | diff --git a/docs/rules/no-unnecessary-state-wrap.md b/docs/rules/no-unnecessary-state-wrap.md new file mode 100644 index 000000000..55404c6f0 --- /dev/null +++ b/docs/rules/no-unnecessary-state-wrap.md @@ -0,0 +1,96 @@ +--- +pageClass: 'rule-details' +sidebarDepth: 0 +title: 'svelte/no-unnecessary-state-wrap' +description: 'Disallow unnecessary $state wrapping of reactive classes' +--- + +# svelte/no-unnecessary-state-wrap + +> Disallow unnecessary $state wrapping of reactive classes + +- :exclamation: **_This rule has not been released yet._** +- :gear: This rule is included in `"plugin:svelte/recommended"`. +- :bulb: Some problems reported by this rule are manually fixable by editor [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). + +## :book: Rule Details + +In Svelte 5, several built-in classes from `svelte/reactivity` are already reactive by default: + +- `SvelteSet` +- `SvelteMap` +- `SvelteURL` +- `SvelteURLSearchParams` +- `SvelteDate` +- `MediaQuery` + +Therefore, wrapping them with `$state` is unnecessary and can lead to confusion. + + + +```svelte + +``` + +## :wrench: Options + +```json +{ + "svelte/no-unnecessary-state-wrap": [ + "error", + { + "additionalReactiveClasses": [] + } + ] +} +``` + +- `additionalReactiveClasses` ... An array of class names that should also be considered reactive. This is useful when you have custom classes that are inherently reactive. Default is `[]`. + +### Examples with Options + +#### `additionalReactiveClasses` + +```svelte + +``` + +## :mag: Implementation + +- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts) +- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/no-unnecessary-state-wrap.ts) diff --git a/packages/eslint-plugin-svelte/src/configs/flat/recommended.ts b/packages/eslint-plugin-svelte/src/configs/flat/recommended.ts index fc7f0890f..9906aead2 100644 --- a/packages/eslint-plugin-svelte/src/configs/flat/recommended.ts +++ b/packages/eslint-plugin-svelte/src/configs/flat/recommended.ts @@ -32,6 +32,7 @@ const config: Linter.Config[] = [ 'svelte/no-store-async': 'error', 'svelte/no-svelte-internal': 'error', 'svelte/no-unknown-style-directive-property': 'error', + 'svelte/no-unnecessary-state-wrap': 'error', 'svelte/no-unused-svelte-ignore': 'error', 'svelte/no-useless-children-snippet': 'error', 'svelte/no-useless-mustaches': 'error', diff --git a/packages/eslint-plugin-svelte/src/rule-types.ts b/packages/eslint-plugin-svelte/src/rule-types.ts index 30f15c1c2..a5e4769a0 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -256,6 +256,11 @@ export interface RuleOptions { * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unknown-style-directive-property/ */ 'svelte/no-unknown-style-directive-property'?: Linter.RuleEntry + /** + * Disallow unnecessary $state wrapping of reactive classes + * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unnecessary-state-wrap/ + */ + 'svelte/no-unnecessary-state-wrap'?: Linter.RuleEntry /** * disallow the use of a class in the template without a corresponding style * @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-class-name/ @@ -508,6 +513,10 @@ type SvelteNoUnknownStyleDirectiveProperty = []|[{ ignoreProperties?: [string, ...(string)[]] ignorePrefixed?: boolean }] +// ----- svelte/no-unnecessary-state-wrap ----- +type SvelteNoUnnecessaryStateWrap = []|[{ + additionalReactiveClasses?: string[] +}] // ----- svelte/no-unused-class-name ----- type SvelteNoUnusedClassName = []|[{ allowedClassNames?: string[] diff --git a/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts b/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts new file mode 100644 index 000000000..fbf743922 --- /dev/null +++ b/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts @@ -0,0 +1,138 @@ +import { createRule } from '../utils/index.js'; +import { getSourceCode } from '../utils/compat.js'; +import { ReferenceTracker } from '@eslint-community/eslint-utils'; +import type { TSESTree } from '@typescript-eslint/types'; + +const REACTIVE_CLASSES = [ + 'SvelteSet', + 'SvelteMap', + 'SvelteURL', + 'SvelteURLSearchParams', + 'SvelteDate', + 'MediaQuery' +]; + +export default createRule('no-unnecessary-state-wrap', { + meta: { + docs: { + description: 'Disallow unnecessary $state wrapping of reactive classes', + category: 'Best Practices', + recommended: true + }, + schema: [ + { + type: 'object', + properties: { + additionalReactiveClasses: { + type: 'array', + items: { + type: 'string' + }, + uniqueItems: true + } + }, + additionalProperties: false + } + ], + messages: { + unnecessaryStateWrap: '{{className}} is already reactive, $state wrapping is unnecessary.', + suggestRemoveStateWrap: 'Remove unnecessary $state wrapping' + }, + type: 'suggestion', + hasSuggestions: true, + conditions: [ + { + svelteVersions: ['5'], + runes: [true, 'undetermined'] + } + ] + }, + create(context) { + const options = context.options[0] ?? {}; + const additionalReactiveClasses = options.additionalReactiveClasses ?? []; + + const referenceTracker = new ReferenceTracker(getSourceCode(context).scopeManager.globalScope!); + const traceMap: Record> = {}; + for (const reactiveClass of REACTIVE_CLASSES) { + traceMap[reactiveClass] = { + [ReferenceTracker.CALL]: true, + [ReferenceTracker.CONSTRUCT]: true + }; + } + + // Track all reactive class imports and their aliases + const references = referenceTracker.iterateEsmReferences({ + 'svelte/reactivity': { + [ReferenceTracker.ESM]: true, + ...traceMap + } + }); + + const referenceNodeAndNames = Array.from(references).map(({ node, path }) => { + return { + node, + name: path[path.length - 1] + }; + }); + + function reportUnnecessaryStateWrap( + stateNode: TSESTree.Node, + targetNode: TSESTree.Node, + className: string + ) { + context.report({ + node: targetNode, + messageId: 'unnecessaryStateWrap', + data: { + className + }, + suggest: [ + { + messageId: 'suggestRemoveStateWrap', + fix(fixer) { + return fixer.replaceText(stateNode, getSourceCode(context).getText(targetNode)); + } + } + ] + }); + } + + return { + CallExpression(node: TSESTree.CallExpression) { + if (node.callee.type !== 'Identifier' || node.callee.name !== '$state') { + return; + } + + for (const arg of node.arguments) { + if ( + (arg.type === 'NewExpression' || arg.type === 'CallExpression') && + arg.callee.type === 'Identifier' + ) { + const name = arg.callee.name; + if (additionalReactiveClasses.includes(name)) { + const parent = node.parent; + if (parent?.type === 'VariableDeclarator' && parent.id.type === 'Identifier') { + reportUnnecessaryStateWrap(node, arg, name); + } + } + } + } + }, + + 'Program:exit': () => { + for (const { node, name } of referenceNodeAndNames) { + if ( + node.parent?.type === 'CallExpression' && + node.parent.callee.type === 'Identifier' && + node.parent.callee.name === '$state' + ) { + const parent = node.parent.parent; + if (parent?.type === 'VariableDeclarator' && parent.id.type === 'Identifier') { + reportUnnecessaryStateWrap(node.parent, node, name); + } + } + } + } + }; + } +}); diff --git a/packages/eslint-plugin-svelte/src/utils/rules.ts b/packages/eslint-plugin-svelte/src/utils/rules.ts index 2a2ddeeda..8789c42a2 100644 --- a/packages/eslint-plugin-svelte/src/utils/rules.ts +++ b/packages/eslint-plugin-svelte/src/utils/rules.ts @@ -50,6 +50,7 @@ import noSvelteInternal from '../rules/no-svelte-internal.js'; import noTargetBlank from '../rules/no-target-blank.js'; import noTrailingSpaces from '../rules/no-trailing-spaces.js'; import noUnknownStyleDirectiveProperty from '../rules/no-unknown-style-directive-property.js'; +import noUnnecessaryStateWrap from '../rules/no-unnecessary-state-wrap.js'; import noUnusedClassName from '../rules/no-unused-class-name.js'; import noUnusedSvelteIgnore from '../rules/no-unused-svelte-ignore.js'; import noUselessChildrenSnippet from '../rules/no-useless-children-snippet.js'; @@ -122,6 +123,7 @@ export const rules = [ noTargetBlank, noTrailingSpaces, noUnknownStyleDirectiveProperty, + noUnnecessaryStateWrap, noUnusedClassName, noUnusedSvelteIgnore, noUselessChildrenSnippet, diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/_requirements.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/_requirements.json new file mode 100644 index 000000000..498661308 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/_requirements.json @@ -0,0 +1,3 @@ +{ + "svelte": ">=5.0.0" +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-config.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-config.json new file mode 100644 index 000000000..8bde8d11e --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-config.json @@ -0,0 +1,7 @@ +{ + "options": [ + { + "additionalReactiveClasses": ["CustomReactiveClass1", "CustomReactiveClass2"] + } + ] +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-errors.yaml new file mode 100644 index 000000000..30518c2f7 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-errors.yaml @@ -0,0 +1,34 @@ +- message: CustomReactiveClass1 is already reactive, $state wrapping is unnecessary. + line: 5 + column: 25 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + +- message: CustomReactiveClass2 is already reactive, $state wrapping is unnecessary. + line: 6 + column: 25 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-input.svelte new file mode 100644 index 000000000..29aef1979 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-input.svelte @@ -0,0 +1,10 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/basic-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/basic-errors.yaml new file mode 100644 index 000000000..4c2f5b9bd --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/basic-errors.yaml @@ -0,0 +1,174 @@ +- message: SvelteSet is already reactive, $state wrapping is unnecessary. + line: 12 + column: 21 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + +- message: SvelteMap is already reactive, $state wrapping is unnecessary. + line: 13 + column: 21 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + +- message: SvelteURL is already reactive, $state wrapping is unnecessary. + line: 14 + column: 21 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + +- message: SvelteURLSearchParams is already reactive, $state wrapping is unnecessary. + line: 15 + column: 24 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + +- message: SvelteDate is already reactive, $state wrapping is unnecessary. + line: 16 + column: 22 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + +- message: MediaQuery is already reactive, $state wrapping is unnecessary. + line: 17 + column: 28 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/basic-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/basic-input.svelte new file mode 100644 index 000000000..5af31b886 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/basic-input.svelte @@ -0,0 +1,22 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/import-alias-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/import-alias-errors.yaml new file mode 100644 index 000000000..ecde6627d --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/import-alias-errors.yaml @@ -0,0 +1,28 @@ +- message: SvelteSet is already reactive, $state wrapping is unnecessary. + line: 5 + column: 21 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: > + +- message: SvelteMap is already reactive, $state wrapping is unnecessary. + line: 6 + column: 21 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: > + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/import-alias-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/import-alias-input.svelte new file mode 100644 index 000000000..b9ff44b60 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/import-alias-input.svelte @@ -0,0 +1,7 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/_requirements.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/_requirements.json new file mode 100644 index 000000000..498661308 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/_requirements.json @@ -0,0 +1,3 @@ +{ + "svelte": ">=5.0.0" +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/additional-class-config.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/additional-class-config.json new file mode 100644 index 000000000..8bde8d11e --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/additional-class-config.json @@ -0,0 +1,7 @@ +{ + "options": [ + { + "additionalReactiveClasses": ["CustomReactiveClass1", "CustomReactiveClass2"] + } + ] +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/additional-class-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/additional-class-input.svelte new file mode 100644 index 000000000..ac036cb9c --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/additional-class-input.svelte @@ -0,0 +1,9 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/basic-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/basic-input.svelte new file mode 100644 index 000000000..77798ee8e --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/basic-input.svelte @@ -0,0 +1,22 @@ + diff --git a/packages/eslint-plugin-svelte/tests/src/rules/no-unnecessary-state-wrap.ts b/packages/eslint-plugin-svelte/tests/src/rules/no-unnecessary-state-wrap.ts new file mode 100644 index 000000000..a4d8a3952 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/src/rules/no-unnecessary-state-wrap.ts @@ -0,0 +1,12 @@ +import { RuleTester } from '../../utils/eslint-compat.js'; +import rule from '../../../src/rules/no-unnecessary-state-wrap.js'; +import { loadTestCases } from '../../utils/utils.js'; + +const tester = new RuleTester({ + languageOptions: { + ecmaVersion: 2020, + sourceType: 'module' + } +}); + +tester.run('no-unnecessary-state-wrap', rule as any, loadTestCases('no-unnecessary-state-wrap')); From 17229aecdc88b9d468db42beda422aa65e51f9a3 Mon Sep 17 00:00:00 2001 From: baseballyama Date: Sun, 23 Feb 2025 23:02:33 +0900 Subject: [PATCH 2/2] add allowReassign option --- docs/rules/no-unnecessary-state-wrap.md | 21 ++++++++++++- .../eslint-plugin-svelte/src/rule-types.ts | 1 + .../src/rules/no-unnecessary-state-wrap.ts | 24 +++++++++++++-- .../invalid/allow-reassign-config.json | 7 +++++ .../invalid/allow-reassign-errors.yaml | 30 +++++++++++++++++++ .../invalid/allow-reassign-input.svelte | 8 +++++ .../valid/allow-reassign-config.json | 7 +++++ .../valid/allow-reassign-input.svelte | 10 +++++++ 8 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-config.json create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/allow-reassign-config.json create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/allow-reassign-input.svelte diff --git a/docs/rules/no-unnecessary-state-wrap.md b/docs/rules/no-unnecessary-state-wrap.md index 55404c6f0..8e10ae3ec 100644 --- a/docs/rules/no-unnecessary-state-wrap.md +++ b/docs/rules/no-unnecessary-state-wrap.md @@ -65,13 +65,15 @@ Therefore, wrapping them with `$state` is unnecessary and can lead to confusion. "svelte/no-unnecessary-state-wrap": [ "error", { - "additionalReactiveClasses": [] + "additionalReactiveClasses": [], + "allowReassign": false } ] } ``` - `additionalReactiveClasses` ... An array of class names that should also be considered reactive. This is useful when you have custom classes that are inherently reactive. Default is `[]`. +- `allowReassign` ... If `true`, allows `$state` wrapping of reactive classes when the variable is reassigned. Default is `false`. ### Examples with Options @@ -90,6 +92,23 @@ Therefore, wrapping them with `$state` is unnecessary and can lead to confusion. ``` +#### `allowReassign` + +```svelte + +``` + ## :mag: Implementation - [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts) diff --git a/packages/eslint-plugin-svelte/src/rule-types.ts b/packages/eslint-plugin-svelte/src/rule-types.ts index a5e4769a0..203655984 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -516,6 +516,7 @@ type SvelteNoUnknownStyleDirectiveProperty = []|[{ // ----- svelte/no-unnecessary-state-wrap ----- type SvelteNoUnnecessaryStateWrap = []|[{ additionalReactiveClasses?: string[] + allowReassign?: boolean }] // ----- svelte/no-unused-class-name ----- type SvelteNoUnusedClassName = []|[{ diff --git a/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts b/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts index fbf743922..6c1ff5ef7 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts @@ -29,6 +29,9 @@ export default createRule('no-unnecessary-state-wrap', { type: 'string' }, uniqueItems: true + }, + allowReassign: { + type: 'boolean' } }, additionalProperties: false @@ -50,6 +53,7 @@ export default createRule('no-unnecessary-state-wrap', { create(context) { const options = context.options[0] ?? {}; const additionalReactiveClasses = options.additionalReactiveClasses ?? []; + const allowReassign = options.allowReassign ?? false; const referenceTracker = new ReferenceTracker(getSourceCode(context).scopeManager.globalScope!); const traceMap: Record> = {}; @@ -75,11 +79,25 @@ export default createRule('no-unnecessary-state-wrap', { }; }); + function isReassigned(identifier: TSESTree.Identifier): boolean { + const variable = getSourceCode(context).scopeManager.getDeclaredVariables( + identifier.parent + )[0]; + return variable.references.some((ref) => { + return ref.isWrite() && ref.identifier !== identifier; + }); + } + function reportUnnecessaryStateWrap( stateNode: TSESTree.Node, targetNode: TSESTree.Node, - className: string + className: string, + identifier?: TSESTree.Identifier ) { + if (allowReassign && identifier && isReassigned(identifier)) { + return; + } + context.report({ node: targetNode, messageId: 'unnecessaryStateWrap', @@ -112,7 +130,7 @@ export default createRule('no-unnecessary-state-wrap', { if (additionalReactiveClasses.includes(name)) { const parent = node.parent; if (parent?.type === 'VariableDeclarator' && parent.id.type === 'Identifier') { - reportUnnecessaryStateWrap(node, arg, name); + reportUnnecessaryStateWrap(node, arg, name, parent.id); } } } @@ -128,7 +146,7 @@ export default createRule('no-unnecessary-state-wrap', { ) { const parent = node.parent.parent; if (parent?.type === 'VariableDeclarator' && parent.id.type === 'Identifier') { - reportUnnecessaryStateWrap(node.parent, node, name); + reportUnnecessaryStateWrap(node.parent, node, name, parent.id); } } } diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-config.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-config.json new file mode 100644 index 000000000..e68fc5b00 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-config.json @@ -0,0 +1,7 @@ +{ + "options": [ + { + "allowReassign": true + } + ] +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-errors.yaml new file mode 100644 index 000000000..536b5f9c6 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-errors.yaml @@ -0,0 +1,30 @@ +- message: SvelteSet is already reactive, $state wrapping is unnecessary. + line: 6 + column: 21 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + +- message: SvelteMap is already reactive, $state wrapping is unnecessary. + line: 7 + column: 19 + suggestions: + - desc: Remove unnecessary $state wrapping + messageId: suggestRemoveStateWrap + output: | + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-input.svelte new file mode 100644 index 000000000..85a1b156b --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/allow-reassign-input.svelte @@ -0,0 +1,8 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/allow-reassign-config.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/allow-reassign-config.json new file mode 100644 index 000000000..e68fc5b00 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/allow-reassign-config.json @@ -0,0 +1,7 @@ +{ + "options": [ + { + "allowReassign": true + } + ] +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/allow-reassign-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/allow-reassign-input.svelte new file mode 100644 index 000000000..0f882919d --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/valid/allow-reassign-input.svelte @@ -0,0 +1,10 @@ +