From 0f774533bd4741284937a1b4f7fb344eaa3390d7 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 6 Mar 2024 11:08:57 +0100 Subject: [PATCH] invalid-rest-eachblock-binding --- .../compiler/phases/2-analyze/validation.js | 13 ++++++++++- packages/svelte/src/compiler/phases/scope.js | 23 +++++++++++++++++-- packages/svelte/src/compiler/types/index.d.ts | 5 ++++ packages/svelte/src/compiler/warnings.js | 5 +++- .../rest-eachblock-binding-2/_config.js | 4 ---- .../rest-eachblock-binding-3/_config.js | 4 ---- .../_config.js | 4 ---- .../samples/rest-eachblock-binding/_config.js | 4 ---- 8 files changed, 42 insertions(+), 20 deletions(-) delete mode 100644 packages/svelte/tests/validator/samples/rest-eachblock-binding-2/_config.js delete mode 100644 packages/svelte/tests/validator/samples/rest-eachblock-binding-3/_config.js delete mode 100644 packages/svelte/tests/validator/samples/rest-eachblock-binding-nested-rest/_config.js delete mode 100644 packages/svelte/tests/validator/samples/rest-eachblock-binding/_config.js diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index 94233681ca..7b9a52e04f 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -298,13 +298,24 @@ const validation = { // TODO handle mutations of non-state/props in runes mode } + const binding = context.state.scope.get(left.name); + if (node.name === 'group') { - const binding = context.state.scope.get(left.name); if (!binding) { error(node, 'INTERNAL', 'Cannot find declaration for bind:group'); } } + if (binding?.kind === 'each' && binding.metadata?.inside_rest) { + warn( + context.state.analysis.warnings, + binding.node, + context.path, + 'invalid-rest-eachblock-binding', + binding.node.name + ); + } + const parent = context.path.at(-1); if ( diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index a0d95347f9..bf998dfafb 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -112,7 +112,8 @@ export class Scope { prop_alias: null, expression: null, mutation: null, - reassigned: false + reassigned: false, + metadata: null }; this.declarations.set(node.name, binding); this.root.conflicts.add(node.name); @@ -534,7 +535,25 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { // declarations for (const id of extract_identifiers(node.context)) { - scope.declare(id, 'each', 'const'); + const binding = scope.declare(id, 'each', 'const'); + + let inside_rest = false; + let is_rest_id = false; + walk(node.context, null, { + Identifier(node) { + if (inside_rest && node === id) { + is_rest_id = true; + } + }, + RestElement(_, { next }) { + const prev = inside_rest; + inside_rest = true; + next(); + inside_rest = prev; + } + }); + + binding.metadata = { inside_rest: is_rest_id }; } if (node.context.type !== 'Identifier') { scope.declare(b.id('$$item'), 'derived', 'synthetic'); diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 11e71f63a1..a04d1c39af 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -288,6 +288,11 @@ export interface Binding { expression: Expression | ((id: Identifier) => Expression) | null; /** If this is set, all mutations should use this expression */ mutation: ((assignment: AssignmentExpression, context: Context) => Expression) | null; + /** Additional metadata, varies per binding type */ + metadata: { + /** `true` if is (inside) a rest parameter */ + inside_rest?: boolean; + } | null; } export * from './template.js'; diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index c3f9b33364..37eec3f089 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -197,7 +197,10 @@ const a11y = { /** @satisfies {Warnings} */ const state = { 'static-state-reference': () => - `State referenced in its own scope will never update. Did you mean to reference it inside a closure?` + `State referenced in its own scope will never update. Did you mean to reference it inside a closure?`, + /** @param {string} name */ + 'invalid-rest-eachblock-binding': (name) => + `The rest operator (...) will create a new object and binding '${name}' with the original object will not work` }; /** @satisfies {Warnings} */ diff --git a/packages/svelte/tests/validator/samples/rest-eachblock-binding-2/_config.js b/packages/svelte/tests/validator/samples/rest-eachblock-binding-2/_config.js deleted file mode 100644 index 7fc1f9f61b..0000000000 --- a/packages/svelte/tests/validator/samples/rest-eachblock-binding-2/_config.js +++ /dev/null @@ -1,4 +0,0 @@ -import { test } from '../../test'; - -// TODO this likely works in the new world - remove this warning? -export default test({ skip: true }); diff --git a/packages/svelte/tests/validator/samples/rest-eachblock-binding-3/_config.js b/packages/svelte/tests/validator/samples/rest-eachblock-binding-3/_config.js deleted file mode 100644 index 7fc1f9f61b..0000000000 --- a/packages/svelte/tests/validator/samples/rest-eachblock-binding-3/_config.js +++ /dev/null @@ -1,4 +0,0 @@ -import { test } from '../../test'; - -// TODO this likely works in the new world - remove this warning? -export default test({ skip: true }); diff --git a/packages/svelte/tests/validator/samples/rest-eachblock-binding-nested-rest/_config.js b/packages/svelte/tests/validator/samples/rest-eachblock-binding-nested-rest/_config.js deleted file mode 100644 index a7d521e510..0000000000 --- a/packages/svelte/tests/validator/samples/rest-eachblock-binding-nested-rest/_config.js +++ /dev/null @@ -1,4 +0,0 @@ -import { test } from '../../test'; - -// TODO this maybe works in the new world - remove this warning? -export default test({ skip: true }); diff --git a/packages/svelte/tests/validator/samples/rest-eachblock-binding/_config.js b/packages/svelte/tests/validator/samples/rest-eachblock-binding/_config.js deleted file mode 100644 index 7fc1f9f61b..0000000000 --- a/packages/svelte/tests/validator/samples/rest-eachblock-binding/_config.js +++ /dev/null @@ -1,4 +0,0 @@ -import { test } from '../../test'; - -// TODO this likely works in the new world - remove this warning? -export default test({ skip: true });