chore: refactor logic for ignoring warnings

packages/svelte/src/compiler/legacy.js

@@ -4,6 +4,7 @@ import {
 	regex_not_whitespace,
 	regex_starts_with_whitespaces
 } from './phases/patterns.js';
+import { extract_svelte_ignore } from './utils/extract_svelte_ignore.js';
 
 /**
  * Some of the legacy Svelte AST nodes remove whitespace from the start and end of their children.
@@ -197,7 +198,13 @@ export function convert(source, ast) {
 			ClassDirective(node) {
 				return { ...node, type: 'Class' };
 			},
-			ComplexSelector(node, { visit }) {
+			Comment(node) {
+				return {
+					...node,
+					ignores: extract_svelte_ignore(node.data)
+				};
+			},
+			ComplexSelector(node) {
 				const children = [];
 
 				for (const child of node.children) {

packages/svelte/src/compiler/phases/1-parse/state/element.js

@@ -1,5 +1,3 @@
-import { extract_svelte_ignore } from '../../../utils/extract_svelte_ignore.js';
-import fuzzymatch from '../utils/fuzzymatch.js';
 import { is_void } from '../utils/names.js';
 import read_expression from '../read/expression.js';
 import { read_script } from '../read/script.js';
@@ -87,8 +85,7 @@ export default function tag(parser) {
 			type: 'Comment',
 			start,
 			end: parser.index,
-			data,
-			ignores: extract_svelte_ignore(data)
+			data
 		});
 
 		return;

packages/svelte/src/compiler/phases/2-analyze/a11y.js

@@ -667,9 +667,8 @@ function get_static_text_value(attribute) {
 /**
  * @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} node
  * @param {import('./types.js').AnalysisState} state
- * @param {import('#compiler').SvelteNode[]} path
  */
-function check_element(node, state, path) {
+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;
 
@@ -680,8 +679,7 @@ function check_element(node, state, path) {
 	 * @param  {Parameters<import('../../warnings.js').AllWarnings[T]>} args
 	 * @returns {void}
 	 */
-	const push_warning = (node, code, ...args) =>
-		warn(state.analysis.warnings, node, path, code, ...args);
+	const push_warning = (node, code, ...args) => warn(state.analysis.warnings, node, code, ...args);
 
 	/** @type {Map<string, import('#compiler').Attribute>} */
 	const attribute_map = new Map();
@@ -1165,9 +1163,9 @@ function check_element(node, state, path) {
  */
 export const a11y_validators = {
 	RegularElement(node, context) {
-		check_element(node, context.state, context.path);
+		check_element(node, context.state);
 	},
 	SvelteElement(node, context) {
-		check_element(node, context.state, context.path);
+		check_element(node, context.state);
 	}
 };

packages/svelte/src/compiler/phases/2-analyze/css/css-warn.js

@@ -26,7 +26,7 @@ const visitors = {
 		if (!node.metadata.used) {
 			const content = context.state.stylesheet.content;
 			const text = content.styles.substring(node.start - content.start, node.end - content.start);
-			warn(context.state.warnings, node, context.path, 'css-unused-selector', text);
+			warn(context.state.warnings, node, 'css-unused-selector', text);
 		}
 
 		context.next();

packages/svelte/src/compiler/phases/2-analyze/index.js

@@ -24,6 +24,7 @@ import { analyze_css } from './css/css-analyze.js';
 import { prune } from './css/css-prune.js';
 import { hash } from './utils.js';
 import { warn_unused } from './css/css-warn.js';
+import { extract_svelte_ignore } from '../../utils/extract_svelte_ignore.js';
 
 /**
  * @param {import('#compiler').Script | null} script
@@ -330,7 +331,7 @@ export function analyze_component(root, source, options) {
 				} else if (declaration !== null && Runes.includes(/** @type {any} */ (name))) {
 					for (const { node, path } of references) {
 						if (path.at(-1)?.type === 'CallExpression') {
-							warn(warnings, node, [], 'store-with-rune-name', store_name);
+							warn(warnings, node, 'store-with-rune-name', store_name);
 						}
 					}
 				}
@@ -407,7 +408,7 @@ export function analyze_component(root, source, options) {
 	};
 
 	if (!options.customElement && root.options?.customElement) {
-		warn(analysis.warnings, root.options, [], 'missing-custom-element-compile-option');
+		warn(analysis.warnings, root.options, 'missing-custom-element-compile-option');
 	}
 
 	if (analysis.runes) {
@@ -433,7 +434,8 @@ export function analyze_component(root, source, options) {
 				component_slots: new Set(),
 				expression: null,
 				private_derived_state: [],
-				function_depth: scope.function_depth
+				function_depth: scope.function_depth,
+				ignores: new Set()
 			};
 
 			walk(
@@ -475,7 +477,8 @@ export function analyze_component(root, source, options) {
 				component_slots: new Set(),
 				expression: null,
 				private_derived_state: [],
-				function_depth: scope.function_depth
+				function_depth: scope.function_depth,
+				ignores: new Set()
 			};
 
 			walk(
@@ -495,7 +498,7 @@ export function analyze_component(root, source, options) {
 					(r) => r.node !== binding.node && r.path.at(-1)?.type !== 'ExportSpecifier'
 				);
 				if (!references.length && !instance.scope.declarations.has(`$${name}`)) {
-					warn(warnings, binding.node, [], 'unused-export-let', name);
+					warn(warnings, binding.node, 'unused-export-let', name);
 				}
 			}
 		}
@@ -535,15 +538,15 @@ export function analyze_component(root, source, options) {
 									type === 'AwaitBlock' ||
 									type === 'KeyBlock'
 								) {
-									warn(warnings, binding.node, [], 'non-state-reference', name);
+									warn(warnings, binding.node, 'non-state-reference', name);
 									continue outer;
 								}
 							}
 							continue inner;
 						}
 					}
 
-					warn(warnings, binding.node, [], 'non-state-reference', name);
+					warn(warnings, binding.node, 'non-state-reference', name);
 					continue outer;
 				}
 			}
@@ -557,7 +560,13 @@ export function analyze_component(root, source, options) {
 		for (const element of analysis.elements) {
 			prune(analysis.css.ast, element);
 		}
-		warn_unused(analysis.css.ast, analysis.warnings);
+
+		if (
+			!analysis.css.ast.content.comment ||
+			!extract_svelte_ignore(analysis.css.ast.content.comment.data).includes('css-unused-selector')
+		) {
+			warn_unused(analysis.css.ast, analysis.warnings);
+		}
 
 		outer: for (const element of analysis.elements) {
 			if (element.metadata.scoped) {
@@ -679,7 +688,7 @@ const legacy_scope_tweaker = {
 				(d) => d.scope === state.analysis.module.scope && d.declaration_kind !== 'const'
 			)
 		) {
-			warn(state.analysis.warnings, node, path, 'module-script-reactive-declaration');
+			warn(state.analysis.warnings, node, 'module-script-reactive-declaration');
 		}
 
 		if (
@@ -1045,6 +1054,65 @@ const function_visitor = (node, context) => {
 
 /** @type {import('./types').Visitors} */
 const common_visitors = {
+	_(node, context) {
+		// @ts-expect-error
+		const comments = /** @type {import('estree').Comment[]} */ (node.leadingComments);
+
+		if (comments) {
+			/** @type {string[]} */
+			const ignores = [];
+
+			for (const comment of comments) {
+				ignores.push(...extract_svelte_ignore(comment.value));
+			}
+
+			if (ignores.length > 0) {
+				// @ts-expect-error see below
+				node.ignores = new Set([...context.state.ignores, ...ignores]);
+			}
+		}
+
+		// @ts-expect-error
+		if (node.ignores) {
+			context.next({
+				...context.state,
+				// @ts-expect-error see below
+				ignores: node.ignores
+			});
+		} else if (context.state.ignores.size > 0) {
+			// @ts-expect-error
+			node.ignores = context.state.ignores;
+		}
+	},
+	Fragment(node, context) {
+		/** @type {string[]} */
+		let ignores = [];
+
+		for (const child of node.nodes) {
+			if (child.type === 'Text' && child.data.trim() === '') {
+				continue;
+			}
+
+			if (child.type === 'Comment') {
+				ignores.push(...extract_svelte_ignore(child.data));
+			} else {
+				const combined_ignores = new Set(context.state.ignores);
+				for (const ignore of ignores) combined_ignores.add(ignore);
+
+				if (combined_ignores.size > 0) {
+					// TODO this is a grotesque hack that's made necessary by the fact that
+					// we can't call `context.visit(...)` here, because we do the convoluted
+					// visitor merging thing. I'm increasingly of the view that we should
+					// rearchitect this stuff and have a single visitor per node. It'd be
+					// more efficient and much simpler.
+					// @ts-expect-error
+					child.ignores = combined_ignores;
+				}
+
+				ignores = [];
+			}
+		}
+	},
 	Attribute(node, context) {
 		if (node.value === true) return;
 
@@ -1142,7 +1210,7 @@ const common_visitors = {
 					binding.kind === 'derived') &&
 				context.state.function_depth === binding.scope.function_depth
 			) {
-				warn(context.state.analysis.warnings, node, context.path, 'static-state-reference');
+				warn(context.state.analysis.warnings, node, 'static-state-reference');
 			}
 		}
 	},

packages/svelte/src/compiler/phases/2-analyze/types.d.ts

@@ -22,6 +22,7 @@ export interface AnalysisState {
 	expression: ExpressionTag | ClassDirective | SpreadAttribute | null;
 	private_derived_state: string[];
 	function_depth: number;
+	ignores: Set<string>;
 }
 
 export interface LegacyAnalysisState extends AnalysisState {

packages/svelte/src/compiler/phases/2-analyze/validation.js

@@ -134,7 +134,6 @@ function validate_element(node, context) {
 					warn(
 						context.state.analysis.warnings,
 						attribute,
-						context.path,
 						'global-event-reference',
 						attribute.name
 					);
@@ -147,15 +146,14 @@ function validate_element(node, context) {
 			}
 
 			if (attribute.name === 'is' && context.state.options.namespace !== 'foreign') {
-				warn(context.state.analysis.warnings, attribute, context.path, 'avoid-is');
+				warn(context.state.analysis.warnings, attribute, 'avoid-is');
 			}
 
 			const correct_name = react_attributes.get(attribute.name);
 			if (correct_name) {
 				warn(
 					context.state.analysis.warnings,
 					attribute,
-					context.path,
 					'invalid-html-attribute',
 					attribute.name,
 					correct_name
@@ -235,7 +233,7 @@ function validate_attribute_name(attribute, context) {
 		!attribute.name.startsWith('xlink:') &&
 		!attribute.name.startsWith('xml:')
 	) {
-		warn(context.state.analysis.warnings, attribute, context.path, 'illegal-attribute-character');
+		warn(context.state.analysis.warnings, attribute, 'illegal-attribute-character');
 	}
 }
 
@@ -313,7 +311,7 @@ function validate_block_not_empty(node, context) {
 	// Assumption: If the block has zero elements, someone's in the middle of typing it out,
 	// so don't warn in that case because it would be distracting.
 	if (node.nodes.length === 1 && node.nodes[0].type === 'Text' && !node.nodes[0].raw.trim()) {
-		warn(context.state.analysis.warnings, node.nodes[0], context.path, 'empty-block');
+		warn(context.state.analysis.warnings, node.nodes[0], 'empty-block');
 	}
 }
 
@@ -377,7 +375,6 @@ const validation = {
 			warn(
 				context.state.analysis.warnings,
 				binding.node,
-				context.path,
 				'invalid-rest-eachblock-binding',
 				binding.node.name
 			);
@@ -534,13 +531,7 @@ const validation = {
 			binding.declaration_kind === 'import' &&
 			binding.references.length === 0
 		) {
-			warn(
-				context.state.analysis.warnings,
-				node,
-				context.path,
-				'component-name-lowercase',
-				node.name
-			);
+			warn(context.state.analysis.warnings, node, 'component-name-lowercase', node.name);
 		}
 
 		validate_element(node, context);
@@ -579,13 +570,7 @@ const validation = {
 			!VoidElements.includes(node.name) &&
 			!SVGElements.includes(node.name)
 		) {
-			warn(
-				context.state.analysis.warnings,
-				node,
-				context.path,
-				'invalid-self-closing-tag',
-				node.name
-			);
+			warn(context.state.analysis.warnings, node, 'invalid-self-closing-tag', node.name);
 		}
 
 		context.next({
@@ -781,7 +766,7 @@ export const validation_legacy = merge(validation, a11y_validators, {
 			(state.ast_type !== 'instance' ||
 				/** @type {import('#compiler').SvelteNode} */ (path.at(-1)).type !== 'Program')
 		) {
-			warn(state.analysis.warnings, node, path, 'no-reactive-declaration');
+			warn(state.analysis.warnings, node, 'no-reactive-declaration');
 		}
 	},
 	UpdateExpression(node, { state }) {
@@ -984,12 +969,12 @@ export const validation_runes_js = {
 		const allowed_depth = context.state.ast_type === 'module' ? 0 : 1;
 
 		if (context.state.scope.function_depth > allowed_depth) {
-			warn(context.state.analysis.warnings, node, context.path, 'avoid-nested-class');
+			warn(context.state.analysis.warnings, node, 'avoid-nested-class');
 		}
 	},
 	NewExpression(node, context) {
 		if (node.callee.type === 'ClassExpression' && context.state.scope.function_depth > 0) {
-			warn(context.state.analysis.warnings, node, context.path, 'avoid-inline-class');
+			warn(context.state.analysis.warnings, node, 'avoid-inline-class');
 		}
 	}
 };
@@ -1134,15 +1119,15 @@ export const validation_runes = merge(validation, a11y_validators, {
 		}
 		next({ ...state });
 	},
-	VariableDeclarator(node, { state, path }) {
+	VariableDeclarator(node, { state }) {
 		ensure_no_module_import_conflict(node, state);
 
 		const init = node.init;
 		const rune = get_rune(init, state.scope);
 
 		if (rune === null) {
 			if (init?.type === 'Identifier' && init.name === '$props' && !state.scope.get('props')) {
-				warn(state.analysis.warnings, node, path, 'invalid-props-declaration');
+				warn(state.analysis.warnings, node, 'invalid-props-declaration');
 			}
 			return;
 		}
@@ -1195,29 +1180,29 @@ export const validation_runes = merge(validation, a11y_validators, {
 				arg.type === 'CallExpression' &&
 				(arg.callee.type === 'ArrowFunctionExpression' || arg.callee.type === 'FunctionExpression')
 			) {
-				warn(state.analysis.warnings, node, path, 'derived-iife');
+				warn(state.analysis.warnings, node, 'derived-iife');
 			}
 		}
 	},
-	AssignmentPattern(node, { state, path }) {
+	AssignmentPattern(node, { state }) {
 		if (
 			node.right.type === 'Identifier' &&
 			node.right.name === '$bindable' &&
 			!state.scope.get('bindable')
 		) {
-			warn(state.analysis.warnings, node, path, 'invalid-bindable-declaration');
+			warn(state.analysis.warnings, node, 'invalid-bindable-declaration');
 		}
 	},
-	SlotElement(node, { state, path }) {
+	SlotElement(node, { state }) {
 		if (!state.analysis.custom_element) {
-			warn(state.analysis.warnings, node, path, 'deprecated-slot-element');
+			warn(state.analysis.warnings, node, 'deprecated-slot-element');
 		}
 	},
 	OnDirective(node, { state, path }) {
 		const parent_type = path.at(-1)?.type;
 		// Don't warn on component events; these might not be under the author's control so the warning would be unactionable
 		if (parent_type === 'RegularElement' || parent_type === 'SvelteElement') {
-			warn(state.analysis.warnings, node, path, 'deprecated-event-handler', node.name);
+			warn(state.analysis.warnings, node, 'deprecated-event-handler', node.name);
 		}
 	},
 	// TODO this is a code smell. need to refactor this stuff

packages/svelte/src/compiler/types/legacy-nodes.d.ts

@@ -198,6 +198,14 @@ export interface LegacyWindow extends BaseElement {
 	type: 'Window';
 }
 
+export interface LegacyComment extends BaseNode {
+	type: 'Comment';
+	/** the contents of the comment */
+	data: string;
+	/** any svelte-ignore directives — <!-- svelte-ignore a b c --> would result in ['a', 'b', 'c'] */
+	ignores: string[];
+}
+
 type LegacyDirective =
 	| LegacyAnimation
 	| LegacyBinding
@@ -213,6 +221,7 @@ export type LegacyAttributeLike = LegacyAttribute | LegacySpread | LegacyDirecti
 export type LegacyElementLike =
 	| LegacyBody
 	| LegacyCatchBlock
+	| LegacyComment
 	| LegacyDocument
 	| LegacyElement
 	| LegacyHead

packages/svelte/src/compiler/types/template.d.ts

@@ -131,8 +131,6 @@ export interface Comment extends BaseNode {
 	type: 'Comment';
 	/** the contents of the comment */
 	data: string;
-	/** any svelte-ignore directives — <!-- svelte-ignore a b c --> would result in ['a', 'b', 'c'] */
-	ignores: string[];
 }
 
 /** A `{@const ...}` tag */

packages/svelte/src/compiler/utils/extract_svelte_ignore.js

@@ -15,41 +15,3 @@ export function extract_svelte_ignore(text) {
 				.filter(Boolean)
 		: [];
 }
-
-/**
- * @template {{ leadingComments?: Array<{ value: string }> }} Node
- * @param {Node} node
- * @returns {string[]}
- */
-export function extract_svelte_ignore_from_comments(node) {
-	return (node.leadingComments || []).flatMap(
-		/** @param {any} comment */ (comment) => extract_svelte_ignore(comment.value)
-	);
-}
-
-/**
- * @param {import('#compiler').TemplateNode} node
- * @param {import('#compiler').TemplateNode[]} template_nodes
- * @returns {string[]}
- */
-export function extract_ignores_above_position(node, template_nodes) {
-	const previous_node_idx = template_nodes.indexOf(node) - 1;
-	if (previous_node_idx < 0) {
-		return [];
-	}
-
-	const ignores = [];
-	for (let i = previous_node_idx; i >= 0; i--) {
-		const node = template_nodes[i];
-		if (node.type !== 'Comment' && node.type !== 'Text') {
-			return ignores;
-		}
-		if (node.type === 'Comment') {
-			if (node.ignores.length) {
-				ignores.push(...node.ignores);
-			}
-		}
-	}
-
-	return ignores;
-}

packages/svelte/src/compiler/warnings.js

@@ -1,8 +1,3 @@
-import {
-	extract_ignores_above_position,
-	extract_svelte_ignore_from_comments
-} from './utils/extract_svelte_ignore.js';
-
 /** @typedef {Record<string, (...args: any[]) => string>} Warnings */
 
 /** @satisfies {Warnings} */
@@ -277,52 +272,15 @@ const warnings = {
  * @template {keyof AllWarnings} T
  * @param {import('./phases/types').RawWarning[]} array the array to push the warning to, if not ignored
  * @param {{ start?: number, end?: number, type?: string, parent?: import('#compiler').SvelteNode | null, leadingComments?: import('estree').Comment[] } | null} node the node related to the warning
- * @param {import('#compiler').SvelteNode[]} path the path to the node, so that we can traverse upwards to find svelte-ignore comments
  * @param {T} code the warning code
  * @param  {Parameters<AllWarnings[T]>} args the arguments to pass to the warning function
  * @returns {void}
  */
-export function warn(array, node, path, code, ...args) {
-	const fn = warnings[code];
+export function warn(array, node, code, ...args) {
+	// @ts-expect-error
+	if (node.ignores?.has(code)) return;
 
-	// Traverse the AST upwards to find any svelte-ignore comments.
-	// This assumes that people don't have their code littered with warnings,
-	// at which point this might become inefficient.
-	/** @type {string[]} */
-	const ignores = [];
-
-	if (node) {
-		// comments inside JavaScript (estree)
-		if ('leadingComments' in node) {
-			ignores.push(...extract_svelte_ignore_from_comments(node));
-		}
-	}
-
-	for (let i = path.length - 1; i >= 0; i--) {
-		const current = path[i];
-
-		// comments inside JavaScript (estree)
-		if ('leadingComments' in current) {
-			ignores.push(...extract_svelte_ignore_from_comments(current));
-		}
-
-		// Svelte nodes
-		if (current.type === 'Fragment') {
-			ignores.push(
-				...extract_ignores_above_position(
-					/** @type {import('#compiler').TemplateNode} */ (path[i + 1] ?? node),
-					current.nodes
-				)
-			);
-		}
-
-		// Style nodes
-		if (current.type === 'StyleSheet' && current.content.comment) {
-			ignores.push(...current.content.comment.ignores);
-		}
-	}
-
-	if (ignores.includes(code)) return;
+	const fn = warnings[code];
 
 	const start = node?.start;
 	const end = node?.end;

packages/svelte/tests/parser-modern/samples/comment-before-script/output.json

@@ -11,8 +11,7 @@
 				"type": "Comment",
 				"start": 0,
 				"end": 27,
-				"data": "should not error out",
-				"ignores": []
+				"data": "should not error out"
 			},
 			{
 				"type": "Text",

packages/svelte/types/index.d.ts

@@ -955,6 +955,14 @@ declare module 'svelte/compiler' {
 		type: 'Window';
 	}
 
+	interface LegacyComment extends BaseNode_1 {
+		type: 'Comment';
+		/** the contents of the comment */
+		data: string;
+		/** any svelte-ignore directives — <!-- svelte-ignore a b c --> would result in ['a', 'b', 'c'] */
+		ignores: string[];
+	}
+
 	type LegacyDirective =
 		| LegacyAnimation
 		| LegacyBinding
@@ -970,6 +978,7 @@ declare module 'svelte/compiler' {
 	type LegacyElementLike =
 		| LegacyBody
 		| LegacyCatchBlock
+		| LegacyComment
 		| LegacyDocument
 		| LegacyElement
 		| LegacyHead
@@ -1355,8 +1364,6 @@ declare module 'svelte/compiler' {
 		type: 'Comment';
 		/** the contents of the comment */
 		data: string;
-		/** any svelte-ignore directives — <!-- svelte-ignore a b c --> would result in ['a', 'b', 'c'] */
-		ignores: string[];
 	}
 
 	/** A `{@const ...}` tag */