From 71601ba2e5310ed27a41a4060fc1bb3617c3c91c Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Sat, 17 Feb 2024 20:38:17 +0100 Subject: [PATCH] fix: warn against accidental global event referenced (#10442) * fix: warn against accidental global event referenced closes #10393 * remove list * remove else * tweak * update test --------- Co-authored-by: Rich Harris --- .changeset/chilly-snakes-scream.md | 5 ++++ .../compiler/phases/2-analyze/validation.js | 27 +++++++++++++++---- packages/svelte/src/compiler/utils/ast.js | 2 +- packages/svelte/src/compiler/warnings.js | 5 +++- .../samples/global-event-reference/_config.js | 3 +++ .../global-event-reference/input.svelte | 9 +++++++ .../global-event-reference/warnings.json | 26 ++++++++++++++++++ 7 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 .changeset/chilly-snakes-scream.md create mode 100644 packages/svelte/tests/validator/samples/global-event-reference/_config.js create mode 100644 packages/svelte/tests/validator/samples/global-event-reference/input.svelte create mode 100644 packages/svelte/tests/validator/samples/global-event-reference/warnings.json diff --git a/.changeset/chilly-snakes-scream.md b/.changeset/chilly-snakes-scream.md new file mode 100644 index 0000000000..dfdc19458e --- /dev/null +++ b/.changeset/chilly-snakes-scream.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: warn against accidental global event referenced diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index 1a854ce478..959f508742 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -1,5 +1,11 @@ import { error } from '../../errors.js'; -import { extract_identifiers, get_parent, is_text_attribute, object } from '../../utils/ast.js'; +import { + extract_identifiers, + get_parent, + is_expression_attribute, + is_text_attribute, + object +} from '../../utils/ast.js'; import { warn } from '../../warnings.js'; import fuzzymatch from '../1-parse/utils/fuzzymatch.js'; import { disallowed_parapgraph_contents, interactive_elements } from '../1-parse/utils/html.js'; @@ -66,12 +72,23 @@ function validate_element(node, context) { } if (attribute.name.startsWith('on') && attribute.name.length > 2) { + if (!is_expression_attribute(attribute)) { + error(attribute, 'invalid-event-attribute-value'); + } + + const value = attribute.value[0].expression; if ( - attribute.value === true || - is_text_attribute(attribute) || - attribute.value.length > 1 + value.type === 'Identifier' && + value.name === attribute.name && + !context.state.scope.get(value.name) ) { - error(attribute, 'invalid-event-attribute-value'); + warn( + context.state.analysis.warnings, + attribute, + context.path, + 'global-event-reference', + attribute.name + ); } } diff --git a/packages/svelte/src/compiler/utils/ast.js b/packages/svelte/src/compiler/utils/ast.js index 33bdc451c8..464c2081f3 100644 --- a/packages/svelte/src/compiler/utils/ast.js +++ b/packages/svelte/src/compiler/utils/ast.js @@ -36,7 +36,7 @@ export function is_text_attribute(attribute) { * @param {import('#compiler').Attribute} attribute * @returns {attribute is import('#compiler').Attribute & { value: [import('#compiler').ExpressionTag] }} */ -function is_expression_attribute(attribute) { +export function is_expression_attribute(attribute) { return ( attribute.value !== true && attribute.value.length === 1 && diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 9a6124f023..2f8308e27f 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -12,7 +12,10 @@ const css = { /** @satisfies {Warnings} */ const attributes = { - 'avoid-is': () => 'The "is" attribute is not supported cross-browser and should be avoided' + 'avoid-is': () => 'The "is" attribute is not supported cross-browser and should be avoided', + /** @param {string} name */ + 'global-event-reference': (name) => + `You are referencing globalThis.${name}. Did you forget to declare a variable with that name?` }; /** @satisfies {Warnings} */ diff --git a/packages/svelte/tests/validator/samples/global-event-reference/_config.js b/packages/svelte/tests/validator/samples/global-event-reference/_config.js new file mode 100644 index 0000000000..f47bee71df --- /dev/null +++ b/packages/svelte/tests/validator/samples/global-event-reference/_config.js @@ -0,0 +1,3 @@ +import { test } from '../../test'; + +export default test({}); diff --git a/packages/svelte/tests/validator/samples/global-event-reference/input.svelte b/packages/svelte/tests/validator/samples/global-event-reference/input.svelte new file mode 100644 index 0000000000..9a34449b42 --- /dev/null +++ b/packages/svelte/tests/validator/samples/global-event-reference/input.svelte @@ -0,0 +1,9 @@ + + + + + + + diff --git a/packages/svelte/tests/validator/samples/global-event-reference/warnings.json b/packages/svelte/tests/validator/samples/global-event-reference/warnings.json new file mode 100644 index 0000000000..ccc1e482b5 --- /dev/null +++ b/packages/svelte/tests/validator/samples/global-event-reference/warnings.json @@ -0,0 +1,26 @@ +[ + { + "code": "global-event-reference", + "message": "You are referencing globalThis.onkeydown. Did you forget to declare a variable with that name?", + "start": { + "column": 8, + "line": 8 + }, + "end": { + "column": 19, + "line": 8 + } + }, + { + "code": "global-event-reference", + "message": "You are referencing globalThis.onkeydown. Did you forget to declare a variable with that name?", + "start": { + "column": 8, + "line": 9 + }, + "end": { + "column": 29, + "line": 9 + } + } +]