Skip to content

Commit

Permalink
feat: warn on unknown warning codes in runes mode (#11549)
Browse files Browse the repository at this point in the history
Related to #11414

---------

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
  • Loading branch information
Rich-Harris and dummdidumm committed May 14, 2024
1 parent f6b8004 commit 5cb432b
Show file tree
Hide file tree
Showing 19 changed files with 308 additions and 27 deletions.
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 @@ -1105,7 +1109,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 @@ -1136,7 +1141,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

0 comments on commit 5cb432b

Please sign in to comment.