Fix shorthand-property-no-redundant-values false negatives for func…

…tions

.changeset/famous-deers-build.md

@@ -0,0 +1,5 @@
+---
+"stylelint": minor
+---
+
+Fixed: `shorthand-property-no-redundant-values` false negatives for functions

lib/rules/shorthand-property-no-redundant-values/__tests__/index.mjs

@@ -78,23 +78,11 @@ testRule({
 			description: 'ignore not shorthandable property',
 		},
 		{
-			code: 'a { border-radius: 1px / 1px }',
+			code: 'a { border-radius: 1px / 1px; }',
 			description: 'ignore ellipse',
 		},
 		{
-			code: 'a { margin: calc(1px + 1px) calc(1px + 1px); }',
-			description: 'ignore calc function',
-		},
-		{
-			code: 'a { margin: some-function(1px + 1px) some-function(1px + 1px); }',
-			description: 'ignore all function',
-		},
-		{
-			code: 'a { margin: var(--margin) var(--margin); }',
-			description: 'ignore variables',
-		},
-		{
-			code: 'a { border-color: #FFFFFF transparent transparent }',
+			code: 'a { border-color: #FFFFFF transparent transparent; }',
 			description: 'ignore upper case value',
 		},
 		{
@@ -381,6 +369,30 @@ testRule({
 			line: 1,
 			column: 5,
 		},
+		{
+			code: 'a { margin: var(--margin) var(--margin); }',
+			fixed: 'a { margin: var(--margin); }',
+			message: messages.rejected('var(--margin) var(--margin)', 'var(--margin)'),
+			line: 1,
+			column: 5,
+		},
+		{
+			code: 'a { margin: calc(1px + 1px) calc(1px + 1px); }',
+			fixed: 'a { margin: calc(1px + 1px); }',
+			message: messages.rejected('calc(1px + 1px) calc(1px + 1px)', 'calc(1px + 1px)'),
+			line: 1,
+			column: 5,
+		},
+		{
+			code: 'a { margin: some-function(1px + 1px) some-function(1px + 1px); }',
+			fixed: 'a { margin: some-function(1px + 1px); }',
+			message: messages.rejected(
+				'some-function(1px + 1px) some-function(1px + 1px)',
+				'some-function(1px + 1px)',
+			),
+			line: 1,
+			column: 5,
+		},
 	],
 });
 

lib/rules/shorthand-property-no-redundant-values/index.cjs

@@ -5,10 +5,12 @@
 const valueParser = require('postcss-value-parser');
 const isStandardSyntaxDeclaration = require('../../utils/isStandardSyntaxDeclaration.cjs');
 const isStandardSyntaxProperty = require('../../utils/isStandardSyntaxProperty.cjs');
+const isStandardSyntaxValue = require('../../utils/isStandardSyntaxValue.cjs');
 const report = require('../../utils/report.cjs');
 const ruleMessages = require('../../utils/ruleMessages.cjs');
 const validateOptions = require('../../utils/validateOptions.cjs');
 const vendor = require('../../utils/vendor.cjs');
+const typeGuards = require('../../utils/typeGuards.cjs');
 
 const ruleName = 'shorthand-property-no-redundant-values';
 
@@ -32,16 +34,6 @@ const propertiesWithShorthandNotation = new Set([
 	'inset',
 ]);
 
-const ignoredCharacters = ['+', '*', '/', '(', ')', '$', '@', '--', 'var('];
-
-/**
- * @param {string} value
- * @returns {boolean}
- */
-function hasIgnoredCharacters(value) {
-	return ignoredCharacters.some((char) => value.includes(char));
-}
-
 /**
  * @param {string} property
  * @returns {boolean}
@@ -125,40 +117,45 @@ const rule = (primary, _secondaryOptions, context) => {
 		}
 
 		root.walkDecls((decl) => {
-			if (!isStandardSyntaxDeclaration(decl) || !isStandardSyntaxProperty(decl.prop)) {
-				return;
-			}
+			if (!isStandardSyntaxDeclaration(decl)) return;
+
+			const { prop, value } = decl;
 
-			const prop = decl.prop;
-			const value = decl.value;
+			if (!isStandardSyntaxProperty(prop)) return;
+
+			if (!isStandardSyntaxValue(value)) return;
 
 			const normalizedProp = vendor.unprefixed(prop.toLowerCase());
 
-			if (hasIgnoredCharacters(value) || !isShorthandProperty(normalizedProp)) {
-				return;
-			}
+			if (!isShorthandProperty(normalizedProp)) return;
 
 			/** @type {string[]} */
 			const valuesToShorthand = [];
 
-			valueParser(value).walk((valueNode) => {
-				if (valueNode.type !== 'word') {
-					return;
+			let shouldIgnore = false;
+
+			for (const valueNode of valueParser(value).nodes) {
+				if (typeGuards.isValueDiv(valueNode)) {
+					shouldIgnore = true;
+					break;
+				}
+
+				if (!(typeGuards.isValueWord(valueNode) || typeGuards.isValueFunction(valueNode))) {
+					continue;
 				}
 
 				valuesToShorthand.push(valueParser.stringify(valueNode));
-			});
+			}
+
+			if (shouldIgnore) return;
 
 			if (valuesToShorthand.length <= 1 || valuesToShorthand.length > 4) {
 				return;
 			}
 
-			const shortestForm = canCondense(
-				valuesToShorthand[0] || '',
-				valuesToShorthand[1] || '',
-				valuesToShorthand[2] || '',
-				valuesToShorthand[3] || '',
-			);
+			const [top, right, bottom, left] = valuesToShorthand;
+
+			const shortestForm = canCondense(top ?? '', right ?? '', bottom ?? '', left ?? '');
 			const shortestFormString = shortestForm.filter(Boolean).join(' ');
 			const valuesFormString = valuesToShorthand.join(' ');
 

lib/rules/shorthand-property-no-redundant-values/index.mjs

@@ -2,11 +2,14 @@ import valueParser from 'postcss-value-parser';
 
 import isStandardSyntaxDeclaration from '../../utils/isStandardSyntaxDeclaration.mjs';
 import isStandardSyntaxProperty from '../../utils/isStandardSyntaxProperty.mjs';
+import isStandardSyntaxValue from '../../utils/isStandardSyntaxValue.mjs';
 import report from '../../utils/report.mjs';
 import ruleMessages from '../../utils/ruleMessages.mjs';
 import validateOptions from '../../utils/validateOptions.mjs';
 import vendor from '../../utils/vendor.mjs';
 
+import { isValueDiv, isValueFunction, isValueWord } from '../../utils/typeGuards.mjs';
+
 const ruleName = 'shorthand-property-no-redundant-values';
 
 const messages = ruleMessages(ruleName, {
@@ -29,16 +32,6 @@ const propertiesWithShorthandNotation = new Set([
 	'inset',
 ]);
 
-const ignoredCharacters = ['+', '*', '/', '(', ')', '$', '@', '--', 'var('];
-
-/**
- * @param {string} value
- * @returns {boolean}
- */
-function hasIgnoredCharacters(value) {
-	return ignoredCharacters.some((char) => value.includes(char));
-}
-
 /**
  * @param {string} property
  * @returns {boolean}
@@ -122,40 +115,45 @@ const rule = (primary, _secondaryOptions, context) => {
 		}
 
 		root.walkDecls((decl) => {
-			if (!isStandardSyntaxDeclaration(decl) || !isStandardSyntaxProperty(decl.prop)) {
-				return;
-			}
+			if (!isStandardSyntaxDeclaration(decl)) return;
+
+			const { prop, value } = decl;
 
-			const prop = decl.prop;
-			const value = decl.value;
+			if (!isStandardSyntaxProperty(prop)) return;
+
+			if (!isStandardSyntaxValue(value)) return;
 
 			const normalizedProp = vendor.unprefixed(prop.toLowerCase());
 
-			if (hasIgnoredCharacters(value) || !isShorthandProperty(normalizedProp)) {
-				return;
-			}
+			if (!isShorthandProperty(normalizedProp)) return;
 
 			/** @type {string[]} */
 			const valuesToShorthand = [];
 
-			valueParser(value).walk((valueNode) => {
-				if (valueNode.type !== 'word') {
-					return;
+			let shouldIgnore = false;
+
+			for (const valueNode of valueParser(value).nodes) {
+				if (isValueDiv(valueNode)) {
+					shouldIgnore = true;
+					break;
+				}
+
+				if (!(isValueWord(valueNode) || isValueFunction(valueNode))) {
+					continue;
 				}
 
 				valuesToShorthand.push(valueParser.stringify(valueNode));
-			});
+			}
+
+			if (shouldIgnore) return;
 
 			if (valuesToShorthand.length <= 1 || valuesToShorthand.length > 4) {
 				return;
 			}
 
-			const shortestForm = canCondense(
-				valuesToShorthand[0] || '',
-				valuesToShorthand[1] || '',
-				valuesToShorthand[2] || '',
-				valuesToShorthand[3] || '',
-			);
+			const [top, right, bottom, left] = valuesToShorthand;
+
+			const shortestForm = canCondense(top ?? '', right ?? '', bottom ?? '', left ?? '');
 			const shortestFormString = shortestForm.filter(Boolean).join(' ');
 			const valuesFormString = valuesToShorthand.join(' ');
 

lib/utils/typeGuards.cjs

@@ -53,6 +53,14 @@ function isDocument(node) {
 	return node.type === 'document';
 }
 
+/**
+ * @param {import('postcss-value-parser').Node} node
+ * @returns {node is import('postcss-value-parser').DivNode}
+ */
+function isValueDiv(node) {
+	return node.type === 'div';
+}
+
 /**
  * @param {import('postcss-value-parser').Node} node
  * @returns {node is import('postcss-value-parser').FunctionNode}
@@ -92,6 +100,7 @@ exports.isDeclaration = isDeclaration;
 exports.isDocument = isDocument;
 exports.isRoot = isRoot;
 exports.isRule = isRule;
+exports.isValueDiv = isValueDiv;
 exports.isValueFunction = isValueFunction;
 exports.isValueSpace = isValueSpace;
 exports.isValueWord = isValueWord;

lib/utils/typeGuards.mjs

@@ -49,6 +49,14 @@ export function isDocument(node) {
 	return node.type === 'document';
 }
 
+/**
+ * @param {import('postcss-value-parser').Node} node
+ * @returns {node is import('postcss-value-parser').DivNode}
+ */
+export function isValueDiv(node) {
+	return node.type === 'div';
+}
+
 /**
  * @param {import('postcss-value-parser').Node} node
  * @returns {node is import('postcss-value-parser').FunctionNode}