Refactor font-family-name-quotes to use fix callback instead of `…

…context.fix` and fix false negatives (#7777) Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
stylelint · Jun 21, 2024 · 6d6b98b · 6d6b98b
1 parent f2f9a6a
commit 6d6b98b
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 100 deletions.
diff --git a/.changeset/khaki-eels-love.md b/.changeset/khaki-eels-love.md
@@ -0,0 +1,5 @@
+---
+"stylelint": minor
+---
+
+Fixed: `font-family-name-quotes` false negatives for `-moz-*`/`-webkit-*` keywords
diff --git a/lib/reference/keywords.cjs b/lib/reference/keywords.cjs
@@ -28,6 +28,56 @@ const fontFamilyKeywords = uniteSets(basicKeywords, [
 	'ui-rounded',
 ]);
 
+const appleSystemFonts = new Set([
+	'-apple-system',
+	'-apple-system-headline',
+	'-apple-system-body',
+	'-apple-system-subheadline',
+	'-apple-system-footnote',
+	'-apple-system-caption1',
+	'-apple-system-caption2',
+	'-apple-system-short-headline',
+	'-apple-system-short-body',
+	'-apple-system-short-subheadline',
+	'-apple-system-short-footnote',
+	'-apple-system-short-caption1',
+	'-apple-system-tall-body',
+	'-apple-system-title0',
+	'-apple-system-title1',
+	'-apple-system-title2',
+	'-apple-system-title3',
+	'-apple-system-title4',
+]);
+
+const mozillaSystemFonts = new Set([
+	'-moz-button',
+	'-moz-desktop',
+	'-moz-dialog',
+	'-moz-document',
+	'-moz-field',
+	'-moz-fixed',
+	'-moz-info',
+	'-moz-list',
+	'-moz-pull-down-menu',
+	'-moz-window',
+	'-moz-workspace',
+]);
+
+const webkitSystemFonts = new Set([
+	'-webkit-body',
+	'-webkit-control',
+	'-webkit-mini-control',
+	'-webkit-pictograph',
+	'-webkit-small-control',
+	'-webkit-standard',
+]);
+
+const prefixedSystemFonts = uniteSets(
+	appleSystemFonts,
+	mozillaSystemFonts,
+	webkitSystemFonts,
+);
+
 const fontWeightRelativeKeywords = new Set(['bolder', 'lighter']);
 
 const fontWeightAbsoluteKeywords = new Set(['normal', 'bold']);
@@ -501,5 +551,6 @@ exports.listStylePositionKeywords = listStylePositionKeywords;
 exports.listStyleShorthandKeywords = listStyleShorthandKeywords;
 exports.listStyleTypeKeywords = listStyleTypeKeywords;
 exports.namedColorsKeywords = namedColorsKeywords;
+exports.prefixedSystemFonts = prefixedSystemFonts;
 exports.systemColorsKeywords = systemColorsKeywords;
 exports.systemFontKeywords = systemFontKeywords;
diff --git a/lib/reference/keywords.mjs b/lib/reference/keywords.mjs
@@ -24,6 +24,56 @@ export const fontFamilyKeywords = uniteSets(basicKeywords, [
 	'ui-rounded',
 ]);
 
+const appleSystemFonts = new Set([
+	'-apple-system',
+	'-apple-system-headline',
+	'-apple-system-body',
+	'-apple-system-subheadline',
+	'-apple-system-footnote',
+	'-apple-system-caption1',
+	'-apple-system-caption2',
+	'-apple-system-short-headline',
+	'-apple-system-short-body',
+	'-apple-system-short-subheadline',
+	'-apple-system-short-footnote',
+	'-apple-system-short-caption1',
+	'-apple-system-tall-body',
+	'-apple-system-title0',
+	'-apple-system-title1',
+	'-apple-system-title2',
+	'-apple-system-title3',
+	'-apple-system-title4',
+]);
+
+const mozillaSystemFonts = new Set([
+	'-moz-button',
+	'-moz-desktop',
+	'-moz-dialog',
+	'-moz-document',
+	'-moz-field',
+	'-moz-fixed',
+	'-moz-info',
+	'-moz-list',
+	'-moz-pull-down-menu',
+	'-moz-window',
+	'-moz-workspace',
+]);
+
+const webkitSystemFonts = new Set([
+	'-webkit-body',
+	'-webkit-control',
+	'-webkit-mini-control',
+	'-webkit-pictograph',
+	'-webkit-small-control',
+	'-webkit-standard',
+]);
+
+export const prefixedSystemFonts = uniteSets(
+	appleSystemFonts,
+	mozillaSystemFonts,
+	webkitSystemFonts,
+);
+
 export const fontWeightRelativeKeywords = new Set(['bolder', 'lighter']);
 
 export const fontWeightAbsoluteKeywords = new Set(['normal', 'bold']);

diff --git a/lib/rules/font-family-name-quotes/__tests__/index.mjs b/lib/rules/font-family-name-quotes/__tests__/index.mjs
@@ -586,6 +586,26 @@ testRule({
 				"a { font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', 'Roboto', sans-serif; }",
 			message: messages.rejected('BlinkMacSystemFont'),
 		},
+		{
+			code: "a { font-family: '-webkit-control', '-moz-button'; }",
+			fixed: 'a { font-family: -webkit-control, -moz-button; }',
+			warnings: [
+				{
+					message: messages.rejected('-webkit-control'),
+					line: 1,
+					column: 18,
+					endLine: 1,
+					endColumn: 35,
+				},
+				{
+					message: messages.rejected('-moz-button'),
+					line: 1,
+					column: 37,
+					endLine: 1,
+					endColumn: 50,
+				},
+			],
+		},
 		{
 			code: 'a { font: italic 300 16px/30px Arial, serif; }',
 			fixed: 'a { font: italic 300 16px/30px "Arial", serif; }',

diff --git a/lib/rules/font-family-name-quotes/index.cjs b/lib/rules/font-family-name-quotes/index.cjs
@@ -2,8 +2,8 @@
 // please instead edit the ESM counterpart and rebuild with Rollup (npm run build).
 'use strict';
 
-const findFontFamily = require('../../utils/findFontFamily.cjs');
 const keywords = require('../../reference/keywords.cjs');
+const findFontFamily = require('../../utils/findFontFamily.cjs');
 const isStandardSyntaxValue = require('../../utils/isStandardSyntaxValue.cjs');
 const isVariable = require('../../utils/isVariable.cjs');
 const report = require('../../utils/report.cjs');
@@ -27,7 +27,7 @@ const meta = {
  * @returns {boolean}
  */
 function isSystemFontKeyword(font) {
-	if (font.startsWith('-apple-')) {
+	if (keywords.prefixedSystemFonts.has(font)) {
 		return true;
 	}
 
@@ -109,6 +109,8 @@ const makeMutableFontFamilies = (fontFamilies, decl) => {
 				this.hasQuotes = false;
 				decl.value = decl.value.slice(0, openIndex) + this.name + decl.value.substring(closeIndex);
 				this.resetIndexes(-2);
+
+				return decl.rangeBy({ word: this.name });
 			},
 			addQuotes() {
 				if (this.hasQuotes === true) return;
@@ -121,6 +123,8 @@ const makeMutableFontFamilies = (fontFamilies, decl) => {
 
 				decl.value = decl.value.slice(0, openIndex) + fixedName + decl.value.substring(closeIndex);
 				this.resetIndexes(2);
+
+				return decl.rangeBy({ word: fixedName });
 			},
 		};
 
@@ -131,7 +135,7 @@ const makeMutableFontFamilies = (fontFamilies, decl) => {
 };
 
 /** @type {import('stylelint').Rule} */
-const rule = (primary, _secondary, context) => {
+const rule = (primary) => {
 	return (root, result) => {
 		const validOptions = validateOptions(result, ruleName, {
 			actual: primary,
@@ -173,13 +177,7 @@ const rule = (primary, _secondary, context) => {
 			// and system font keywords in all cases
 			if (keywords.fontFamilyKeywords.has(family.toLowerCase()) || isSystemFontKeyword(family)) {
 				if (hasQuotes) {
-					if (context.fix) {
-						fontFamilyNode.removeQuotes();
-
-						return;
-					}
-
-					return complain(messages.rejected(family), rawFamily, decl);
+					return complain('rejected', fontFamilyNode, decl);
 				}
 
 				return;
@@ -191,75 +189,53 @@ const rule = (primary, _secondary, context) => {
 			switch (primary) {
 				case 'always-unless-keyword':
 					if (!hasQuotes) {
-						if (context.fix) {
-							fontFamilyNode.addQuotes();
-
-							return;
-						}
-
-						return complain(messages.expected(family), rawFamily, decl);
+						return complain('expected', fontFamilyNode, decl);
 					}
 
 					return;
 
 				case 'always-where-recommended':
 					if (!recommended && hasQuotes) {
-						if (context.fix) {
-							fontFamilyNode.removeQuotes();
-
-							return;
-						}
-
-						return complain(messages.rejected(family), rawFamily, decl);
+						return complain('rejected', fontFamilyNode, decl);
 					}
 
 					if (recommended && !hasQuotes) {
-						if (context.fix) {
-							fontFamilyNode.addQuotes();
-
-							return;
-						}
-
-						return complain(messages.expected(family), rawFamily, decl);
+						return complain('expected', fontFamilyNode, decl);
 					}
 
 					return;
 
 				case 'always-where-required':
 					if (!required && hasQuotes) {
-						if (context.fix) {
-							fontFamilyNode.removeQuotes();
-
-							return;
-						}
-
-						return complain(messages.rejected(family), rawFamily, decl);
+						return complain('rejected', fontFamilyNode, decl);
 					}
 
 					if (required && !hasQuotes) {
-						if (context.fix) {
-							fontFamilyNode.addQuotes();
-
-							return;
-						}
-
-						return complain(messages.expected(family), rawFamily, decl);
+						return complain('expected', fontFamilyNode, decl);
 					}
 			}
 		}
 
 		/**
-		 * @param {string} message
-		 * @param {string} family
+		 * @param {keyof messages} messageType
+		 * @param {MutableNode} fontFamilyNode
 		 * @param {import('postcss').Declaration} decl
 		 */
-		function complain(message, family, decl) {
+		function complain(messageType, fontFamilyNode, decl) {
+			const { name, rawName } = fontFamilyNode;
+			const fix = () => {
+				return messageType === 'expected'
+					? fontFamilyNode.addQuotes()
+					: fontFamilyNode.removeQuotes();
+			};
+
 			report({
 				result,
 				ruleName,
-				message,
+				message: messages[messageType](name),
 				node: decl,
-				word: family,
+				word: rawName,
+				fix,
 			});
 		}
 	};