put nested class warning on the declaration, not the usage site (#9592)

Co-authored-by: Rich Harris <[email protected]>

Author: Rich-Harris

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

@@ -580,24 +580,19 @@ export const validation_runes_js = {
 			private_derived_state
 		});
 	},
-	NewExpression(node, context) {
-		const callee = node.callee;
-
-		const binding = callee.type === 'Identifier' ? context.state.scope.get(callee.name) : null;
-		const is_module = context.state.ast_type === 'module';
+	ClassDeclaration(node, context) {
 		// In modules, we allow top-level module scope only, in components, we allow the component scope,
 		// which is function_depth of 1. With the exception of `new class` which is also not allowed at
 		// component scope level either.
-		const allowed_depth = is_module ? 0 : 1;
+		const allowed_depth = context.state.ast_type === 'module' ? 0 : 1;
 
-		if (
-			(callee.type === 'ClassExpression' && context.state.scope.function_depth > 0) ||
-			(binding !== null &&
-				binding.initial !== null &&
-				binding.initial.type === 'ClassDeclaration' &&
-				binding.scope.function_depth > allowed_depth)
-		) {
-			warn(context.state.analysis.warnings, node, context.path, 'inline-new-class');
+		if (context.state.scope.function_depth > allowed_depth) {
+			warn(context.state.analysis.warnings, node, context.path, 'avoid-nested-class');
+		}
+	},
+	NewExpression(node, context) {
+		if (node.callee.type === 'ClassExpression' && context.state.scope.function_depth > 0) {
+			warn(context.state.analysis.warnings, node, context.path, 'avoid-inline-class');
 		}
 	}
 };
@@ -741,6 +736,8 @@ export const validation_runes = merge(validation, a11y_validators, {
 			}
 		}
 	},
+	// TODO this is a code smell. need to refactor this stuff
 	ClassBody: validation_runes_js.ClassBody,
+	ClassDeclaration: validation_runes_js.ClassDeclaration,
 	NewExpression: validation_runes_js.NewExpression
 });

packages/svelte/src/compiler/warnings.js

@@ -193,9 +193,9 @@ const state = {
 
 /** @satisfies {Warnings} */
 const performance = {
-	'inline-new-class': () =>
-		`Creating inline classes will likely cause performance issues. ` +
-		`Instead, declare the class at the module-level and create new instances from the class reference.`
+	'avoid-inline-class': () =>
+		`Avoid 'new class' — instead, declare the class at the top level scope`,
+	'avoid-nested-class': () => `Avoid declaring classes below the top level scope`
 };
 
 /** @satisfies {Warnings} */

packages/svelte/tests/validator/samples/inline-new-class-2/warnings.json

@@ -1,14 +1,14 @@
 [
 	{
-		"code": "inline-new-class",
-		"message": "Creating inline classes will likely cause performance issues. Instead, declare the class at the module-level and create new instances from the class reference.",
+		"code": "avoid-nested-class",
+		"message": "Avoid declaring classes below the top level scope",
 		"start": {
-			"column": 12,
-			"line": 6
+			"column": 2,
+			"line": 3
 		},
 		"end": {
-			"column": 21,
-			"line": 6
+			"column": 3,
+			"line": 5
 		}
 	}
 ]

packages/svelte/tests/validator/samples/inline-new-class-4/warnings.json

@@ -1,7 +1,7 @@
 [
 	{
-		"code": "inline-new-class",
-		"message": "Creating inline classes will likely cause performance issues. Instead, declare the class at the module-level and create new instances from the class reference.",
+		"code": "avoid-inline-class",
+		"message": "Avoid 'new class' — instead, declare the class at the top level scope",
 		"start": {
 			"column": 12,
 			"line": 3

packages/svelte/tests/validator/samples/inline-new-class/warnings.json

@@ -1,7 +1,7 @@
 [
 	{
-		"code": "inline-new-class",
-		"message": "Creating inline classes will likely cause performance issues. Instead, declare the class at the module-level and create new instances from the class reference.",
+		"code": "avoid-inline-class",
+		"message": "Avoid 'new class' — instead, declare the class at the top level scope",
 		"start": {
 			"column": 11,
 			"line": 2