feat: add no-unnecessary-state-wrap rule

.changeset/popular-needles-remain.md

@@ -0,0 +1,5 @@
+---
+'eslint-plugin-svelte': minor
+---
+
+feat: add `no-unnecessary-state-wrap` rule

README.md

@@ -302,6 +302,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-props](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-props/) | Warns about defined Props properties that are unused | :star: |
 | [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |

docs/rules.md

@@ -59,6 +59,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-props](./rules/no-unused-props.md)                                     | Warns about defined Props properties that are unused                                                                                      | :star:         |
 | [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md)                     | disallow unused svelte-ignore comments                                                                                                    | :star:         |

docs/rules/no-unnecessary-state-wrap.md

@@ -0,0 +1,115 @@
+---
+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: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
+- :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.
+
+<!--eslint-skip-->
+
+```svelte
+<script>
+  /* eslint svelte/no-unnecessary-state-wrap: "error" */
+  import {
+    SvelteSet,
+    SvelteMap,
+    SvelteURL,
+    SvelteURLSearchParams,
+    SvelteDate,
+    MediaQuery
+  } from 'svelte/reactivity';
+
+  // ✓ GOOD
+  const set1 = new SvelteSet();
+  const map1 = new SvelteMap();
+  const url1 = new SvelteURL('https://example.com');
+  const params1 = new SvelteURLSearchParams('key=value');
+  const date1 = new SvelteDate();
+  const mediaQuery1 = new MediaQuery('(min-width: 800px)');
+
+  // ✗ BAD
+  const set2 = $state(new SvelteSet());
+  const map2 = $state(new SvelteMap());
+  const url2 = $state(new SvelteURL('https://example.com'));
+  const params2 = $state(new SvelteURLSearchParams('key=value'));
+  const date2 = $state(new SvelteDate());
+  const mediaQuery2 = $state(new MediaQuery('(min-width: 800px)'));
+</script>
+```
+
+## :wrench: Options
+
+```json
+{
+  "svelte/no-unnecessary-state-wrap": [
+    "error",
+    {
+      "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
+
+#### `additionalReactiveClasses`
+
+```svelte
+<script>
+  /* eslint svelte/no-unnecessary-state-wrap: ["error", { "additionalReactiveClasses": ["MyReactiveClass"] }] */
+  import { MyReactiveClass } from './foo';
+
+  // ✓ GOOD
+  const myState1 = new MyReactiveClass();
+
+  // ✗ BAD
+  const myState2 = $state(new MyReactiveClass());
+</script>
+```
+
+#### `allowReassign`
+
+```svelte
+<script>
+  /* eslint svelte/no-unnecessary-state-wrap: ["error", { "allowReassign": true }] */
+  import { SvelteSet } from 'svelte/reactivity';
+
+  // ✓ GOOD
+  let set1 = $state(new SvelteSet());
+  set1 = new SvelteSet([1, 2, 3]); // Variable is reassigned
+
+  // ✗ BAD
+  const set2 = $state(new SvelteSet()); // const cannot be reassigned
+  let set3 = $state(new SvelteSet()); // Variable is never reassigned
+</script>
+```
+
+## :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)

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-props': 'error',
 			'svelte/no-unused-svelte-ignore': 'error',
 			'svelte/no-useless-children-snippet': 'error',

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<SvelteNoUnknownStyleDirectiveProperty>
+  /**
+   * 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<SvelteNoUnnecessaryStateWrap>
   /**
    * 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/
@@ -518,6 +523,11 @@ type SvelteNoUnknownStyleDirectiveProperty = []|[{
   ignoreProperties?: [string, ...(string)[]]
   ignorePrefixed?: boolean
 }]
+// ----- svelte/no-unnecessary-state-wrap -----
+type SvelteNoUnnecessaryStateWrap = []|[{
+  additionalReactiveClasses?: string[]
+  allowReassign?: boolean
+}]
 // ----- svelte/no-unused-class-name -----
 type SvelteNoUnusedClassName = []|[{
   allowedClassNames?: string[]

packages/eslint-plugin-svelte/src/rules/no-unnecessary-state-wrap.ts

@@ -0,0 +1,156 @@
+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
+					},
+					allowReassign: {
+						type: 'boolean'
+					}
+				},
+				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 allowReassign = options.allowReassign ?? false;
+
+		const referenceTracker = new ReferenceTracker(getSourceCode(context).scopeManager.globalScope!);
+		const traceMap: Record<string, Record<string, boolean>> = {};
+		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 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,
+			identifier?: TSESTree.Identifier
+		) {
+			if (allowReassign && identifier && isReassigned(identifier)) {
+				return;
+			}
+
+			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, parent.id);
+							}
+						}
+					}
+				}
+			},
+
+			'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, parent.id);
+						}
+					}
+				}
+			}
+		};
+	}
+});

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 noUnusedProps from '../rules/no-unused-props.js';
 import noUnusedSvelteIgnore from '../rules/no-unused-svelte-ignore.js';
@@ -124,6 +125,7 @@ export const rules = [
 	noTargetBlank,
 	noTrailingSpaces,
 	noUnknownStyleDirectiveProperty,
+	noUnnecessaryStateWrap,
 	noUnusedClassName,
 	noUnusedProps,
 	noUnusedSvelteIgnore,

packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/_requirements.json

@@ -0,0 +1,3 @@
+{
+	"svelte": ">=5.0.0"
+}

packages/eslint-plugin-svelte/tests/fixtures/rules/no-unnecessary-state-wrap/invalid/additional-class-config.json

@@ -0,0 +1,7 @@
+{
+	"options": [
+		{
+			"additionalReactiveClasses": ["CustomReactiveClass1", "CustomReactiveClass2"]
+		}
+	]
+}

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: |
+        <script>
+        	import { CustomReactiveClass1, CustomReactiveClass2 } from 'foo';
+
+        	// These should be reported as unnecessary $state wrapping
+        	const custom1 = new CustomReactiveClass1();
+        	const custom2 = $state(new CustomReactiveClass2());
+
+        	// Regular state usage is still valid
+        	const regularState = $state(42);
+        </script>
+- message: CustomReactiveClass2 is already reactive, $state wrapping is unnecessary.
+  line: 6
+  column: 25
+  suggestions:
+    - desc: Remove unnecessary $state wrapping
+      messageId: suggestRemoveStateWrap
+      output: |
+        <script>
+        	import { CustomReactiveClass1, CustomReactiveClass2 } from 'foo';
+
+        	// These should be reported as unnecessary $state wrapping
+        	const custom1 = $state(new CustomReactiveClass1());
+        	const custom2 = new CustomReactiveClass2();
+
+        	// Regular state usage is still valid
+        	const regularState = $state(42);
+        </script>