From d6c985960b81a97a6e41af8b7c1f0f0b790ae62a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 May 2025 09:53:27 -0400 Subject: [PATCH] unskip and fix test --- .../98-reference/.generated/compile-errors.md | 6 +++ .../svelte/messages/compile-errors/script.md | 4 ++ packages/svelte/src/compiler/errors.js | 29 +++++++---- .../visitors/AssignmentExpression.js | 2 +- .../2-analyze/visitors/BindDirective.js | 2 +- .../2-analyze/visitors/UpdateExpression.js | 2 +- .../phases/2-analyze/visitors/shared/utils.js | 50 ++++++++++++++++--- .../class-state-constructor-9/_config.js | 5 -- .../class-state-constructor-9/errors.json | 12 ++--- 9 files changed, 81 insertions(+), 31 deletions(-) delete mode 100644 packages/svelte/tests/validator/samples/class-state-constructor-9/_config.js diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index 8fdbe87fb0..db848a0299 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -872,6 +872,12 @@ class Counter { ...but it can only happen once. +### state_field_invalid_assignment + +``` +Cannot assign to a state field before its declaration +``` + ### state_invalid_export ``` diff --git a/packages/svelte/messages/compile-errors/script.md b/packages/svelte/messages/compile-errors/script.md index cfd6a2ae00..e11975aef2 100644 --- a/packages/svelte/messages/compile-errors/script.md +++ b/packages/svelte/messages/compile-errors/script.md @@ -236,6 +236,10 @@ class Counter { ...but it can only happen once. +## state_field_invalid_assignment + +> Cannot assign to a state field before its declaration + ## state_invalid_export > Cannot export state from a module if it is reassigned. Either export a function returning the state value or only mutate the state value's properties diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 47183d2e80..25e72340c6 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -104,16 +104,6 @@ export function constant_binding(node, thing) { e(node, 'constant_binding', `Cannot bind to ${thing}\nhttps://svelte.dev/e/constant_binding`); } -/** - * `%name%` has already been declared on this class - * @param {null | number | NodeLike} node - * @param {string} name - * @returns {never} - */ -export function state_field_duplicate(node, name) { - e(node, 'state_field_duplicate', `\`${name}\` has already been declared on this class\nhttps://svelte.dev/e/state_field_duplicate`); -} - /** * `%name%` has already been declared * @param {null | number | NodeLike} node @@ -471,6 +461,25 @@ export function snippet_parameter_assignment(node) { e(node, 'snippet_parameter_assignment', `Cannot reassign or bind to snippet parameter\nhttps://svelte.dev/e/snippet_parameter_assignment`); } +/** + * `%name%` has already been declared on this class + * @param {null | number | NodeLike} node + * @param {string} name + * @returns {never} + */ +export function state_field_duplicate(node, name) { + e(node, 'state_field_duplicate', `\`${name}\` has already been declared on this class\nhttps://svelte.dev/e/state_field_duplicate`); +} + +/** + * Cannot assign to a state field before its declaration + * @param {null | number | NodeLike} node + * @returns {never} + */ +export function state_field_invalid_assignment(node) { + e(node, 'state_field_invalid_assignment', `Cannot assign to a state field before its declaration\nhttps://svelte.dev/e/state_field_invalid_assignment`); +} + /** * Cannot export state from a module if it is reassigned. Either export a function returning the state value or only mutate the state value's properties * @param {null | number | NodeLike} node diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js index a64c89cd88..673c79f2df 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js @@ -8,7 +8,7 @@ import { validate_assignment } from './shared/utils.js'; * @param {Context} context */ export function AssignmentExpression(node, context) { - validate_assignment(node, node.left, context.state); + validate_assignment(node, node.left, context); if (context.state.reactive_statement) { const id = node.left.type === 'MemberExpression' ? object(node.left) : node.left; diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js index 18ea79262b..9f02e7fa5a 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js @@ -158,7 +158,7 @@ export function BindDirective(node, context) { return; } - validate_assignment(node, node.expression, context.state); + validate_assignment(node, node.expression, context); const assignee = node.expression; const left = object(assignee); diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/UpdateExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/UpdateExpression.js index 741effc67a..13f4b9019e 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/UpdateExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/UpdateExpression.js @@ -8,7 +8,7 @@ import { validate_assignment } from './shared/utils.js'; * @param {Context} context */ export function UpdateExpression(node, context) { - validate_assignment(node, node.argument, context.state); + validate_assignment(node, node.argument, context); if (context.state.reactive_statement) { const id = node.argument.type === 'MemberExpression' ? object(node.argument) : node.argument; diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js index 12e21c386c..89921832ae 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js @@ -4,24 +4,25 @@ /** @import { Scope } from '../../../scope' */ /** @import { NodeLike } from '../../../../errors.js' */ import * as e from '../../../../errors.js'; -import { extract_identifiers } from '../../../../utils/ast.js'; +import { extract_identifiers, get_parent } from '../../../../utils/ast.js'; import * as w from '../../../../warnings.js'; import * as b from '#compiler/builders'; import { get_rune } from '../../../scope.js'; +import { get_name } from '../../../nodes.js'; /** * @param {AssignmentExpression | UpdateExpression | AST.BindDirective} node * @param {Pattern | Expression} argument - * @param {AnalysisState} state + * @param {Context} context */ -export function validate_assignment(node, argument, state) { - validate_no_const_assignment(node, argument, state.scope, node.type === 'BindDirective'); +export function validate_assignment(node, argument, context) { + validate_no_const_assignment(node, argument, context.state.scope, node.type === 'BindDirective'); if (argument.type === 'Identifier') { - const binding = state.scope.get(argument.name); + const binding = context.state.scope.get(argument.name); - if (state.analysis.runes) { - if (binding?.node === state.analysis.props_id) { + if (context.state.analysis.runes) { + if (binding?.node === context.state.analysis.props_id) { e.constant_assignment(node, '$props.id()'); } @@ -34,6 +35,41 @@ export function validate_assignment(node, argument, state) { e.snippet_parameter_assignment(node); } } + + if (argument.type === 'MemberExpression' && argument.object.type === 'ThisExpression') { + const name = + argument.computed && argument.property.type !== 'Literal' + ? null + : get_name(argument.property); + + const field = + name !== null && + context.state.state_fields && + Object.hasOwn(context.state.state_fields, name) && + context.state.state_fields[name]; + + // check we're not assigning to a state field before its declaration in the constructor + if (field && field.node.type === 'AssignmentExpression' && node !== field.node) { + let i = context.path.length; + while (i--) { + const parent = context.path[i]; + + if ( + parent.type === 'FunctionDeclaration' || + parent.type === 'FunctionExpression' || + parent.type === 'ArrowFunctionExpression' + ) { + const grandparent = get_parent(context.path, i - 1); + + if (grandparent.type === 'MethodDefinition' && grandparent.kind === 'constructor') { + e.state_field_invalid_assignment(node); + } + + break; + } + } + } + } } /** diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-9/_config.js b/packages/svelte/tests/validator/samples/class-state-constructor-9/_config.js deleted file mode 100644 index 18cc8bd6d1..0000000000 --- a/packages/svelte/tests/validator/samples/class-state-constructor-9/_config.js +++ /dev/null @@ -1,5 +0,0 @@ -import { test } from '../../test'; - -export default test({ - skip: true // TODO delete this file so the test runs -}); diff --git a/packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json b/packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json index 691f5d8815..c4cb0991d0 100644 --- a/packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json +++ b/packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json @@ -1,14 +1,14 @@ [ { - "code": "state_field_duplicate", - "message": "`count` has already been declared on this class", + "code": "state_field_invalid_assignment", + "message": "Cannot assign to a state field before its declaration", "start": { - "line": 7, - "column": 2 + "line": 4, + "column": 3 }, "end": { - "line": 7, - "column": 24 + "line": 4, + "column": 18 } } ]