Fix selector-attribute-quotes false positives for "never" (#6571)

Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>

.changeset/chilly-ties-trade.md

@@ -0,0 +1,5 @@
+---
+"stylelint": patch
+---
+
+Fixed: `selector-attribute-quotes` false positives for "never"

lib/rules/selector-attribute-quotes/__tests__/index.js

@@ -182,6 +182,14 @@ testRule({
 			code: "[href=\\'test\\'] { }",
 			description: 'escaped single-quotes are not considered as framing quotes',
 		},
+		{
+			code: `[href="te'st"] { }`,
+			description: 'removing quotes would produce invalid CSS', // see: https://github.com/stylelint/stylelint/issues/4300
+		},
+		{
+			code: `[href='te"st'] { }`,
+			description: 'removing quotes would produce invalid CSS', // see: https://github.com/stylelint/stylelint/issues/4300
+		},
 	],
 
 	reject: [
@@ -266,24 +274,6 @@ testRule({
 			endLine: 1,
 			endColumn: 20,
 		},
-		{
-			code: `[href="te'st"] { }`,
-			fixed: "[href=te\\'st] { }",
-			message: messages.rejected("te'st"),
-			line: 1,
-			column: 7,
-			endLine: 1,
-			endColumn: 14,
-		},
-		{
-			code: `[href='te"st'] { }`,
-			fixed: '[href=te\\"st] { }',
-			message: messages.rejected('te"st'),
-			line: 1,
-			column: 7,
-			endLine: 1,
-			endColumn: 14,
-		},
 		{
 			code: "[href='te\\'s\\'t'] { }",
 			fixed: "[href=te\\'s\\'t] { }",

lib/rules/selector-attribute-quotes/index.js

@@ -2,6 +2,7 @@
 
 const getRuleSelector = require('../../utils/getRuleSelector');
 const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule');
+const isValidIdentifier = require('../../utils/isValidIdentifier');
 const parseSelector = require('../../utils/parseSelector');
 const report = require('../../utils/report');
 const ruleMessages = require('../../utils/ruleMessages');
@@ -64,6 +65,17 @@ const rule = (primary, _secondaryOptions, context) => {
 					}
 
 					if (quoted && primary === 'never') {
+						// some selectors require quotes to be valid;
+						// we pass in the raw string value, which contains the escape characters
+						// necessary to check if escaped characters are valid
+						// see: https://github.com/stylelint/stylelint/issues/4300
+						if (
+							!attributeNode.raws.value ||
+							!isValidIdentifier(attributeNode.raws.value.slice(1, -1))
+						) {
+							return;
+						}
+
 						if (context.fix) {
 							selectorFixed = true;
 							attributeNode.quoteMark = null;

lib/utils/__tests__/isValidIdentifier.test.js

@@ -0,0 +1,24 @@
+'use strict';
+
+const isValidIdentifier = require('../isValidIdentifier');
+
+it('isValidIdentifier', () => {
+	expect(isValidIdentifier('foo')).toBeTruthy();
+	expect(isValidIdentifier('foo1')).toBeTruthy();
+	expect(isValidIdentifier('1')).toBeFalsy();
+	expect(isValidIdentifier('-1')).toBeFalsy();
+	expect(isValidIdentifier('--foo')).toBeTruthy();
+	expect(isValidIdentifier('has a space')).toBeFalsy();
+	expect(isValidIdentifier(null)).toBeFalsy();
+	expect(isValidIdentifier('')).toBeFalsy();
+	expect(isValidIdentifier(' ')).toBeFalsy();
+	expect(isValidIdentifier('foö')).toBeFalsy();
+	expect(isValidIdentifier('\\26 B')).toBeTruthy(); // ISO 10646 character
+	expect(isValidIdentifier('\\000026B')).toBeTruthy(); // ISO 10646 character
+	expect(isValidIdentifier("te'st")).toBeFalsy(); // unescaped quotation mark
+	expect(isValidIdentifier('te"st')).toBeFalsy(); // unescaped quotation mark
+	expect(isValidIdentifier("te\\'st")).toBeTruthy(); // escaped (in CSS) quotation mark
+	expect(isValidIdentifier('te\\"st')).toBeTruthy(); // escaped (in CSS) quotation mark
+	expect(isValidIdentifier("te\\'s\\'t")).toBeTruthy(); // escaped (in CSS) quotation mark
+	expect(isValidIdentifier('te\\"s\\"t')).toBeTruthy(); // escaped (in CSS) quotation mark
+});

lib/utils/isValidIdentifier.js

@@ -0,0 +1,36 @@
+'use strict';
+
+/**
+ * Returns whether a string is a valid CSS identifier
+ * (i.e. only alphanumeric characters, `-`, and `_`;
+ * does not have a leading digit, leading dash followed by digit, or two leading dashes)
+ * furthermore, any escaped or ISO 10646 characters are allowed.
+ * @see https://www.w3.org/TR/CSS2/syndata.html#value-def-identifier
+ * @param {string} ident
+ * @returns {boolean}
+ */
+module.exports = function isValidIdentifier(ident) {
+	if (!ident || ident.trim() === '') {
+		return false;
+	}
+
+	// trims, removes ISO 10646 characters, and singly-escaped characters
+	const trimmedIdent = ident
+		.trim()
+		.replace(/\\[0-9a-f]{1,6}(\\r\\n|[ \t\r\n\f])?/gi, '')
+		.replace(/\\./g, '');
+
+	if (/[^\w-]/.test(trimmedIdent)) {
+		return false;
+	}
+
+	if (/\d/.test(trimmedIdent.charAt(0))) {
+		return false;
+	}
+
+	if (trimmedIdent.charAt(0) === '-' && /\d/.test(trimmedIdent.charAt(1))) {
+		return false;
+	}
+
+	return true;
+};