From bb71afa8216f994686e357131816fe2538e0ce21 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 23 Apr 2024 13:34:54 -0400 Subject: [PATCH 1/8] rename warnings.js to warnings-tmp.js --- packages/svelte/src/compiler/phases/2-analyze/a11y.js | 6 +++--- .../svelte/src/compiler/phases/2-analyze/css/css-warn.js | 2 +- packages/svelte/src/compiler/phases/2-analyze/index.js | 2 +- packages/svelte/src/compiler/phases/2-analyze/validation.js | 2 +- .../svelte/src/compiler/{warnings.js => warnings-tmp.js} | 0 5 files changed, 6 insertions(+), 6 deletions(-) rename packages/svelte/src/compiler/{warnings.js => warnings-tmp.js} (100%) diff --git a/packages/svelte/src/compiler/phases/2-analyze/a11y.js b/packages/svelte/src/compiler/phases/2-analyze/a11y.js index 7d01c6fdb330..c31f20af40d7 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/a11y.js +++ b/packages/svelte/src/compiler/phases/2-analyze/a11y.js @@ -7,7 +7,7 @@ import { regex_starts_with_vowel, regex_whitespaces } from '../patterns.js'; -import { warn } from '../../warnings.js'; +import { warn } from '../../warnings-tmp.js'; import fuzzymatch from '../1-parse/utils/fuzzymatch.js'; import { is_event_attribute, is_text_attribute } from '../../utils/ast.js'; import { ContentEditableBindings } from '../constants.js'; @@ -673,10 +673,10 @@ function check_element(node, state) { if (state.options.namespace === 'foreign') return; /** - * @template {keyof import('../../warnings.js').AllWarnings} T + * @template {keyof import('../../warnings-tmp.js').AllWarnings} T * @param {{ start?: number, end?: number }} node * @param {T} code - * @param {Parameters} args + * @param {Parameters} args * @returns {void} */ const push_warning = (node, code, ...args) => warn(state.analysis.warnings, node, code, ...args); diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-warn.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-warn.js index e2d137198bd3..d2514187a521 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-warn.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-warn.js @@ -1,5 +1,5 @@ import { walk } from 'zimmerframe'; -import { warn } from '../../../warnings.js'; +import { warn } from '../../../warnings-tmp.js'; import { is_keyframes_node } from '../../css.js'; /** diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index d2a331e25393..57126747d877 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -14,7 +14,7 @@ import { ReservedKeywords, Runes, SVGElements } from '../constants.js'; import { Scope, ScopeRoot, create_scopes, get_rune, set_scope } from '../scope.js'; import { merge } from '../visitors.js'; import { validation_legacy, validation_runes, validation_runes_js } from './validation.js'; -import { warn } from '../../warnings.js'; +import { warn } from '../../warnings-tmp.js'; import check_graph_for_cycles from './utils/check_graph_for_cycles.js'; import { regex_starts_with_newline } from '../patterns.js'; import { create_attribute, is_element_node } from '../nodes.js'; diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index f4321b010158..0a0b214cd769 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -12,7 +12,7 @@ import { object, unwrap_optional } from '../../utils/ast.js'; -import { warn } from '../../warnings.js'; +import { warn } from '../../warnings-tmp.js'; import fuzzymatch from '../1-parse/utils/fuzzymatch.js'; import { binding_properties } from '../bindings.js'; import { diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings-tmp.js similarity index 100% rename from packages/svelte/src/compiler/warnings.js rename to packages/svelte/src/compiler/warnings-tmp.js From 34e8562f33fd94425e7c6f00355af0f91c8a279b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 23 Apr 2024 14:07:54 -0400 Subject: [PATCH 2/8] start porting warnings --- .../messages/compile-warnings/attributes.md | 15 ++++ .../svelte/messages/compile-warnings/css.md | 3 + .../svelte/scripts/process-messages/index.js | 1 + .../templates/compile-warnings.js | 51 +++++++++++ packages/svelte/src/compiler/warnings.js | 84 +++++++++++++++++++ 5 files changed, 154 insertions(+) create mode 100644 packages/svelte/messages/compile-warnings/attributes.md create mode 100644 packages/svelte/messages/compile-warnings/css.md create mode 100644 packages/svelte/scripts/process-messages/templates/compile-warnings.js create mode 100644 packages/svelte/src/compiler/warnings.js diff --git a/packages/svelte/messages/compile-warnings/attributes.md b/packages/svelte/messages/compile-warnings/attributes.md new file mode 100644 index 000000000000..dba95975e86a --- /dev/null +++ b/packages/svelte/messages/compile-warnings/attributes.md @@ -0,0 +1,15 @@ +## avoid_is + +The "is" attribute is not supported cross-browser and should be avoided + +## global_event_reference + +You are referencing globalThis.%name%. Did you forget to declare a variable with that name? + +## illegal_attribute_character + +Attributes should not contain ':' characters to prevent ambiguity with Svelte directives + +## invalid_html_attribute + +'%wrong%' is not a valid HTML attribute. Did you mean '%right%'? diff --git a/packages/svelte/messages/compile-warnings/css.md b/packages/svelte/messages/compile-warnings/css.md new file mode 100644 index 000000000000..1272fe69a178 --- /dev/null +++ b/packages/svelte/messages/compile-warnings/css.md @@ -0,0 +1,3 @@ +## css_unused_selector + +Unused CSS selector "%name%" diff --git a/packages/svelte/scripts/process-messages/index.js b/packages/svelte/scripts/process-messages/index.js index 52cb23aec398..74110add9dba 100644 --- a/packages/svelte/scripts/process-messages/index.js +++ b/packages/svelte/scripts/process-messages/index.js @@ -211,3 +211,4 @@ function transform(name, dest) { } transform('compile-errors', 'src/compiler/errors.js'); +transform('compile-warnings', 'src/compiler/warnings.js'); diff --git a/packages/svelte/scripts/process-messages/templates/compile-warnings.js b/packages/svelte/scripts/process-messages/templates/compile-warnings.js new file mode 100644 index 000000000000..0b4b6b7541be --- /dev/null +++ b/packages/svelte/scripts/process-messages/templates/compile-warnings.js @@ -0,0 +1,51 @@ +import { getLocator } from 'locate-character'; + +/** @typedef {{ start?: number, end?: number }} NodeLike */ + +// TODO no need to export these, it's temporary +/** @type {import('#compiler').Warning[]} */ +export let warnings = []; +/** @type {string | undefined} */ +export let filename; +export let locator = getLocator('', { offsetLine: 1 }); + +/** + * @param {{ + * source: string; + * filename: string | undefined; + * }} options + * @returns {import('#compiler').Warning[]} + */ +export function reset_warnings(options) { + filename = options.filename; + locator = getLocator(options.source, { offsetLine: 1 }); + + return (warnings = []); +} + +/** + * + * @param {null | NodeLike} node + * @param {string} code + * @param {string} message + */ +function w(node, code, message) { + // @ts-expect-error + if (node.ignores?.has(code)) return; + + warnings.push({ + code, + message, + filename, + start: node?.start !== undefined ? locator(node.start) : undefined, + end: node?.end !== undefined ? locator(node.end) : undefined + }); +} + +/** + * @param {null | NodeLike} node + * @param {string} PARAMETER + */ +export function CODE(node, PARAMETER) { + w(node, 'CODE', MESSAGE); +} diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js new file mode 100644 index 000000000000..51b5bdf17bf6 --- /dev/null +++ b/packages/svelte/src/compiler/warnings.js @@ -0,0 +1,84 @@ +/* This file is generated by scripts/process-messages.js. Do not edit! */ + +import { getLocator } from 'locate-character'; + +/** @typedef {{ start?: number, end?: number }} NodeLike */ +// TODO no need to export these, it's temporary +/** @type {import('#compiler').Warning[]} */ +export let warnings = []; +/** @type {string | undefined} */ +export let filename; +export let locator = getLocator('', { offsetLine: 1 }); + +/** + * @param {{ + * source: string; + * filename: string | undefined; + * }} options + * @returns {import('#compiler').Warning[]} + */ +export function reset_warnings(options) { + filename = options.filename; + locator = getLocator(options.source, { offsetLine: 1 }); + return warnings = []; +} + +/** + * + * @param {null | NodeLike} node + * @param {string} code + * @param {string} message + */ +function w(node, code, message) { + // @ts-expect-error + if (node.ignores?.has(code)) return; + + warnings.push({ + code, + message, + filename, + start: node?.start !== undefined ? locator(node.start) : undefined, + end: node?.end !== undefined ? locator(node.end) : undefined + }); +} + +/** + * @param {null | NodeLike} node + + */ +export function avoid_is(node) { + w(node, "avoid_is", "The \"is\" attribute is not supported cross-browser and should be avoided"); +} + +/** + * @param {null | NodeLike} node + * @param {string} name + */ +export function global_event_reference(node, name) { + w(node, "global_event_reference", `You are referencing globalThis.${name}. Did you forget to declare a variable with that name?`); +} + +/** + * @param {null | NodeLike} node + + */ +export function illegal_attribute_character(node) { + w(node, "illegal_attribute_character", "Attributes should not contain ':' characters to prevent ambiguity with Svelte directives"); +} + +/** + * @param {null | NodeLike} node + * @param {string} wrong + * @param {string} right + */ +export function invalid_html_attribute(node, wrong, right) { + w(node, "invalid_html_attribute", `'${wrong}' is not a valid HTML attribute. Did you mean '${right}'?`); +} + +/** + * @param {null | NodeLike} node + * @param {string} name + */ +export function css_unused_selector(node, name) { + w(node, "css_unused_selector", `Unused CSS selector "${name}"`); +} \ No newline at end of file From 4cc013fa6f33523e6753c686c440902acc7af9b7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 23 Apr 2024 14:08:01 -0400 Subject: [PATCH 3/8] centralise stuff --- packages/svelte/src/compiler/index.js | 8 +- .../src/compiler/phases/2-analyze/a11y.js | 90 ++++++++----------- .../compiler/phases/2-analyze/css/css-warn.js | 9 +- .../src/compiler/phases/2-analyze/index.js | 33 +++---- .../compiler/phases/2-analyze/validation.js | 50 ++++------- .../src/compiler/phases/3-transform/index.js | 43 +-------- .../svelte/src/compiler/phases/types.d.ts | 10 +-- packages/svelte/src/compiler/warnings-tmp.js | 12 +-- 8 files changed, 88 insertions(+), 167 deletions(-) diff --git a/packages/svelte/src/compiler/index.js b/packages/svelte/src/compiler/index.js index eee35bf2a81e..c2516a2e5fd1 100644 --- a/packages/svelte/src/compiler/index.js +++ b/packages/svelte/src/compiler/index.js @@ -8,6 +8,7 @@ import { remove_typescript_nodes } from './phases/1-parse/remove_typescript_node import { analyze_component, analyze_module } from './phases/2-analyze/index.js'; import { transform_component, transform_module } from './phases/3-transform/index.js'; import { validate_component_options, validate_module_options } from './validate-options.js'; +import { reset_warnings } from './warnings.js'; export { default as preprocess } from './preprocess/index.js'; /** @@ -20,6 +21,7 @@ export { default as preprocess } from './preprocess/index.js'; */ export function compile(source, options) { try { + const warnings = reset_warnings({ source, filename: options.filename }); const validated = validate_component_options(options, ''); let parsed = _parse(source); @@ -44,6 +46,7 @@ export function compile(source, options) { const analysis = analyze_component(parsed, source, combined_options); const result = transform_component(analysis, source, combined_options); + result.warnings = warnings; result.ast = to_public_ast(source, parsed, options.modernAst); return result; } catch (e) { @@ -65,9 +68,12 @@ export function compile(source, options) { */ export function compileModule(source, options) { try { + const warnings = reset_warnings({ source, filename: options.filename }); const validated = validate_module_options(options, ''); const analysis = analyze_module(parse_acorn(source, false), validated); - return transform_module(analysis, source, validated); + const result = transform_module(analysis, source, validated); + result.warnings = warnings; + return result; } catch (e) { if (e instanceof CompileError) { handle_compile_error(e, options.filename, source); diff --git a/packages/svelte/src/compiler/phases/2-analyze/a11y.js b/packages/svelte/src/compiler/phases/2-analyze/a11y.js index c31f20af40d7..17b4a94ebaca 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/a11y.js +++ b/packages/svelte/src/compiler/phases/2-analyze/a11y.js @@ -672,15 +672,6 @@ function check_element(node, state) { // foreign namespace means elements can have completely different meanings, therefore we don't check them if (state.options.namespace === 'foreign') return; - /** - * @template {keyof import('../../warnings-tmp.js').AllWarnings} T - * @param {{ start?: number, end?: number }} node - * @param {T} code - * @param {Parameters} args - * @returns {void} - */ - const push_warning = (node, code, ...args) => warn(state.analysis.warnings, node, code, ...args); - /** @type {Map} */ const attribute_map = new Map(); @@ -727,17 +718,17 @@ function check_element(node, state) { if (name.startsWith('aria-')) { if (invisible_elements.includes(node.name)) { // aria-unsupported-elements - push_warning(attribute, 'a11y-aria-attributes', node.name); + warn(attribute, 'a11y-aria-attributes', node.name); } const type = name.slice(5); if (!aria_attributes.includes(type)) { const match = fuzzymatch(type, aria_attributes); - push_warning(attribute, 'a11y-unknown-aria-attribute', type, match); + warn(attribute, 'a11y-unknown-aria-attribute', type, match); } if (name === 'aria-hidden' && regex_heading_tags.test(node.name)) { - push_warning(attribute, 'a11y-hidden', node.name); + warn(attribute, 'a11y-hidden', node.name); } // aria-proptypes @@ -747,7 +738,7 @@ function check_element(node, state) { if (value !== null && value !== undefined) { const schema = aria.get(/** @type {import('aria-query').ARIAProperty} */ (name)); if (schema !== undefined && !is_valid_aria_attribute_value(schema, value)) { - push_warning(attribute, 'a11y-incorrect-aria-attribute-type', schema, name); + warn(attribute, 'a11y-incorrect-aria-attribute-type', schema, name); } } @@ -758,7 +749,7 @@ function check_element(node, state) { !is_interactive_element(node.name, attribute_map) && !attribute_map.has('tabindex') ) { - push_warning(attribute, 'a11y-aria-activedescendant-has-tabindex'); + warn(attribute, 'a11y-aria-activedescendant-has-tabindex'); } } @@ -766,7 +757,7 @@ function check_element(node, state) { if (name === 'role') { if (invisible_elements.includes(node.name)) { // aria-unsupported-elements - push_warning(attribute, 'a11y-misplaced-role', node.name); + warn(attribute, 'a11y-misplaced-role', node.name); } const value = get_static_value(attribute); @@ -776,10 +767,10 @@ function check_element(node, state) { /** @type {import('aria-query').ARIARoleDefinitionKey} current_role */ (c_r); if (current_role && is_abstract_role(current_role)) { - push_warning(attribute, 'a11y-no-abstract-role', current_role); + warn(attribute, 'a11y-no-abstract-role', current_role); } else if (current_role && !aria_roles.includes(current_role)) { const match = fuzzymatch(current_role, aria_roles); - push_warning(attribute, 'a11y-unknown-role', current_role, match); + warn(attribute, 'a11y-unknown-role', current_role, match); } // no-redundant-roles @@ -788,7 +779,7 @@ function check_element(node, state) { //