fix: false positives for bind: with member in svelte/no-immutable-reactive-statements rule (#585)

Author: ota-meshi

.changeset/kind-baboons-judge.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-svelte": patch
+---
+
+fix: false positives for `bind:` with member in `svelte/no-immutable-reactive-statements` rule

src/rules/html-quotes.ts

@@ -192,8 +192,8 @@ export default createRule('html-quotes', {
 					return;
 				}
 				if (
-					node.key.range[0] <= node.expression.range![0] &&
-					node.expression.range![1] <= node.key.range[1]
+					node.key.range[0] <= node.expression.range[0] &&
+					node.expression.range[1] <= node.key.range[1]
 				) {
 					// shorthand
 					return;

src/rules/no-immutable-reactive-statements.ts

@@ -2,6 +2,7 @@ import type { AST } from 'svelte-eslint-parser';
 import { createRule } from '../utils';
 import type { Scope, Variable, Reference, Definition } from '@typescript-eslint/scope-manager';
 import type { TSESTree } from '@typescript-eslint/types';
+import { findVariable, iterateIdentifiers } from '../utils/ast-utils';
 
 export default createRule('no-immutable-reactive-statements', {
 	meta: {
@@ -104,31 +105,49 @@ export default createRule('no-immutable-reactive-statements', {
 				) {
 					return true;
 				}
-				if (isMutableMember(reference.identifier)) {
+				if (hasWriteMember(reference.identifier)) {
 					return true;
 				}
 			}
 			return false;
+		}
 
-			function isMutableMember(
-				expr: TSESTree.Identifier | TSESTree.JSXIdentifier | TSESTree.MemberExpression
-			): boolean {
-				if (expr.type === 'JSXIdentifier') return false;
-				const parent = expr.parent;
-				if (parent.type === 'AssignmentExpression') {
-					return parent.left === expr;
-				}
-				if (parent.type === 'UpdateExpression') {
-					return parent.argument === expr;
-				}
-				if (parent.type === 'UnaryExpression') {
-					return parent.operator === 'delete' && parent.argument === expr;
-				}
-				if (parent.type === 'MemberExpression') {
-					return parent.object === expr && isMutableMember(parent);
-				}
-				return false;
+		/** Checks whether the given expression has writing to a member or not. */
+		function hasWriteMember(
+			expr: TSESTree.Identifier | TSESTree.JSXIdentifier | TSESTree.MemberExpression
+		): boolean {
+			if (expr.type === 'JSXIdentifier') return false;
+			const parent = expr.parent as TSESTree.Node | AST.SvelteNode;
+			if (parent.type === 'AssignmentExpression') {
+				return parent.left === expr;
+			}
+			if (parent.type === 'UpdateExpression') {
+				return parent.argument === expr;
+			}
+			if (parent.type === 'UnaryExpression') {
+				return parent.operator === 'delete' && parent.argument === expr;
+			}
+			if (parent.type === 'MemberExpression') {
+				return parent.object === expr && hasWriteMember(parent);
 			}
+			if (parent.type === 'SvelteDirective') {
+				return parent.kind === 'Binding' && parent.expression === expr;
+			}
+			if (parent.type === 'SvelteEachBlock') {
+				return parent.expression === expr && hasWriteReference(parent.context);
+			}
+
+			return false;
+		}
+
+		/** Checks whether the given pattern has writing or not. */
+		function hasWriteReference(pattern: TSESTree.DestructuringPattern): boolean {
+			for (const id of iterateIdentifiers(pattern)) {
+				const variable = findVariable(context, id);
+				if (variable && hasWrite(variable)) return true;
+			}
+
+			return false;
 		}
 
 		/**

src/utils/ast-utils.ts

@@ -230,6 +230,40 @@ export function findVariable(context: RuleContext, node: TSESTree.Identifier): V
 	// Remove the $ and search for the variable again, as it may be a store access variable.
 	return eslintUtils.findVariable(initialScope, node.name.slice(1));
 }
+/**
+ * Iterate the identifiers of a given pattern node.
+ */
+export function* iterateIdentifiers(
+	node: TSESTree.DestructuringPattern
+): Iterable<TSESTree.Identifier> {
+	const buffer = [node];
+	let pattern: TSESTree.DestructuringPattern | undefined;
+	while ((pattern = buffer.shift())) {
+		if (pattern.type === 'Identifier') {
+			yield pattern;
+		} else if (pattern.type === 'ArrayPattern') {
+			for (const element of pattern.elements) {
+				if (element) {
+					buffer.push(element);
+				}
+			}
+		} else if (pattern.type === 'ObjectPattern') {
+			for (const property of pattern.properties) {
+				if (property.type === 'Property') {
+					buffer.push(property.value as TSESTree.DestructuringPattern);
+				} else if (property.type === 'RestElement') {
+					buffer.push(property);
+				}
+			}
+		} else if (pattern.type === 'AssignmentPattern') {
+			buffer.push(pattern.left);
+		} else if (pattern.type === 'RestElement') {
+			buffer.push(pattern.argument);
+		} else if (pattern.type === 'MemberExpression') {
+			// noop
+		}
+	}
+}
 
 /**
  * Gets the scope for the current node
@@ -367,8 +401,8 @@ export function getMustacheTokens(
 		return null;
 	}
 	if (
-		node.key.range[0] <= node.expression.range![0] &&
-		node.expression.range![1] <= node.key.range[1]
+		node.key.range[0] <= node.expression.range[0] &&
+		node.expression.range[1] <= node.key.range[1]
 	) {
 		// shorthand
 		return null;

tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable-member-input.svelte renamed to tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable-member01-input.svelte

@@ -0,0 +1,7 @@
+<script>
+	const values = ['a', 'b'];
+	$: first = values[0];
+</script>
+
+{first}
+<input bind:value={values[0]} />

tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable-member02-wth-bind-input.svelte

@@ -0,0 +1,9 @@
+<script>
+	const values = ['a', 'b'];
+	$: first = values[0];
+</script>
+
+{first}
+{#each values as value}
+	<input bind:value />
+{/each}

tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable-member03-with-each-input.svelte

@@ -0,0 +1,11 @@
+<script>
+	const values = [['a'], ['b']];
+	$: first = values[0][0];
+</script>
+
+{first}
+{#each values as a}
+	{#each a as value}
+		<input bind:value />
+	{/each}
+{/each}

tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable-member04-with-each-input.svelte

@@ -0,0 +1,10 @@
+<script>
+	const values = ['a', 'b'];
+	$: first = values[0];
+</script>
+
+{first}
+{values}
+{#each values as value}
+	<button on:click={() => (value = 'foo')}>Foo</button>
+{/each}

tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable-member05-with-each-input.svelte

@@ -0,0 +1,9 @@
+<script>
+	const values = [{ a: 'a' }, { a: 'b' }];
+	$: first = values[0].a;
+</script>
+
+{first}
+{#each values as { a }}
+	<input bind:value={a} />
+{/each}