chore: css unused selector warnings (#11098)

The character adjustments in the existing warnings are because we remove some tabs from empty lines when initializing the Svelte 5 repo; the warnings were just not checked at that time yet.
sveltejs · Apr 9, 2024 · d2b6159 · d2b6159
1 parent ed9bab9
commit d2b6159
Show file tree

Hide file tree

Showing 31 changed files with 374 additions and 107 deletions.
diff --git a/packages/svelte/src/compiler/phases/1-parse/read/style.js b/packages/svelte/src/compiler/phases/1-parse/read/style.js
@@ -36,7 +36,8 @@ export default function read_style(parser, start, attributes) {
 		content: {
 			start: content_start,
 			end: content_end,
-			styles: parser.template.slice(content_start, content_end)
+			styles: parser.template.slice(content_start, content_end),
+			comment: null
 		}
 	};
 }

diff --git a/packages/svelte/src/compiler/phases/1-parse/state/element.js b/packages/svelte/src/compiler/phases/1-parse/state/element.js
@@ -283,25 +283,26 @@ export default function tag(parser) {
 
 	if (is_top_level_script_or_style) {
 		parser.eat('>', true);
-		if (name === 'script') {
-			const content = read_script(parser, start, element.attributes);
 
-			/** @type {import('#compiler').Comment | null} */
-			let prev_comment = null;
-			for (let i = current.fragment.nodes.length - 1; i >= 0; i--) {
-				const node = current.fragment.nodes[i];
+		/** @type {import('#compiler').Comment | null} */
+		let prev_comment = null;
+		for (let i = current.fragment.nodes.length - 1; i >= 0; i--) {
+			const node = current.fragment.nodes[i];
 
-				if (i === current.fragment.nodes.length - 1 && node.end !== start) {
-					break;
-				}
+			if (i === current.fragment.nodes.length - 1 && node.end !== start) {
+				break;
+			}
 
-				if (node.type === 'Comment') {
-					prev_comment = node;
-					break;
-				} else if (node.type !== 'Text' || node.data.trim()) {
-					break;
-				}
+			if (node.type === 'Comment') {
+				prev_comment = node;
+				break;
+			} else if (node.type !== 'Text' || node.data.trim()) {
+				break;
 			}
+		}
+
+		if (name === 'script') {
+			const content = read_script(parser, start, element.attributes);
 			if (prev_comment) {
 				// We take advantage of the fact that the root will never have leadingComments set,
 				// and set the previous comment to it so that the warning mechanism can later
@@ -318,6 +319,7 @@ export default function tag(parser) {
 			}
 		} else {
 			const content = read_style(parser, start, element.attributes);
+			content.content.comment = prev_comment;
 
 			if (current.css) error(start, 'duplicate-style-element');
 			current.css = content;

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
@@ -0,0 +1,34 @@
+import { walk } from 'zimmerframe';
+import { warn } from '../../../warnings.js';
+import { is_keyframes_node } from '../../css.js';
+
+/**
+ * @param {import('#compiler').Css.StyleSheet} stylesheet
+ * @param {import('../../types.js').RawWarning[]} warnings
+ */
+export function warn_unused(stylesheet, warnings) {
+	walk(stylesheet, { warnings, stylesheet }, visitors);
+}
+
+/** @type {import('zimmerframe').Visitors<import('#compiler').Css.Node, { warnings: import('../../types.js').RawWarning[], stylesheet: import('#compiler').Css.StyleSheet }>} */
+const visitors = {
+	Atrule(node, context) {
+		if (!is_keyframes_node(node)) {
+			context.next();
+		}
+	},
+	PseudoClassSelector(node, context) {
+		if (node.name === 'is' || node.name === 'where') {
+			context.next();
+		}
+	},
+	ComplexSelector(node, context) {
+		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);
+		}
+
+		context.next();
+	}
+};
diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js
@@ -23,6 +23,7 @@ import { should_proxy_or_freeze } from '../3-transform/client/utils.js';
 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';
 
 /**
  * @param {import('#compiler').Script | null} script
@@ -548,6 +549,7 @@ 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);
 
 		outer: for (const element of analysis.elements) {
 			if (element.metadata.scoped) {

diff --git a/packages/svelte/src/compiler/types/css.d.ts b/packages/svelte/src/compiler/types/css.d.ts
@@ -1,3 +1,5 @@
+import type { Comment } from '#compiler';
+
 export namespace Css {
 	export interface BaseNode {
 		start: number;
@@ -12,6 +14,8 @@ export namespace Css {
 			start: number;
 			end: number;
 			styles: string;
+			/** Possible comment atop the style tag */
+			comment: Comment | null;
 		};
 	}
 

diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js
@@ -7,7 +7,8 @@ import {
 
 /** @satisfies {Warnings} */
 const css = {
-	'unused-selector': () => 'Unused CSS selector'
+	/** @param {string} name */
+	'css-unused-selector': (name) => `Unused CSS selector "${name}"`
 };
 
 /** @satisfies {Warnings} */
@@ -300,6 +301,11 @@ export function warn(array, node, path, code, ...args) {
 				)
 			);
 		}
+
+		// Style nodes
+		if (current.type === 'StyleSheet' && current.content.comment) {
+			ignores.push(...current.content.comment.ignores);
+		}
 	}
 
 	if (ignores.includes(code)) return;

diff --git a/packages/svelte/tests/css/samples/attribute-selector-case-sensitive/_config.js b/packages/svelte/tests/css/samples/attribute-selector-case-sensitive/_config.js
@@ -0,0 +1,20 @@
+import { test } from '../../test';
+
+export default test({
+	warnings: [
+		{
+			code: 'css-unused-selector',
+			end: {
+				character: 44,
+				column: 14,
+				line: 4
+			},
+			message: 'Unused CSS selector "p[type=\'B\' s]"',
+			start: {
+				character: 31,
+				column: 1,
+				line: 4
+			}
+		}
+	]
+});
diff --git a/packages/svelte/tests/css/samples/descendant-selector-unmatched/_config.js b/packages/svelte/tests/css/samples/descendant-selector-unmatched/_config.js
@@ -0,0 +1,20 @@
+import { test } from '../../test';
+
+export default test({
+	warnings: [
+		{
+			code: 'css-unused-selector',
+			end: {
+				character: 33,
+				column: 6,
+				line: 6
+			},
+			message: 'Unused CSS selector "x y z"',
+			start: {
+				character: 28,
+				column: 1,
+				line: 6
+			}
+		}
+	]
+});
diff --git a/packages/svelte/tests/css/samples/general-siblings-combinator-each-else/_config.js b/packages/svelte/tests/css/samples/general-siblings-combinator-each-else/_config.js
@@ -5,8 +5,8 @@ export default test({
 		{
 			code: 'css-unused-selector',
 			message: 'Unused CSS selector ".b ~ .c"',
-			start: { character: 199, column: 1, line: 13 },
-			end: { character: 206, column: 8, line: 13 }
+			start: { character: 198, column: 1, line: 13 },
+			end: { character: 205, column: 8, line: 13 }
 		}
 	]
 });
diff --git a/packages/svelte/tests/css/samples/general-siblings-combinator-rendertag-global/_config.js b/packages/svelte/tests/css/samples/general-siblings-combinator-rendertag-global/_config.js
@@ -2,12 +2,19 @@ import { test } from '../../test';
 
 export default test({
 	warnings: [
-		// TODO
-		// {
-		// 	code: 'css-unused-selector',
-		// 	message: 'Unused CSS selector ".a ~ .b"',
-		// 	start: { character: 111, column: 1, line: 10 },
-		// 	end: { character: 118, column: 8, line: 10 }
-		// },
+		{
+			code: 'css-unused-selector',
+			end: {
+				character: 479,
+				column: 19,
+				line: 22
+			},
+			message: 'Unused CSS selector ":global(.x) + .bar"',
+			start: {
+				character: 461,
+				column: 1,
+				line: 22
+			}
+		}
 	]
 });
diff --git a/packages/svelte/tests/css/samples/general-siblings-combinator-slot-global/_config.js b/packages/svelte/tests/css/samples/general-siblings-combinator-slot-global/_config.js
@@ -2,12 +2,19 @@ import { test } from '../../test';
 
 export default test({
 	warnings: [
-		// TODO
-		// {
-		// 	code: 'css-unused-selector',
-		// 	message: 'Unused CSS selector ".a ~ .b"',
-		// 	start: { character: 111, column: 1, line: 10 },
-		// 	end: { character: 118, column: 8, line: 10 }
-		// },
+		{
+			code: 'css-unused-selector',
+			end: {
+				character: 472,
+				column: 19,
+				line: 22
+			},
+			message: 'Unused CSS selector ":global(.x) + .bar"',
+			start: {
+				character: 454,
+				column: 1,
+				line: 22
+			}
+		}
 	]
 });
diff --git a/packages/svelte/tests/css/samples/general-siblings-combinator-slot/_config.js b/packages/svelte/tests/css/samples/general-siblings-combinator-slot/_config.js
@@ -5,38 +5,38 @@ export default test({
 		{
 			code: 'css-unused-selector',
 			message: 'Unused CSS selector ".a ~ .b"',
-			start: { character: 111, column: 1, line: 10 },
-			end: { character: 118, column: 8, line: 10 }
+			start: { character: 110, column: 1, line: 10 },
+			end: { character: 117, column: 8, line: 10 }
 		},
 		{
 			code: 'css-unused-selector',
 			message: 'Unused CSS selector ".b ~ .c"',
-			start: { character: 138, column: 1, line: 11 },
-			end: { character: 145, column: 8, line: 11 }
+			start: { character: 137, column: 1, line: 11 },
+			end: { character: 144, column: 8, line: 11 }
 		},
 		{
 			code: 'css-unused-selector',
 			message: 'Unused CSS selector ".c ~ .f"',
-			start: { character: 165, column: 1, line: 12 },
-			end: { character: 172, column: 8, line: 12 }
+			start: { character: 164, column: 1, line: 12 },
+			end: { character: 171, column: 8, line: 12 }
 		},
 		{
 			code: 'css-unused-selector',
 			message: 'Unused CSS selector ".f ~ .g"',
-			start: { character: 192, column: 1, line: 13 },
-			end: { character: 199, column: 8, line: 13 }
+			start: { character: 191, column: 1, line: 13 },
+			end: { character: 198, column: 8, line: 13 }
 		},
 		{
 			code: 'css-unused-selector',
 			message: 'Unused CSS selector ".b ~ .f"',
-			start: { character: 219, column: 1, line: 14 },
-			end: { character: 226, column: 8, line: 14 }
+			start: { character: 218, column: 1, line: 14 },
+			end: { character: 225, column: 8, line: 14 }
 		},
 		{
 			code: 'css-unused-selector',
 			message: 'Unused CSS selector ".b ~ .g"',
-			start: { character: 246, column: 1, line: 15 },
-			end: { character: 253, column: 8, line: 15 }
+			start: { character: 245, column: 1, line: 15 },
+			end: { character: 252, column: 8, line: 15 }
 		}
 	]
 });
diff --git a/packages/svelte/tests/css/samples/general-siblings-combinator-svelteelement/_config.js b/packages/svelte/tests/css/samples/general-siblings-combinator-svelteelement/_config.js
@@ -2,12 +2,19 @@ import { test } from '../../test';
 
 export default test({
 	warnings: [
-		// TODO
-		// {
-		// 	code: 'css-unused-selector',
-		// 	message: 'Unused CSS selector ".a ~ .b"',
-		// 	start: { character: 111, column: 1, line: 10 },
-		// 	end: { character: 118, column: 8, line: 10 }
-		// },
+		{
+			code: 'css-unused-selector',
+			end: {
+				character: 496,
+				column: 10,
+				line: 26
+			},
+			message: 'Unused CSS selector ".x + .bar"',
+			start: {
+				character: 487,
+				column: 1,
+				line: 26
+			}
+		}
 	]
 });
diff --git a/packages/svelte/tests/css/samples/host/_config.js b/packages/svelte/tests/css/samples/host/_config.js
@@ -6,12 +6,12 @@ export default test({
 			code: 'css-unused-selector',
 			message: 'Unused CSS selector ":host > span"',
 			start: {
-				character: 147,
+				character: 145,
 				column: 1,
 				line: 18
 			},
 			end: {
-				character: 159,
+				character: 157,
 				column: 13,
 				line: 18
 			}

diff --git a/packages/svelte/tests/css/samples/is/_config.js b/packages/svelte/tests/css/samples/is/_config.js
@@ -0,0 +1,20 @@
+import { test } from '../../test';
+
+export default test({
+	warnings: [
+		{
+			code: 'css-unused-selector',
+			end: {
+				character: 38,
+				column: 11,
+				line: 6
+			},
+			message: 'Unused CSS selector "z"',
+			start: {
+				character: 37,
+				column: 10,
+				line: 6
+			}
+		}
+	]
+});