do the analysis upfront, it's way simpler

packages/svelte/src/compiler/phases/2-analyze/index.js

@@ -48,7 +48,6 @@ import { Literal } from './visitors/Literal.js';
 import { MemberExpression } from './visitors/MemberExpression.js';
 import { NewExpression } from './visitors/NewExpression.js';
 import { OnDirective } from './visitors/OnDirective.js';
-import { PropertyDefinition } from './visitors/PropertyDefinition.js';
 import { RegularElement } from './visitors/RegularElement.js';
 import { RenderTag } from './visitors/RenderTag.js';
 import { SlotElement } from './visitors/SlotElement.js';
@@ -165,7 +164,6 @@ const visitors = {
 	MemberExpression,
 	NewExpression,
 	OnDirective,
-	PropertyDefinition,
 	RegularElement,
 	RenderTag,
 	SlotElement,

packages/svelte/src/compiler/phases/2-analyze/visitors/AssignmentExpression.js

@@ -23,6 +23,5 @@ export function AssignmentExpression(node, context) {
 		}
 	}
 
-	context.state.class?.register?.(node, context);
 	context.next();
 }

packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js

@@ -1,4 +1,4 @@
-/** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, Identifier, PrivateIdentifier, Literal } from 'estree' */
+/** @import { AssignmentExpression, ClassBody, PropertyDefinition, Expression, Identifier, PrivateIdentifier, Literal, MethodDefinition } from 'estree' */
 /** @import { AST } from '#compiler' */
 /** @import { Context } from '../types' */
 /** @import { StateCreationRuneName } from '../../../../utils.js' */
@@ -19,78 +19,90 @@ export function ClassBody(node, context) {
 
 	const analyzed = new ClassAnalysis();
 
-	context.next({
+	/** @type {string[]} */
-		...context.state,
+	const seen = [];
-		class: analyzed
-	});
-}
 
-/** @typedef { StateCreationRuneName | 'regular'} PropertyAssignmentType */
+	/** @type {MethodDefinition | null} */
-/** @typedef {{ type: PropertyAssignmentType; node: AssignmentExpression | PropertyDefinition; }} PropertyAssignmentDetails */
+	let constructor = null;
-
-const reassignable_assignments = new Set(['$state', '$state.raw', 'regular']);
-
-class ClassAnalysis {
-	/** @type {Map<string, PropertyAssignmentDetails>} */
-	#public_assignments = new Map();
-
-	/** @type {Map<string, PropertyAssignmentDetails>} */
-	#private_assignments = new Map();
 
 	/**
-	 * Determines if the node is a valid assignment to a class property, and if so,
+	 * @param {PropertyDefinition | AssignmentExpression} node
-	 * registers the assignment.
+	 * @param {Expression | PrivateIdentifier} key
-	 * @param {AssignmentExpression | PropertyDefinition} node
+	 * @param {Expression | null | undefined} value
-	 * @param {Context} context
 	 */
-	register(node, context) {
+	function handle(node, key, value) {
-		/** @type {string} */
+		const name = get_name(key);
-		let name;
+		if (!name) return;
-		/** @type {PropertyAssignmentType} */
+
-		let type;
+		const rune = get_rune(value, context.state.scope);
-		/** @type {boolean} */
+
-		let is_private;
+		if (rune && is_state_creation_rune(rune)) {
-
+			if (seen.includes(name)) {
-		if (node.type === 'AssignmentExpression') {
+				e.constructor_state_reassignment(node); // TODO the same thing applies to duplicate fields, so the code/message needs to change
-			if (!this.is_class_property_assignment_at_constructor_root(node, context.path)) {
-				return;
 			}
 
-			let maybe_name = get_name_for_identifier(node.left.property);
+			analyzed.fields[name] = {
-			if (!maybe_name) {
+				node,
-				return;
+				type: rune
+			};
 		}
 
-			name = maybe_name;
+		if (value) {
-			type = this.#get_assignment_type(node, context);
+			seen.push(name);
-			is_private = node.left.property.type === 'PrivateIdentifier';
+		}
+	}
 
-			this.#check_for_conflicts(node, name, type, is_private);
+	for (const child of node.body) {
-		} else {
+		if (child.type === 'PropertyDefinition' && !child.computed) {
-			if (!this.#is_assigned_property(node)) {
+			handle(child, child.key, child.value);
-				return;
 		}
 
-			let maybe_name = get_name_for_identifier(node.key);
+		if (
-			if (!maybe_name) {
+			child.type === 'MethodDefinition' &&
-				return;
+			child.key.type === 'Identifier' &&
+			child.key.name === 'constructor'
+		) {
+			constructor = child;
 		}
+	}
+
+	if (constructor) {
+		for (const statement of constructor.value.body.body) {
+			if (statement.type !== 'ExpressionStatement') continue;
+			if (statement.expression.type !== 'AssignmentExpression') continue;
 
-			name = maybe_name;
+			const { left, right } = statement.expression;
-			type = this.#get_assignment_type(node, context);
-			is_private = node.key.type === 'PrivateIdentifier';
 
-			// we don't need to check for conflicts here because they're not possible yet
+			if (left.type !== 'MemberExpression') continue;
+			if (left.object.type !== 'ThisExpression') continue;
+			if (left.computed && left.property.type !== 'Literal') continue;
+
+			handle(statement.expression, left.property, right);
+		}
 	}
 
-		// we don't have to validate anything other than conflicts here, because the rune placement rules
+	context.next({
-		// catch all of the other weirdness.
+		...context.state,
-		const map = is_private ? this.#private_assignments : this.#public_assignments;
+		class: analyzed
-		if (!map.has(name)) {
+	});
-			map.set(name, { type, node });
 }
+
+/** @param {Expression | PrivateIdentifier} node */
+function get_name(node) {
+	if (node.type === 'Literal') return String(node.value);
+	if (node.type === 'PrivateIdentifier') return '#' + node.name;
+	if (node.type === 'Identifier') return node.name;
+
+	return null;
 }
 
+/** @typedef { StateCreationRuneName | 'regular'} PropertyAssignmentType */
+/** @typedef {{ type: PropertyAssignmentType; node: AssignmentExpression | PropertyDefinition; }} PropertyAssignmentDetails */
+
+class ClassAnalysis {
+	/** @type {Record<string, PropertyAssignmentDetails>} */
+	fields = {};
+
 	/**
 	 * @template {AST.SvelteNode} T
 	 * @param {AST.SvelteNode} node
@@ -119,70 +131,4 @@ class ClassAnalysis {
 			maybe_constructor.kind === 'constructor'
 		);
 	}
-
-	/**
-	 * We only care about properties that have values assigned to them -- if they don't,
-	 * they can't be a conflict for state declared in the constructor.
-	 * @param {PropertyDefinition} node
-	 * @returns {node is PropertyDefinition & { key: { type: 'PrivateIdentifier' | 'Identifier' | 'Literal' }; value: Expression; static: false; computed: false }}
-	 */
-	#is_assigned_property(node) {
-		return (
-			(node.key.type === 'PrivateIdentifier' ||
-				node.key.type === 'Identifier' ||
-				node.key.type === 'Literal') &&
-			Boolean(node.value) &&
-			!node.static &&
-			!node.computed
-		);
-	}
-
-	/**
-	 * Checks for conflicts with existing assignments. A conflict occurs if:
-	 * - The original assignment used `$derived` or `$derived.by` (these can never be reassigned)
-	 * - The original assignment used `$state`, `$state.raw`, or `regular` and is being assigned to with any type other than `regular`
-	 * @param {AssignmentExpression} node
-	 * @param {string} name
-	 * @param {PropertyAssignmentType} type
-	 * @param {boolean} is_private
-	 */
-	#check_for_conflicts(node, name, type, is_private) {
-		const existing = (is_private ? this.#private_assignments : this.#public_assignments).get(name);
-		if (!existing) {
-			return;
-		}
-
-		if (reassignable_assignments.has(existing.type) && type === 'regular') {
-			return;
-		}
-
-		e.constructor_state_reassignment(node);
-	}
-
-	/**
-	 * @param {AssignmentExpression | PropertyDefinition} node
-	 * @param {Context} context
-	 * @returns {PropertyAssignmentType}
-	 */
-	#get_assignment_type(node, context) {
-		const value = node.type === 'AssignmentExpression' ? node.right : node.value;
-		const rune = get_rune(value, context.state.scope);
-		if (rune === null) {
-			return 'regular';
-		}
-		if (is_state_creation_rune(rune)) {
-			return rune;
-		}
-		// this does mean we return `regular` for some other runes (like `$trace` or `$state.raw`)
-		// -- this is ok because the rune placement rules will throw if they're invalid.
-		return 'regular';
-	}
-}
-
-/**
- *
- * @param {PrivateIdentifier | Identifier | Literal} node
- */
-function get_name_for_identifier(node) {
-	return node.type === 'Literal' ? node.value?.toString() : node.name;
 }

packages/svelte/src/compiler/phases/2-analyze/visitors/PropertyDefinition.js

@@ -1,12 +0,0 @@
-/** @import { PropertyDefinition } from 'estree' */
-/** @import { Context } from '../types' */
-
-/**
- *
- * @param {PropertyDefinition} node
- * @param {Context} context
- */
-export function PropertyDefinition(node, context) {
-	context.state.class?.register(node, context);
-	context.next();
-}