diff --git a/.changeset/twelve-shrimps-run.md b/.changeset/twelve-shrimps-run.md new file mode 100644 index 0000000000..70b6a4a527 --- /dev/null +++ b/.changeset/twelve-shrimps-run.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: tighten up `$` prefix validation diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassDeclaration.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassDeclaration.js index 49be955eba..ec865c8313 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassDeclaration.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassDeclaration.js @@ -1,12 +1,17 @@ /** @import { ClassDeclaration } from 'estree' */ /** @import { Context } from '../types' */ import * as w from '../../../warnings.js'; +import { validate_identifier_name } from './shared/utils.js'; /** * @param {ClassDeclaration} node * @param {Context} context */ export function ClassDeclaration(node, context) { + if (context.state.analysis.runes) { + validate_identifier_name(context.state.scope.get(node.id.name)); + } + // 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. diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/FunctionDeclaration.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/FunctionDeclaration.js index 6d7074628e..2e996431d1 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/FunctionDeclaration.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/FunctionDeclaration.js @@ -1,11 +1,16 @@ /** @import { FunctionDeclaration } from 'estree' */ /** @import { Context } from '../types' */ import { visit_function } from './shared/function.js'; +import { validate_identifier_name } from './shared/utils.js'; /** * @param {FunctionDeclaration} node * @param {Context} context */ export function FunctionDeclaration(node, context) { + if (context.state.analysis.runes) { + validate_identifier_name(context.state.scope.get(node.id.name)); + } + visit_function(node, context); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/VariableDeclarator.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/VariableDeclarator.js index 6d9b3e29b9..a7d08d315d 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/VariableDeclarator.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/VariableDeclarator.js @@ -2,7 +2,7 @@ /** @import { Binding } from '#compiler' */ /** @import { Context } from '../types' */ import { get_rune } from '../../scope.js'; -import { ensure_no_module_import_conflict } from './shared/utils.js'; +import { ensure_no_module_import_conflict, validate_identifier_name } from './shared/utils.js'; import * as e from '../../../errors.js'; import { extract_paths } from '../../../utils/ast.js'; import { equal } from '../../../utils/assert.js'; @@ -17,6 +17,11 @@ export function VariableDeclarator(node, context) { if (context.state.analysis.runes) { const init = node.init; const rune = get_rune(init, context.state.scope); + const paths = extract_paths(node.id); + + for (const path of paths) { + validate_identifier_name(context.state.scope.get(/** @type {Identifier} */ (path.node).name)); + } // TODO feels like this should happen during scope creation? if ( @@ -26,7 +31,7 @@ export function VariableDeclarator(node, context) { rune === '$derived.by' || rune === '$props' ) { - for (const path of extract_paths(node.id)) { + for (const path of paths) { // @ts-ignore this fails in CI for some insane reason const binding = /** @type {Binding} */ (context.state.scope.get(path.node.name)); binding.kind = diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js index dad55a1152..266016fcc8 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js @@ -1,5 +1,5 @@ -/** @import { AssignmentExpression, Expression, Pattern, PrivateIdentifier, Super, UpdateExpression, VariableDeclarator } from 'estree' */ -/** @import { AST } from '#compiler' */ +/** @import { AssignmentExpression, Expression, Identifier, Pattern, PrivateIdentifier, Super, UpdateExpression, VariableDeclarator } from 'estree' */ +/** @import { AST, Binding } from '#compiler' */ /** @import { AnalysisState, Context } from '../../types' */ /** @import { Scope } from '../../../scope' */ /** @import { NodeLike } from '../../../../errors.js' */ @@ -196,3 +196,32 @@ export function is_pure(node, context) { // TODO add more cases (safe Svelte imports, etc) return false; } + +/** + * Checks if the name is valid, which it is when it's not starting with (or is) a dollar sign or if it's a function parameter. + * The second argument is the depth of the scope, which is there for backwards compatibility reasons: In Svelte 4, you + * were allowed to define `$`-prefixed variables anywhere below the top level of components. Once legacy mode is gone, this + * argument can be removed / the call sites adjusted accordingly. + * @param {Binding | null} binding + * @param {number | undefined} [function_depth] + */ +export function validate_identifier_name(binding, function_depth) { + if (!binding) return; + + const declaration_kind = binding.declaration_kind; + + if ( + declaration_kind !== 'synthetic' && + declaration_kind !== 'param' && + declaration_kind !== 'rest_param' && + (!function_depth || function_depth <= 1) + ) { + const node = binding.node; + + if (node.name === '$') { + e.dollar_binding_invalid(node); + } else if (node.name.startsWith('$')) { + e.dollar_prefix_invalid(node); + } + } +} diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 2f25cb251d..c1cf1e4055 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -14,6 +14,7 @@ import { } from '../utils/ast.js'; import { is_reserved, is_rune } from '../../utils.js'; import { determine_slot } from '../utils/slot.js'; +import { validate_identifier_name } from './2-analyze/visitors/shared/utils.js'; export class Scope { /** @type {ScopeRoot} */ @@ -78,20 +79,6 @@ export class Scope { * @returns {Binding} */ declare(node, kind, declaration_kind, initial = null) { - if (node.name === '$') { - e.dollar_binding_invalid(node); - } - - if ( - node.name.startsWith('$') && - declaration_kind !== 'synthetic' && - declaration_kind !== 'param' && - declaration_kind !== 'rest_param' && - this.function_depth <= 1 - ) { - e.dollar_prefix_invalid(node); - } - if (this.parent) { if (declaration_kind === 'var' && this.#porous) { return this.parent.declare(node, kind, declaration_kind); @@ -123,6 +110,9 @@ export class Scope { prop_alias: null, metadata: null }; + + validate_identifier_name(binding, this.function_depth); + this.declarations.set(node.name, binding); this.root.conflicts.add(node.name); return binding; diff --git a/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-legacy/_config.js b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-legacy/_config.js new file mode 100644 index 0000000000..cd7851d05e --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-legacy/_config.js @@ -0,0 +1,9 @@ +import { test } from '../../test'; + +export default test({ + error: { + code: 'dollar_binding_invalid', + message: 'The $ name is reserved, and cannot be used for variables and imports', + position: [108, 109] + } +}); diff --git a/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-legacy/main.svelte b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-legacy/main.svelte new file mode 100644 index 0000000000..3276e84bec --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-legacy/main.svelte @@ -0,0 +1,11 @@ + + + diff --git a/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration/_config.js b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes-2/_config.js similarity index 100% rename from packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration/_config.js rename to packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes-2/_config.js diff --git a/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes-2/main.svelte b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes-2/main.svelte new file mode 100644 index 0000000000..e24add5453 --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes-2/main.svelte @@ -0,0 +1,8 @@ + + + diff --git a/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes/_config.js b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes/_config.js new file mode 100644 index 0000000000..cd7a4ab291 --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes/_config.js @@ -0,0 +1,8 @@ +import { test } from '../../test'; + +export default test({ + error: { + code: 'dollar_binding_invalid', + message: 'The $ name is reserved, and cannot be used for variables and imports' + } +}); diff --git a/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration/main.svelte b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes/main.svelte similarity index 50% rename from packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration/main.svelte rename to packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes/main.svelte index 019c9cf7b4..42e702447f 100644 --- a/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration/main.svelte +++ b/packages/svelte/tests/compiler-errors/samples/dollar-binding-declaration-runes/main.svelte @@ -1,3 +1,5 @@ + +