state_referenced_locally not reported for declaration tags reading outer-scope state

pull/18753/head
LeonardoRosaa 1 week ago
parent 7bc0a70fe6
commit c2f68865a3

@ -29,6 +29,14 @@ export interface AnalysisState {
function_depth: number; 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 // legacy stuff
reactive_statement: null | ReactiveStatement; reactive_statement: null | ReactiveStatement;

@ -249,6 +249,9 @@ export function CallExpression(node, context) {
...context.state, ...context.state,
function_depth: context.state.function_depth + 1, function_depth: context.state.function_depth + 1,
derived_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 expression
}); });
@ -259,7 +262,11 @@ export function CallExpression(node, context) {
// Tell surrounding declaration tag about metadata for correct calculation of blockers etc // Tell surrounding declaration tag about metadata for correct calculation of blockers etc
if (context.state.in_declaration_tag) context.state.expression?.merge(expression); if (context.state.in_declaration_tag) context.state.expression?.merge(expression);
} else if (rune === '$inspect') { } 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 { } else {
context.next(); context.next();
} }

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

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

@ -104,7 +104,8 @@ export function Identifier(node, context) {
if ( if (
context.state.analysis.runes && context.state.analysis.runes &&
node !== binding.node && 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 // 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. // it's likely not using a primitive value and thus this warning isn't that helpful.
((binding.kind === 'state' && ((binding.kind === 'state' &&

@ -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'; const is_top_level = context.path.length === 1 && context.path[0].type === 'Fragment';

@ -149,12 +149,19 @@ export function visit_component(node, context) {
const component_slots = new Set(); const component_slots = new Set();
for (const slot_name in nodes) { for (const slot_name in nodes) {
const slot_scope = node.metadata.scopes[slot_name];
/** @type {AnalysisState} */ /** @type {AnalysisState} */
const state = { const state = {
...context.state, ...context.state,
scope: node.metadata.scopes[slot_name], scope: slot_scope,
parent_element: null, 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); context.visit({ ...node.fragment, nodes: nodes[slot_name] }, state);

@ -21,6 +21,10 @@ export function visit_function(node, context) {
// we generally want to use scope.function_depth unless we specifically increased // we generally want to use scope.function_depth unless we specifically increased
// that in state.function_depth (e.g. a derived) // that in state.function_depth (e.g. a derived)
function_depth: Math.max(context.state.scope.function_depth, context.state.function_depth) + 1, 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 expression: null
}); });
} }

@ -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}

@ -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
}
}
]
Loading…
Cancel
Save