chore: get more validator tests passing (#10714)

Get more validation tests passing:
- const tag cyclic validation (now runtime, based because of new reactivity system)
- illegal-variable-declaration
- illegal-attribute-character
- remove invalid-reactive-var validation as legacy reactive statements are transformed to functions under the hood, which never escape scope - arguably not completely correct, but will be what the user expects anyway
- invalid-rest-eachblock-binding
- remove edge-case redundant-event-modifier warning because event modifiers are deprecated anyway
- invalid-style-directive-modifier
- invalid-tag-property (now a different error)
- module-script-reactive-declaration
- take comment above script into account when silencing warnings
- invalid-css-declaration
- unused-export-let
- invalid-html-attribute
- illegal-store-subscription
- empty-block

Author: dummdidumm

packages/svelte/src/compiler/errors.js

@@ -110,7 +110,8 @@ const css = {
 		`:global(...) must not contain type or universal selectors when used in a compound selector`,
 	'invalid-css-selector': () => `Invalid selector`,
 	'invalid-css-identifier': () => 'Expected a valid CSS identifier',
-	'invalid-nesting-selector': () => `Nesting selectors can only be used inside a rule`
+	'invalid-nesting-selector': () => `Nesting selectors can only be used inside a rule`,
+	'invalid-css-declaration': () => 'Declaration cannot be empty'
 };
 
 /** @satisfies {Errors} */
@@ -278,7 +279,9 @@ const attributes = {
 					directive2
 				)} directive`;
 	},
-	'invalid-let-directive-placement': () => 'let directive at invalid position'
+	'invalid-let-directive-placement': () => 'let directive at invalid position',
+	'invalid-style-directive-modifier': () =>
+		`Invalid 'style:' modifier. Valid modifiers are: 'important'`
 };
 
 /** @satisfies {Errors} */
@@ -330,7 +333,11 @@ const variables = {
 		`${name} is an illegal variable name. To reference a global variable called ${name}, use globalThis.${name}`,
 	/** @param {string} name */
 	'duplicate-declaration': (name) => `'${name}' has already been declared`,
-	'default-export': () => `A component cannot have a default export`
+	'default-export': () => `A component cannot have a default export`,
+	'illegal-variable-declaration': () =>
+		'Cannot declare same variable name which is imported inside <script context="module">',
+	'illegal-store-subscription': () =>
+		'Cannot subscribe to stores that are not declared at the top level of the component'
 };
 
 /** @satisfies {Errors} */
@@ -435,11 +442,6 @@ const errors = {
 	// 		message
 	// 	};
 	// },
-	// contextual_store: {
-	// 	code: 'contextual-store',
-	// 	message:
-	// 		'Stores must be declared at the top level of the component (this may change in a future version of Svelte)'
-	// },
 	// default_export: {
 	// 	code: 'default-export',
 	// 	message: 'A component cannot have a default export'
@@ -448,31 +450,11 @@ const errors = {
 	// 	code: 'illegal-declaration',
 	// 	message: 'The $ prefix is reserved, and cannot be used for variable and import names'
 	// },
-	// illegal_global: /** @param {string} name */ (name) => ({
-	// 	code: 'illegal-global',
-	// 	message: `${name} is an illegal variable name`
-	// }),
-	// illegal_variable_declaration: {
-	// 	code: 'illegal-variable-declaration',
-	// 	message: 'Cannot declare same variable name which is imported inside <script context="module">'
-	// },
 	// invalid_directive_value: {
 	// 	code: 'invalid-directive-value',
 	// 	message:
 	// 		'Can only bind to an identifier (e.g. `foo`) or a member expression (e.g. `foo.bar` or `foo[baz]`)'
 	// },
-	// cyclical_const_tags: /** @param {string[]} cycle */ (cycle) => ({
-	// 	code: 'cyclical-const-tags',
-	// 	message: `Cyclical dependency detected: ${cycle.join(' → ')}`
-	// }),
-	// invalid_var_declaration: {
-	// 	code: 'invalid_var_declaration',
-	// 	message: '"var" scope should not extend outside the reactive block'
-	// },
-	// invalid_style_directive_modifier: /** @param {string} valid */ (valid) => ({
-	// 	code: 'invalid-style-directive-modifier',
-	// 	message: `Valid modifiers for style directives are: ${valid}`
-	// })
 };
 
 // interface is duplicated between here (used internally) and ./interfaces.js

packages/svelte/src/compiler/phases/1-parse/read/style.js

@@ -476,6 +476,10 @@ function read_declaration(parser) {
 
 	const value = read_value(parser);
 
+	if (!value && !property.startsWith('--')) {
+		error(parser.index, 'invalid-css-declaration');
+	}
+
 	const end = parser.index;
 
 	if (!parser.match('}')) {

packages/svelte/src/compiler/phases/1-parse/state/element.js

@@ -268,22 +268,41 @@ export default function tag(parser) {
 		if (name === 'script') {
 			const content = read_script(parser, start, element.attributes);
 
-			if (content) {
-				if (content.context === 'module') {
-					if (current.module) error(start, 'duplicate-script-element');
-					current.module = content;
-				} else {
-					if (current.instance) error(start, 'duplicate-script-element');
-					current.instance = content;
+			/** @type {import('#compiler').Comment | null} */
+			let prev_comment = null;
+			for (let i = current.fragment.nodes.length - 1; i >= 0; i--) {
+				const node = current.fragment.nodes[i];
+
+				if (i === current.fragment.nodes.length - 1 && node.end !== start) {
+					break;
 				}
+
+				if (node.type === 'Comment') {
+					prev_comment = node;
+					break;
+				} else if (node.type !== 'Text' || node.data.trim()) {
+					break;
+				}
+			}
+			if (prev_comment) {
+				// We take advantage of the fact that the root will never have leadingComments set,
+				// and set the previous comment to it so that the warning mechanism can later
+				// inspect the root and see if there was a html comment before it silencing specific warnings.
+				content.content.leadingComments = [{ type: 'Line', value: prev_comment.data }];
+			}
+
+			if (content.context === 'module') {
+				if (current.module) error(start, 'duplicate-script-element');
+				current.module = content;
+			} else {
+				if (current.instance) error(start, 'duplicate-script-element');
+				current.instance = content;
 			}
 		} else {
 			const content = read_style(parser, start, element.attributes);
 
-			if (content) {
-				if (current.css) error(start, 'duplicate-style-element');
-				current.css = content;
-			}
+			if (current.css) error(start, 'duplicate-style-element');
+			current.css = content;
 		}
 		return;
 	}

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

@@ -300,6 +300,25 @@ export function analyze_component(root, options) {
 					declaration.initial.source.value === 'svelte/store'
 				))
 		) {
+			let is_nested_store_subscription = false;
+			for (const reference of references) {
+				for (let i = reference.path.length - 1; i >= 0; i--) {
+					const scope =
+						scopes.get(reference.path[i]) ||
+						module.scopes.get(reference.path[i]) ||
+						instance.scopes.get(reference.path[i]);
+					if (scope) {
+						const owner = scope?.owner(store_name);
+						is_nested_store_subscription =
+							!!owner && owner !== module.scope && owner !== instance.scope;
+						break;
+					}
+				}
+			}
+			if (is_nested_store_subscription) {
+				error(references[0].node, 'illegal-store-subscription');
+			}
+
 			if (options.runes !== false) {
 				if (declaration === null && /[a-z]/.test(store_name[0])) {
 					error(references[0].node, 'illegal-global', name);
@@ -442,6 +461,17 @@ export function analyze_component(root, options) {
 			);
 		}
 
+		for (const [name, binding] of instance.scope.declarations) {
+			if (binding.kind === 'prop' && binding.node.name !== '$$props') {
+				const references = binding.references.filter(
+					(r) => r.node !== binding.node && r.path.at(-1)?.type !== 'ExportSpecifier'
+				);
+				if (!references.length && !instance.scope.declarations.has(`$${name}`)) {
+					warn(warnings, binding.node, [], 'unused-export-let', name);
+				}
+			}
+		}
+
 		analysis.reactive_statements = order_reactive_statements(analysis.reactive_statements);
 	}
 
@@ -590,6 +620,17 @@ const legacy_scope_tweaker = {
 
 		state.reactive_statements.set(node, reactive_statement);
 
+		// Ideally this would be in the validation file, but that isn't possible because this visitor
+		// calls "next" before setting the reactive statements.
+		if (
+			reactive_statement.dependencies.size &&
+			[...reactive_statement.dependencies].every(
+				(d) => d.scope === state.analysis.module.scope && d.declaration_kind !== 'const'
+			)
+		) {
+			warn(state.analysis.warnings, node, path, 'module-script-reactive-declaration');
+		}
+
 		if (
 			node.body.type === 'ExpressionStatement' &&
 			node.body.expression.type === 'AssignmentExpression'
@@ -710,7 +751,8 @@ const legacy_scope_tweaker = {
 				if (
 					binding.kind === 'state' ||
 					binding.kind === 'frozen_state' ||
-					(binding.kind === 'normal' && binding.declaration_kind === 'let')
+					(binding.kind === 'normal' &&
+						(binding.declaration_kind === 'let' || binding.declaration_kind === 'var'))
 				) {
 					binding.kind = 'prop';
 					if (specifier.exported.name !== specifier.local.name) {