Merge bad0395e54 into ce89035ecb

.changeset/grumpy-lands-raise.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: state_referenced_locally not reported for declaration tags reading outer-scope state

packages/svelte/src/compiler/phases/2-analyze/types.d.ts

@@ -29,6 +29,14 @@ export interface AnalysisState {
 
 	function_depth: number;
 
+	/**
+	 * Set while visiting the declaration of a `{let ...}`/`{const ...}` declaration tag: the
+	 * `function_depth` that applied just outside the tag, i.e. the depth a plain reference at
+	 * that same point in the template would use. Lets `state_referenced_locally` also catch
+	 * non-closure reads of state declared *outside* the tag, not just within it.
+	 */
+	outer_function_depth?: number;
+
 	// legacy stuff
 	reactive_statement: null | ReactiveStatement;
 

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

@@ -249,6 +249,9 @@ export function CallExpression(node, context) {
 			...context.state,
 			function_depth: context.state.function_depth + 1,
 			derived_function_depth: context.state.function_depth + 1,
+			// `$derived(...)` is itself the reactive boundary declaration tags are missing, so
+			// an outer-scope reference read here is a legitimate deferred read, not a snapshot
+			outer_function_depth: undefined,
 			expression
 		});
 
@@ -259,7 +262,11 @@ export function CallExpression(node, context) {
 		// Tell surrounding declaration tag about metadata for correct calculation of blockers etc
 		if (context.state.in_declaration_tag) context.state.expression?.merge(expression);
 	} else if (rune === '$inspect') {
-		context.next({ ...context.state, function_depth: context.state.function_depth + 1 });
+		context.next({
+			...context.state,
+			function_depth: context.state.function_depth + 1,
+			outer_function_depth: undefined
+		});
 	} else {
 		context.next();
 	}

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

@@ -41,7 +41,8 @@ export function ConstTag(node, context) {
 		expression: node.metadata.expression,
 		// We're treating this like a $derived under the hood
 		function_depth: context.state.function_depth + 1,
-		derived_function_depth: context.state.function_depth + 1
+		derived_function_depth: context.state.function_depth + 1,
+		outer_function_depth: undefined
 	});
 
 	mark_async_declaration(context, node.metadata, [declaration]);

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

@@ -30,6 +30,7 @@ export function DeclarationTag(node, context) {
 		// `function_depth` we're tracking here (`set_scope` doesn't update `function_depth`).
 		// align them so that `state_referenced_locally` warnings are calculated correctly
 		function_depth: context.state.scope.function_depth,
+		outer_function_depth: context.state.function_depth - 1,
 		expression: node.metadata.expression
 	});
 

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

@@ -104,7 +104,8 @@ export function Identifier(node, context) {
 		if (
 			context.state.analysis.runes &&
 			node !== binding.node &&
-			context.state.function_depth === binding.scope.function_depth &&
+			(context.state.function_depth === binding.scope.function_depth ||
+				context.state.outer_function_depth === binding.scope.function_depth) &&
 			// If we have $state that can be proxied or frozen and isn't re-assigned, then that means
 			// it's likely not using a primitive value and thus this warning isn't that helpful.
 			((binding.kind === 'state' &&

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

@@ -23,7 +23,12 @@ export function SnippetBlock(node, context) {
 		}
 	}
 
-	context.next({ ...context.state, parent_element: null });
+	context.next({
+		...context.state,
+		parent_element: null,
+		function_depth: Math.max(context.state.scope.function_depth, context.state.function_depth) + 1,
+		outer_function_depth: undefined
+	});
 
 	const is_top_level = context.path.length === 1 && context.path[0].type === 'Fragment';
 

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

@@ -149,12 +149,19 @@ export function visit_component(node, context) {
 	const component_slots = new Set();
 
 	for (const slot_name in nodes) {
+		const slot_scope = node.metadata.scopes[slot_name];
+
 		/** @type {AnalysisState} */
 		const state = {
 			...context.state,
-			scope: node.metadata.scopes[slot_name],
+			scope: slot_scope,
 			parent_element: null,
-			component_slots
+			component_slots,
+			// slot content compiles to a snippet (an implicit `children` snippet for the default
+			// slot) - a real closure boundary, invoked separately and possibly many times - so
+			// state_referenced_locally shouldn't treat reads inside it as non-closure captures
+			function_depth: Math.max(slot_scope.function_depth, context.state.function_depth) + 1,
+			outer_function_depth: undefined
 		};
 
 		context.visit({ ...node.fragment, nodes: nodes[slot_name] }, state);

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

@@ -21,6 +21,10 @@ export function visit_function(node, context) {
 		// we generally want to use scope.function_depth unless we specifically increased
 		// that in state.function_depth (e.g. a derived)
 		function_depth: Math.max(context.state.scope.function_depth, context.state.function_depth) + 1,
+		// a real closure boundary makes any outer-scope reference inside it a legitimate
+		// deferred read, not a snapshot capture, so the `outer_function_depth` special-case
+		// from `DeclarationTag` no longer applies once we're inside one
+		outer_function_depth: undefined,
 		expression: null
 	});
 }

packages/svelte/tests/validator/samples/declaration-tag-state-referenced-locally-outer-scope/input.svelte

@@ -0,0 +1,44 @@
+<script>
+	import Child from './Child.svelte';
+
+	let count = $state(0);
+	let promise = $state(Promise.resolve(1));
+</script>
+
+<!-- this is a non-closure read of state declared in `<script>`, so it _should_ warn -->
+{let double1 = count}
+
+<!-- same, but nested inside a block; should still warn -->
+{#each [1, 2, 3] as item}
+	{let double2 = count}
+	{double2}
+{/each}
+
+<!-- same, but nested inside an await block; should still warn -->
+{#await promise then value}
+	{let double3 = count}
+	{double3}
+{/await}
+
+<!-- this reads `count` inside a closure, so it should _not_ warn -->
+{let fn = () => count}
+
+<!-- this reads `count` inside `$derived(...)`, so it should _not_ warn either -->
+{let derived1 = $derived(count * 2)}
+
+<!-- a snippet body is a real closure (invoked separately, possibly many times), so this
+     should _not_ warn either, even though it looks just like the top-level case above -->
+{#snippet mysnippet()}
+	{let double4 = count}
+	{double4}
+{/snippet}
+{@render mysnippet()}
+
+<!-- default slot content compiles to an implicit `children` snippet, so this should
+     _not_ warn either -->
+<Child>
+	{let double5 = count}
+	{double5}
+</Child>
+
+{double1}{fn}{derived1}

packages/svelte/tests/validator/samples/declaration-tag-state-referenced-locally-outer-scope/warnings.json

@@ -0,0 +1,38 @@
+[
+	{
+		"code": "state_referenced_locally",
+		"message": "This reference only captures the initial value of `count`. Did you mean to reference it inside a closure instead?",
+		"start": {
+			"line": 9,
+			"column": 15
+		},
+		"end": {
+			"line": 9,
+			"column": 20
+		}
+	},
+	{
+		"code": "state_referenced_locally",
+		"message": "This reference only captures the initial value of `count`. Did you mean to reference it inside a closure instead?",
+		"start": {
+			"line": 13,
+			"column": 16
+		},
+		"end": {
+			"line": 13,
+			"column": 21
+		}
+	},
+	{
+		"code": "state_referenced_locally",
+		"message": "This reference only captures the initial value of `count`. Did you mean to reference it inside a closure instead?",
+		"start": {
+			"line": 19,
+			"column": 16
+		},
+		"end": {
+			"line": 19,
+			"column": 21
+		}
+	}
+]