[fix]: Warn user when binding rest operator (#7526)

* Fix 6860: Warn user when binding rest operator

* move the binding validation to Binding node

* update test

Co-authored-by: vaibhav rai <vaibhavrai@vaibhavs-MacBook-Pro.local>

* add more test case, supporting deep destructuring and array destructuring

Co-authored-by: vaibhav rai <vaibhavrai@vaibhavs-MacBook-Pro.local>
Co-authored-by: tanhauhau <lhtan93@gmail.com>

src/compiler/compile/compiler_warnings.ts

@@ -186,5 +186,9 @@ export default {
 	redundant_event_modifier_passive: {
 		code: 'redundant-event-modifier',
 		message: 'The passive modifier only works with wheel and touch events'
-	}
+	},
+	invalid_rest_eachblock_binding: (rest_element_name: string) => ({
+		code: 'invalid-rest-eachblock-binding',
+		message: `...${rest_element_name} operator will create a new object and binding propogation with original object will not work`
+	})
 };

src/compiler/compile/nodes/AwaitBlock.ts

@@ -23,6 +23,8 @@ export default class AwaitBlock extends Node {
 	then: ThenBlock;
 	catch: CatchBlock;
 
+	context_rest_properties: Map<string, ESTreeNode> = new Map();
+
 	constructor(component: Component, parent: Node, scope: TemplateScope, info: TemplateNode) {
 		super(component, parent, scope, info);
 
@@ -33,12 +35,12 @@ export default class AwaitBlock extends Node {
 
 		if (this.then_node) {
 			this.then_contexts = [];
-			unpack_destructuring({ contexts: this.then_contexts, node: info.value, scope, component });
+			unpack_destructuring({ contexts: this.then_contexts, node: info.value, scope, component, context_rest_properties: this.context_rest_properties });
 		}
 
 		if (this.catch_node) {
 			this.catch_contexts = [];
-			unpack_destructuring({ contexts: this.catch_contexts, node: info.error, scope, component });
+			unpack_destructuring({ contexts: this.catch_contexts, node: info.error, scope, component, context_rest_properties: this.context_rest_properties });
 		}
 
 		this.pending = new PendingBlock(component, this, scope, info.pending);

src/compiler/compile/nodes/Binding.ts

@@ -11,6 +11,7 @@ import InlineComponent from './InlineComponent';
 import Window from './Window';
 import { clone } from '../../utils/clone';
 import compiler_errors from '../compiler_errors';
+import compiler_warnings from '../compiler_warnings';
 
 // TODO this should live in a specific binding
 const read_only_media_attributes = new Set([
@@ -47,6 +48,7 @@ export default class Binding extends Node {
 		const { name } = get_object(this.expression.node);
 
 		this.is_contextual = Array.from(this.expression.references).some(name => scope.names.has(name));
+		if (this.is_contextual) this.validate_binding_rest_properties(scope);
 
 		// make sure we track this as a mutable ref
 		if (scope.is_let(name)) {
@@ -95,6 +97,18 @@ export default class Binding extends Node {
 	is_readonly_media_attribute() {
 		return read_only_media_attributes.has(this.name);
 	}
+
+	validate_binding_rest_properties(scope: TemplateScope) {
+		this.expression.references.forEach(name => {
+			const each_block = scope.get_owner(name);
+			if (each_block && each_block.type === 'EachBlock') {
+				const rest_node = each_block.context_rest_properties.get(name);
+				if (rest_node) {
+					this.component.warn(rest_node as any, compiler_warnings.invalid_rest_eachblock_binding(name));
+				}
+			}
+		});
+	}
 }
 
 function isElement(node: Node): node is Element {

src/compiler/compile/nodes/ConstTag.ts

@@ -10,6 +10,7 @@ import { extract_identifiers } from 'periscopic';
 import is_reference, { NodeWithPropertyDefinition } from 'is-reference';
 import get_object from '../utils/get_object';
 import compiler_errors from '../compiler_errors';
+import { Node as ESTreeNode } from 'estree';
 
 const allowed_parents = new Set(['EachBlock', 'CatchBlock', 'ThenBlock', 'InlineComponent', 'SlotTemplate', 'IfBlock', 'ElseBlock']);
 
@@ -19,6 +20,7 @@ export default class ConstTag extends Node {
 	contexts: Context[] = [];
 	node: ConstTagType;
 	scope: TemplateScope;
+	context_rest_properties: Map<string, ESTreeNode> = new Map();
 
 	assignees: Set<string> = new Set();
   dependencies: Set<string> = new Set();
@@ -58,7 +60,8 @@ export default class ConstTag extends Node {
 			contexts: this.contexts,
 			node: this.node.expression.left,
 			scope: this.scope,
-			component: this.component
+			component: this.component,
+			context_rest_properties: this.context_rest_properties
 		});
 		this.expression = new Expression(this.component, this, this.scope, this.node.expression.right);
 		this.contexts.forEach(context => {

src/compiler/compile/nodes/EachBlock.ts

@@ -28,7 +28,7 @@ export default class EachBlock extends AbstractBlock {
 	has_animation: boolean;
 	has_binding = false;
 	has_index_binding = false;
-
+	context_rest_properties: Map<string, Node>;
 	else?: ElseBlock;
 
 	constructor(component: Component, parent: Node, scope: TemplateScope, info: TemplateNode) {
@@ -40,9 +40,9 @@ export default class EachBlock extends AbstractBlock {
 		this.index = info.index;
 
 		this.scope = scope.child();
-
+		this.context_rest_properties = new Map();
 		this.contexts = [];
-		unpack_destructuring({ contexts: this.contexts, node: info.context, scope, component });
+		unpack_destructuring({ contexts: this.contexts, node: info.context, scope, component, context_rest_properties: this.context_rest_properties });
 
 		this.contexts.forEach(context => {
 			this.scope.add(context.key.name, this.expression.dependencies, this);

src/compiler/compile/nodes/shared/Context.ts

@@ -20,7 +20,8 @@ export function unpack_destructuring({
 	modifier = (node) => node,
 	default_modifier = (node) => node,
 	scope,
-	component
+	component,
+	context_rest_properties
 }: {
 	contexts: Context[];
 	node: Node;
@@ -28,6 +29,7 @@ export function unpack_destructuring({
 	default_modifier?: Context['default_modifier'];
 	scope: TemplateScope;
 	component: Component;
+	context_rest_properties: Map<string, Node>;
 }) {
 	if (!node) return;
 
@@ -43,6 +45,7 @@ export function unpack_destructuring({
 			modifier,
 			default_modifier
 		});
+		context_rest_properties.set((node.argument as Identifier).name, node);
 	} else if (node.type === 'ArrayPattern') {
 		node.elements.forEach((element, i) => {
 			if (element && element.type === 'RestElement') {
@@ -52,8 +55,10 @@ export function unpack_destructuring({
 					modifier: (node) => x`${modifier(node)}.slice(${i})` as Node,
 					default_modifier,
 					scope,
-					component
+					component,
+					context_rest_properties
 				});
+				context_rest_properties.set((element.argument as Identifier).name, element);
 			} else if (element && element.type === 'AssignmentPattern') {
 				const n = contexts.length;
 				mark_referenced(element.right, scope, component);
@@ -70,7 +75,8 @@ export function unpack_destructuring({
 							to_ctx
 						)}` as Node,
 					scope,
-					component
+					component,
+					context_rest_properties
 				});
 			} else {
 				unpack_destructuring({
@@ -79,7 +85,8 @@ export function unpack_destructuring({
 					modifier: (node) => x`${modifier(node)}[${i}]` as Node,
 					default_modifier,
 					scope,
-					component
+					component,
+					context_rest_properties
 				});
 			}
 		});
@@ -97,8 +104,10 @@ export function unpack_destructuring({
 						)}, [${used_properties}])` as Node,
 					default_modifier,
 					scope,
-					component
+					component,
+					context_rest_properties
 				});
+				context_rest_properties.set((property.argument as Identifier).name, property);
 			} else {
 				const key = property.key as Identifier;
 				const value = property.value;
@@ -121,7 +130,8 @@ export function unpack_destructuring({
 								to_ctx
 							)}` as Node,
 						scope,
-						component
+						component,
+						context_rest_properties
 					});
 				} else {
 					unpack_destructuring({
@@ -130,7 +140,8 @@ export function unpack_destructuring({
 						modifier: (node) => x`${modifier(node)}.${key.name}` as Node,
 						default_modifier,
 						scope,
-						component
+						component,
+						context_rest_properties
 					});
 				}
 			}

test/validator/samples/rest-eachblock-binding-2/input.svelte

@@ -0,0 +1,11 @@
+<script>
+	let objArray = [
+		[1, 2, 3, "4"],
+		[5, 6, 7, "8"],
+	];
+</script>
+
+{#each objArray as [id, ...rest] (id)}
+	<input bind:value={rest[0]} type="text" placeholder={rest[2]} />
+	<br />
+{/each}

test/validator/samples/rest-eachblock-binding-2/warnings.json

@@ -0,0 +1,9 @@
+[
+  {
+    "code": "invalid-rest-eachblock-binding",
+    "message": "...rest operator will create a new object and binding propogation with original object will not work",
+		"pos": 102,
+    "start": { "line": 8, "column": 24, "character": 102 },
+		"end": { "line": 8, "column": 31, "character": 109 }
+  }
+]

test/validator/samples/rest-eachblock-binding-3/input.svelte

@@ -0,0 +1,8 @@
+<script>
+	let objArray = [{ bar: {foo: '1', id: 0, innerValue: "test"} }, { bar: {foo: '2', id:1, innerValue: "Somethin"} }]
+</script>
+
+{#each objArray as { bar: { id, ...rest } } (id)}
+	<input bind:value={rest.innerValue} type="text" placeholder={rest.foo}/>
+	<br/>
+{/each}

test/validator/samples/rest-eachblock-binding-3/warnings.json

@@ -0,0 +1,9 @@
+[
+  {
+    "code": "invalid-rest-eachblock-binding",
+    "message": "...rest operator will create a new object and binding propogation with original object will not work",
+		"pos": 168,
+    "start": { "line": 5, "column": 32, "character": 168 },
+		"end": { "line": 5, "column": 39, "character": 175 }
+  }
+]

test/validator/samples/rest-eachblock-binding/input.svelte

@@ -0,0 +1,8 @@
+<script>
+	let objArray = [{foo: '1', id: 0, innerValue: "test"}, {foo: '2', id:1, innerValue: "Somethin"}]
+</script>
+
+{#each objArray as { id, ...rest } (id)}
+	<input bind:value={rest.innerValue} type="text" placeholder={rest.foo}/>
+	<br/>
+{/each}

test/validator/samples/rest-eachblock-binding/warnings.json

@@ -0,0 +1,9 @@
+[
+  {
+    "code": "invalid-rest-eachblock-binding",
+    "message": "...rest operator will create a new object and binding propogation with original object will not work",
+		"pos": 143,
+		"start": { "line": 5, "column": 25, "character": 143 },
+    "end": { "line": 5, "column": 32, "character": 150 }
+  }
+]