Skip to content

feat: warn on unknown warning codes in runes mode #11549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
May 14, 2024
Merged
9 changes: 9 additions & 0 deletions packages/svelte/messages/compile-warnings/misc.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
## legacy_code

> `%code%` is no longer valid — please use `%suggestion%` instead

## unknown_code

> `%code%` is not a recognised code

> `%code%` is not a recognised code (did you mean `%suggestion%`?)
16 changes: 14 additions & 2 deletions packages/svelte/scripts/process-messages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function transform(name, dest) {

const comments = [];

const ast = acorn.parse(source, {
let ast = acorn.parse(source, {
ecmaVersion: 'latest',
sourceType: 'module',
onComment: (block, value, start, end) => {
Expand All @@ -80,7 +80,7 @@ function transform(name, dest) {
}
});

walk(ast, null, {
ast = walk(ast, null, {
_(node, { next }) {
let comment;

Expand All @@ -100,6 +100,18 @@ function transform(name, dest) {
node.trailingComments = [comments.shift()];
}
}
},
// @ts-expect-error
Identifier(node, context) {
if (node.name === 'CODES') {
return {
type: 'ArrayExpression',
elements: Object.keys(messages[name]).map((code) => ({
type: 'Literal',
value: code
}))
};
}
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ function w(node, code, message) {
});
}

export const codes = CODES;

/**
* MESSAGE
* @param {null | NodeLike} node
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/compiler/legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ export function convert(source, ast) {
Comment(node) {
return {
...node,
ignores: extract_svelte_ignore(node.data)
ignores: extract_svelte_ignore(node.start, node.data, false)
};
},
ComplexSelector(node) {
Expand Down
3 changes: 3 additions & 0 deletions packages/svelte/src/compiler/phases/2-analyze/a11y.js
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,9 @@ function check_element(node, state) {
for (const attribute of node.attributes) {
if (attribute.type !== 'Attribute') continue;

// @ts-expect-error gross hack
attribute.ignores = node.ignores;

const name = attribute.name.toLowerCase();
// aria-props
if (name.startsWith('aria-')) {
Expand Down
21 changes: 15 additions & 6 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -559,10 +559,14 @@ export function analyze_component(root, source, options) {
prune(analysis.css.ast, element);
}

if (
!analysis.css.ast.content.comment ||
!extract_svelte_ignore(analysis.css.ast.content.comment.data).includes('css_unused_selector')
) {
const { comment } = analysis.css.ast.content;
const should_ignore_unused =
comment &&
extract_svelte_ignore(comment.start, comment.data, analysis.runes).includes(
'css_unused_selector'
);

if (!should_ignore_unused) {
warn_unused(analysis.css.ast);
}

Expand Down Expand Up @@ -1102,7 +1106,8 @@ const common_visitors = {
const ignores = [];

for (const comment of comments) {
ignores.push(...extract_svelte_ignore(comment.value));
const start = /** @type {any} */ (comment).start + 2;
ignores.push(...extract_svelte_ignore(start, comment.value, context.state.analysis.runes));
}

if (ignores.length > 0) {
Expand Down Expand Up @@ -1133,7 +1138,11 @@ const common_visitors = {
}

if (child.type === 'Comment') {
ignores.push(...extract_svelte_ignore(child.data));
const start =
child.start +
(context.state.analysis.source.slice(child.start, child.start + 4) === '<!--' ? 4 : 2);

ignores.push(...extract_svelte_ignore(start, child.data, context.state.analysis.runes));
} else {
const combined_ignores = new Set(context.state.ignores);
for (const ignore of ignores) combined_ignores.add(ignore);
Expand Down
59 changes: 50 additions & 9 deletions packages/svelte/src/compiler/utils/extract_svelte_ignore.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,58 @@
import { regex_whitespace } from '../phases/patterns.js';
import fuzzymatch from '../phases/1-parse/utils/fuzzymatch.js';
import * as w from '../warnings.js';

const regex_svelte_ignore = /^\s*svelte-ignore\s+([\s\S]+)\s*$/m;
const regex_svelte_ignore = /^\s*svelte-ignore\s/;

/** @type {Record<string, string>} */
const replacements = {
'non-top-level-reactive-declaration': 'reactive_declaration_invalid_placement'
};

/**
* @param {number} offset
* @param {string} text
* @param {boolean} runes
* @returns {string[]}
*/
export function extract_svelte_ignore(text) {
export function extract_svelte_ignore(offset, text, runes) {
const match = regex_svelte_ignore.exec(text);
return match
? match[1]
.split(regex_whitespace)
.map(/** @param {any} x */ (x) => x.trim())
.filter(Boolean)
: [];
if (!match) return [];

let length = match[0].length;
offset += length;

/** @type {string[]} */
const ignores = [];

// Warnings have to be separated by commas, everything after is interpreted as prose
for (const match of text.slice(length).matchAll(/([\w$-]+)(,)?/gm)) {
const code = match[1];

ignores.push(code);

if (!w.codes.includes(code)) {
const replacement = replacements[code] ?? code.replace(/-/g, '_');

if (runes) {
// The type cast is for some reason necessary to pass the type check in CI
const start = offset + /** @type {number} */ (match.index);
const end = start + code.length;

if (w.codes.includes(replacement)) {
w.legacy_code({ start, end }, code, replacement);
} else {
const suggestion = fuzzymatch(code, w.codes);
w.unknown_code({ start, end }, code, suggestion);
}
} else if (w.codes.includes(replacement)) {
ignores.push(replacement);
}
}

if (!match[2]) {
break;
}
}

return ignores;
}
93 changes: 93 additions & 0 deletions packages/svelte/src/compiler/warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,79 @@ function w(node, code, message) {
});
}

export const codes = [
"a11y_accesskey",
"a11y_aria_activedescendant_has_tabindex",
"a11y_aria_attributes",
"a11y_autocomplete_valid",
"a11y_autofocus",
"a11y_click_events_have_key_events",
"a11y_distracting_elements",
"a11y_figcaption_index",
"a11y_figcaption_parent",
"a11y_hidden",
"a11y_img_redundant_alt",
"a11y_incorrect_aria_attribute_type",
"a11y_incorrect_aria_attribute_type_boolean",
"a11y_incorrect_aria_attribute_type_id",
"a11y_incorrect_aria_attribute_type_idlist",
"a11y_incorrect_aria_attribute_type_integer",
"a11y_incorrect_aria_attribute_type_token",
"a11y_incorrect_aria_attribute_type_tokenlist",
"a11y_incorrect_aria_attribute_type_tristate",
"a11y_interactive_supports_focus",
"a11y_invalid_attribute",
"a11y_label_has_associated_control",
"a11y_media_has_caption",
"a11y_misplaced_role",
"a11y_misplaced_scope",
"a11y_missing_attribute",
"a11y_missing_content",
"a11y_mouse_events_have_key_events",
"a11y_no_abstract_role",
"a11y_no_interactive_element_to_noninteractive_role",
"a11y_no_noninteractive_element_interactions",
"a11y_no_noninteractive_element_to_interactive_role",
"a11y_no_noninteractive_tabindex",
"a11y_no_redundant_roles",
"a11y_no_static_element_interactions",
"a11y_positive_tabindex",
"a11y_role_has_required_aria_props",
"a11y_role_supports_aria_props",
"a11y_role_supports_aria_props_implicit",
"a11y_unknown_aria_attribute",
"a11y_unknown_role",
"legacy_code",
"unknown_code",
"options_deprecated_accessors",
"options_deprecated_immutable",
"options_missing_custom_element",
"options_removed_enable_sourcemap",
"options_removed_hydratable",
"options_removed_loop_guard_timeout",
"options_renamed_ssr_dom",
"derived_iife",
"export_let_unused",
"non_reactive_update",
"perf_avoid_inline_class",
"perf_avoid_nested_class",
"reactive_declaration_invalid_placement",
"reactive_declaration_module_script",
"state_referenced_locally",
"store_rune_conflict",
"css_unused_selector",
"attribute_avoid_is",
"attribute_global_event_reference",
"attribute_illegal_colon",
"attribute_invalid_property_name",
"bind_invalid_each_rest",
"block_empty",
"component_name_lowercase",
"element_invalid_self_closing_tag",
"event_directive_deprecated",
"slot_element_deprecated"
];

/**
* Avoid using accesskey
* @param {null | NodeLike} node
Expand Down Expand Up @@ -414,6 +487,26 @@ export function a11y_unknown_role(node, role, suggestion) {
w(node, "a11y_unknown_role", suggestion ? `Unknown role '${role}'. Did you mean '${suggestion}'?` : `Unknown role '${role}'`);
}

/**
* `%code%` is no longer valid — please use `%suggestion%` instead
* @param {null | NodeLike} node
* @param {string} code
* @param {string} suggestion
*/
export function legacy_code(node, code, suggestion) {
w(node, "legacy_code", `\`${code}\` is no longer valid — please use \`${suggestion}\` instead`);
}

/**
* `%code%` is not a recognised code (did you mean `%suggestion%`?)
* @param {null | NodeLike} node
* @param {string} code
* @param {string | undefined | null} [suggestion]
*/
export function unknown_code(node, code, suggestion) {
w(node, "unknown_code", suggestion ? `\`${code}\` is not a recognised code (did you mean \`${suggestion}\`?)` : `\`${code}\` is not a recognised code`);
}

/**
* The `accessors` option has been deprecated. It will have no effect in runes mode
* @param {null | NodeLike} node
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<!-- svelte-ignore foo bar -->
<!-- svelte-ignore foo, bar -->
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
"html": {
"type": "Fragment",
"start": 0,
"end": 30,
"end": 31,
"children": [
{
"type": "Comment",
"start": 0,
"end": 30,
"data": " svelte-ignore foo bar ",
"end": 31,
"data": " svelte-ignore foo, bar ",
"ignores": ["foo", "bar"]
}
]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<script>
function foo() {
// svelte-ignore non-top-level-reactive-declaration
$: x = 1;
}
</script>

<!-- svelte-ignore a11y-missing-attribute -->
<div>
<img src="this-is-fine.jpg">
</div>

<!-- svelte-ignore a11y-misplaced-scope -->
<div scope></div>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[]
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@
<marquee>but this is still discouraged</marquee>
</div>

<!-- svelte-ignore a11y_misplaced_scope -->
<div scope></div>

<img src="potato.jpg">
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
"code": "a11y_missing_attribute",
"end": {
"column": 22,
"line": 7
"line": 10
},
"message": "`<img>` element should have an alt attribute",
"start": {
"column": 0,
"line": 7
"line": 10
}
}
]
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!-- svelte-ignore a11y_figcaption_parent a11y_missing_attribute -->
<!-- svelte-ignore a11y_figcaption_parent, a11y_missing_attribute -->
<div>
<figure>
<img src="potato.jpg">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!-- svelte-ignore a11y_missing_attribute
<!-- svelte-ignore a11y_missing_attribute,
a11y_distracting_elements -->
<div>
<img src="this-is-fine.jpg">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!-- svelte-ignore a11y_missing_attribute a11y_distracting_elements -->
<!-- svelte-ignore a11y_missing_attribute, a11y_distracting_elements -->
<div>
<img src="this-is-fine.jpg">
<marquee>but this is still discouraged</marquee>
Expand Down
20 changes: 20 additions & 0 deletions packages/svelte/tests/validator/samples/unknown-code/input.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<svelte:options runes={true} />

<!-- svelte-ignore a11y-missing-attribute -->
<div>
<img src="this-is-fine.jpg">
</div>

<!-- svelte-ignore ally_missing_attribute -->
<div>
<img src="this-is-fine.jpg">
</div>

<!-- svelte-ignore a11y-misplaced-scope -->
<div scope></div>

<!-- svelte-ignore a11y_misplaced_scope this is some prose -->
<div scope></div>

<!-- svelte-ignore a11y_misplaced_scope this_is some-ambiguous prose -->
<div scope></div>
Loading
Loading