fix: throw on duplicate class field declarations (#16502)

* fix: throw on duplicate class field declarations

* doh

* handle assignment in constructor

* fix

* apply suggestion from review

* fix

* fix failing test

Author: Ocean-OS

.changeset/odd-phones-taste.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: throw on duplicate class field declarations

documentation/docs/98-reference/.generated/compile-errors.md

@@ -364,6 +364,12 @@ The $ name is reserved, and cannot be used for variables and imports
 The $ prefix is reserved, and cannot be used for variables and imports
 ```
 
+### duplicate_class_field
+
+```
+`%name%` has already been declared
+```
+
 ### each_item_invalid_assignment
 
 ```

packages/svelte/messages/compile-errors/script.md

@@ -30,6 +30,10 @@
 
 > The $ prefix is reserved, and cannot be used for variables and imports
 
+## duplicate_class_field
+
+> `%name%` has already been declared
+
 ## each_item_invalid_assignment
 
 > Cannot reassign or bind to each block argument in runes mode. Use the array and index variables instead (e.g. `array[i] = value` instead of `entry = value`, or `bind:value={array[i]}` instead of `bind:value={entry}`)

packages/svelte/src/compiler/errors.js

@@ -152,6 +152,16 @@ export function dollar_prefix_invalid(node) {
 	e(node, 'dollar_prefix_invalid', `The $ prefix is reserved, and cannot be used for variables and imports\nhttps://svelte.dev/e/dollar_prefix_invalid`);
 }
 
+/**
+ * `%name%` has already been declared
+ * @param {null | number | NodeLike} node
+ * @param {string} name
+ * @returns {never}
+ */
+export function duplicate_class_field(node, name) {
+	e(node, 'duplicate_class_field', `\`${name}\` has already been declared\nhttps://svelte.dev/e/duplicate_class_field`);
+}
+
 /**
  * Cannot reassign or bind to each block argument in runes mode. Use the array and index variables instead (e.g. `array[i] = value` instead of `entry = value`, or `bind:value={array[i]}` instead of `bind:value={entry}`)
  * @param {null | number | NodeLike} node

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

@@ -33,6 +33,9 @@ export function ClassBody(node, context) {
 	/** @type {Map<string, StateField>} */
 	const state_fields = new Map();
 
+	/** @type {Map<string, Array<MethodDefinition['kind'] | 'prop' | 'assigned_prop'>>} */
+	const fields = new Map();
+
 	context.state.analysis.classes.set(node, state_fields);
 
 	/** @type {MethodDefinition | null} */
@@ -54,6 +57,14 @@ export function ClassBody(node, context) {
 				e.state_field_duplicate(node, name);
 			}
 
+			const _key = (key.type === 'PrivateIdentifier' ? '#' : '') + name;
+			const field = fields.get(_key);
+
+			// if there's already a method or assigned field, error
+			if (field && !(field.length === 1 && field[0] === 'prop')) {
+				e.duplicate_class_field(node, _key);
+			}
+
 			state_fields.set(name, {
 				node,
 				type: rune,
@@ -67,10 +78,48 @@ export function ClassBody(node, context) {
 	for (const child of node.body) {
 		if (child.type === 'PropertyDefinition' && !child.computed && !child.static) {
 			handle(child, child.key, child.value);
+			const key = (child.key.type === 'PrivateIdentifier' ? '#' : '') + get_name(child.key);
+			const field = fields.get(key);
+			if (!field) {
+				fields.set(key, [child.value ? 'assigned_prop' : 'prop']);
+				continue;
+			}
+			e.duplicate_class_field(child, key);
 		}
 
-		if (child.type === 'MethodDefinition' && child.kind === 'constructor') {
-			constructor = child;
+		if (child.type === 'MethodDefinition') {
+			if (child.kind === 'constructor') {
+				constructor = child;
+			} else if (!child.computed) {
+				const key = (child.key.type === 'PrivateIdentifier' ? '#' : '') + get_name(child.key);
+				const field = fields.get(key);
+				if (!field) {
+					fields.set(key, [child.kind]);
+					continue;
+				}
+				if (
+					field.includes(child.kind) ||
+					field.includes('prop') ||
+					field.includes('assigned_prop')
+				) {
+					e.duplicate_class_field(child, key);
+				}
+				if (child.kind === 'get') {
+					if (field.length === 1 && field[0] === 'set') {
+						field.push('get');
+						continue;
+					}
+				} else if (child.kind === 'set') {
+					if (field.length === 1 && field[0] === 'get') {
+						field.push('set');
+						continue;
+					}
+				} else {
+					field.push(child.kind);
+					continue;
+				}
+				e.duplicate_class_field(child, key);
+			}
 		}
 	}
 

packages/svelte/tests/validator/samples/class-state-constructor-9/errors.json

@@ -1,14 +1,14 @@
 [
 	{
-		"code": "state_field_invalid_assignment",
-		"message": "Cannot assign to a state field before its declaration",
+		"code": "duplicate_class_field",
+		"message": "`count` has already been declared",
 		"start": {
-			"line": 2,
-			"column": 1
+			"line": 5,
+			"column": 2
 		},
 		"end": {
-			"line": 2,
-			"column": 12
+			"line": 5,
+			"column": 24
 		}
 	}
 ]