From 289d568357bc7351e788ba1da236c4f83616fba7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 27 Nov 2023 09:40:19 -0500 Subject: [PATCH] less invasive approach --- .../src/compiler/phases/2-analyze/index.js | 27 +++++++--- packages/svelte/src/compiler/phases/scope.js | 52 ++++++++----------- .../warnings.json | 2 +- 3 files changed, 45 insertions(+), 36 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 8acea30c95..540a1b9912 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -38,7 +38,7 @@ function js(script, root, allow_reactive_declarations, parent) { body: [] }; - const { scope, scopes } = create_scopes(ast, root, allow_reactive_declarations, false, parent); + const { scope, scopes } = create_scopes(ast, root, allow_reactive_declarations, parent); return { ast, scope, scopes }; } @@ -191,7 +191,7 @@ function get_delegated_event(node, context) { * @returns {import('../types.js').Analysis} */ export function analyze_module(ast, options) { - const { scope, scopes } = create_scopes(ast, new ScopeRoot(), false, false, null); + const { scope, scopes } = create_scopes(ast, new ScopeRoot(), false, null); for (const [name, references] of scope.references) { if (name[0] !== '$' || ReservedKeywords.includes(name)) continue; @@ -242,7 +242,7 @@ export function analyze_component(root, options) { const module = js(root.module, scope_root, false, null); const instance = js(root.instance, scope_root, true, module.scope); - const { scope, scopes } = create_scopes(root.fragment, scope_root, false, true, instance.scope); + const { scope, scopes } = create_scopes(root.fragment, scope_root, false, instance.scope); /** @type {import('../types.js').Template} */ const template = { ast: root.fragment, scope, scopes }; @@ -416,9 +416,24 @@ export function analyze_component(root, options) { // warn on any nonstate declarations that are a) mutated and b) referenced in the template for (const scope of [module.scope, instance.scope]) { - for (const [name, binding] of scope.declarations) { - if (binding.kind === 'normal' && binding.mutated && binding.referenced_in_template) { - warn(warnings, binding.node, [], 'non-state-reference', name); + outer: for (const [name, binding] of scope.declarations) { + if (binding.kind === 'normal' && binding.mutated) { + for (const { path } of binding.references) { + if (path[0].type !== 'Fragment') continue; + for (let i = 1; i < path.length; i += 1) { + const type = path[i].type; + if ( + type === 'FunctionDeclaration' || + type === 'FunctionExpression' || + type === 'ArrowFunctionExpression' + ) { + continue; + } + } + + warn(warnings, binding.node, [], 'non-state-reference', name); + break outer; + } } } } diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 2143f4660c..69af8cc33d 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -169,9 +169,8 @@ export class Scope { /** * @param {import('estree').Identifier} node * @param {import('#compiler').SvelteNode[]} path - * @param {boolean} is_template */ - reference(node, path, is_template) { + reference(node, path) { let references = this.references.get(node.name); if (!references) this.references.set(node.name, (references = [])); @@ -179,10 +178,9 @@ export class Scope { const binding = this.declarations.get(node.name); if (binding) { - if (is_template) binding.referenced_in_template = true; binding.references.push({ node, path }); } else if (this.#parent) { - this.#parent.reference(node, path, is_template); + this.#parent.reference(node, path); } else { // no binding was found, and this is the top level scope, // which means this is a global @@ -217,11 +215,10 @@ export class ScopeRoot { * @param {import('#compiler').SvelteNode} ast * @param {ScopeRoot} root * @param {boolean} allow_reactive_declarations - * @param {boolean} is_template * @param {Scope | null} parent */ -export function create_scopes(ast, root, allow_reactive_declarations, is_template, parent) { - /** @typedef {{ scope: Scope, is_template: boolean }} State */ +export function create_scopes(ast, root, allow_reactive_declarations, parent) { + /** @typedef {{ scope: Scope }} State */ /** * A map of node->associated scope. A node appearing in this map does not necessarily mean that it created a scope @@ -232,9 +229,9 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat scopes.set(ast, scope); /** @type {State} */ - const state = { scope, is_template }; + const state = { scope }; - /** @type {[Scope, { node: import('estree').Identifier; path: import('#compiler').SvelteNode[]; is_template: boolean }][]} */ + /** @type {[Scope, { node: import('estree').Identifier; path: import('#compiler').SvelteNode[] }][]} */ const references = []; /** @type {[Scope, import('estree').Pattern | import('estree').MemberExpression][]} */ @@ -265,7 +262,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat const scope = state.scope.child(true); scopes.set(node, scope); - next({ ...state, scope }); + next({ scope }); }; /** @@ -274,7 +271,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat const SvelteFragment = (node, { state, next }) => { const scope = analyze_let_directives(node, state.scope); scopes.set(node, scope); - next({ ...state, scope }); + next({ scope }); }; /** @@ -320,10 +317,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat Identifier(node, { path, state }) { const parent = path.at(-1); if (parent && is_reference(node, /** @type {import('estree').Node} */ (parent))) { - references.push([ - state.scope, - { node, path: path.slice(), is_template: state.is_template } - ]); + references.push([state.scope, { node, path: path.slice() }]); } }, @@ -346,7 +340,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat } } - next({ ...state, scope }); + next({ scope }); }, SvelteFragment, @@ -354,7 +348,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat RegularElement: SvelteFragment, Component(node, { state, visit, path }) { - state.scope.reference(b.id(node.name), path, false); + state.scope.reference(b.id(node.name), path); // let:x from the default slot is a weird one: // Its scope only applies to children that are not slots themselves. @@ -378,7 +372,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat } else if (child.type === 'SnippetBlock') { visit(child); } else { - visit(child, { ...state, scope }); + visit(child, { scope }); } } }, @@ -412,7 +406,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat if (node.id) scope.declare(node.id, 'normal', 'function'); add_params(scope, node.params); - next({ scope, is_template: false }); + next({ scope }); }, FunctionDeclaration(node, { state, next }) { @@ -422,7 +416,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat scopes.set(node, scope); add_params(scope, node.params); - next({ scope, is_template: false }); + next({ scope }); }, ArrowFunctionExpression(node, { state, next }) { @@ -430,7 +424,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat scopes.set(node, scope); add_params(scope, node.params); - next({ scope, is_template: false }); + next({ scope }); }, ForStatement: create_block_scope, @@ -475,7 +469,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat state.scope.declare(id, 'normal', 'let'); } - next({ ...state, scope }); + next({ scope }); } else { next(); } @@ -513,13 +507,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat (node.key.type !== 'Identifier' || !node.index || node.key.name !== node.index); scope.declare(b.id(node.index), is_keyed ? 'derived' : 'normal', 'const'); } - if (node.key) visit(node.key, { ...state, scope }); + if (node.key) visit(node.key, { scope }); // children for (const child of node.body.nodes) { - visit(child, { ...state, scope }); + visit(child, { scope }); } - if (node.fallback) visit(node.fallback, { ...state, scope }); + if (node.fallback) visit(node.fallback, { scope }); // Check if inner scope shadows something from outer scope. // This is necessary because we need access to the array expression of the each block @@ -586,13 +580,13 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat } } - context.next({ ...state, scope: child_scope }); + context.next({ scope: child_scope }); }, Fragment: (node, context) => { const scope = context.state.scope.child(node.transparent); scopes.set(node, scope); - context.next({ ...state, scope }); + context.next({ scope }); }, BindDirective(node, context) { @@ -629,8 +623,8 @@ export function create_scopes(ast, root, allow_reactive_declarations, is_templat // we do this after the fact, so that we don't need to worry // about encountering references before their declarations - for (const [scope, { node, path, is_template }] of references) { - scope.reference(node, path, is_template); + for (const [scope, { node, path }] of references) { + scope.reference(node, path); } for (const [scope, node] of updates) { diff --git a/packages/svelte/tests/validator/samples/runes-state-rune-not-mutated/warnings.json b/packages/svelte/tests/validator/samples/runes-state-rune-not-mutated/warnings.json index 628f1f2e9d..5d2b639c8d 100644 --- a/packages/svelte/tests/validator/samples/runes-state-rune-not-mutated/warnings.json +++ b/packages/svelte/tests/validator/samples/runes-state-rune-not-mutated/warnings.json @@ -1,6 +1,6 @@ [ { - "code": "state-rune-not-mutated", + "code": "state-not-mutated", "end": { "column": 11, "line": 3