Skip to content

Commit

Permalink
Fix shorthand-property-no-redundant-values false negatives for func…
Browse files Browse the repository at this point in the history 
…tions (#7657)

Co-authored-by: Marc G. <Mouvedia@users.noreply.github.com>
  • Loading branch information
@ybiquitous @Mouvedia
ybiquitous and Mouvedia committed Apr 27, 2024
1 parent c85f75b commit f87f784
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 70 deletions.
5 changes: 5 additions & 0 deletions .changeset/famous-deers-build.md
View file
@@ -0,0 +1,5 @@
---
"stylelint": minor
---

Fixed: `shorthand-property-no-redundant-values` false negatives for functions
29 changes: 21 additions & 8 deletions lib/rules/shorthand-property-no-redundant-values/__tests__/index.mjs
View file
Expand Up @@ -78,23 +78,19 @@ 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 { border-radius: 1px 1px / 1px; }',
description: 'ignore ellipse',
},
{
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',
},
{
Expand Down Expand Up @@ -381,6 +377,23 @@ testRule({
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,
},
],
});

Expand Down
60 changes: 29 additions & 31 deletions lib/rules/shorthand-property-no-redundant-values/index.cjs
View file
Expand Up @@ -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';

Expand All @@ -21,7 +23,7 @@ const meta = {
fixable: true,
};

const propertiesWithShorthandNotation = new Set([
const SUPPORTED_SHORTHANDS = new Set([
'margin',
'padding',
'border-color',
Expand All @@ -32,22 +34,12 @@ 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}
*/
function isShorthandProperty(property) {
return propertiesWithShorthandNotation.has(property);
function isSupportedShorthand(property) {
return SUPPORTED_SHORTHANDS.has(property);
}

/**
Expand Down Expand Up @@ -125,40 +117,46 @@ 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 (!isSupportedShorthand(normalizedProp)) return;

/** @type {string[]} */
const valuesToShorthand = [];

valueParser(value).walk((valueNode) => {
if (valueNode.type !== 'word') {
for (const valueNode of valueParser(value).nodes) {
if (typeGuards.isValueDiv(valueNode)) {
return;
}

// `var()` should be ignored. E.g.
//
// --padding: 20px 5px;
// padding: 0 var(--padding) 0;
//
if (typeGuards.isValueFunction(valueNode) && valueNode.value.toLowerCase() === 'var') {
return;
}

valuesToShorthand.push(valueParser.stringify(valueNode));
});
if (typeGuards.isValueWord(valueNode) || typeGuards.isValueFunction(valueNode)) {
valuesToShorthand.push(valueParser.stringify(valueNode));
}
}

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(' ');

Expand Down
61 changes: 30 additions & 31 deletions lib/rules/shorthand-property-no-redundant-values/index.mjs
View file
Expand Up @@ -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, {
Expand All @@ -18,7 +21,7 @@ const meta = {
fixable: true,
};

const propertiesWithShorthandNotation = new Set([
const SUPPORTED_SHORTHANDS = new Set([
'margin',
'padding',
'border-color',
Expand All @@ -29,22 +32,12 @@ 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}
*/
function isShorthandProperty(property) {
return propertiesWithShorthandNotation.has(property);
function isSupportedShorthand(property) {
return SUPPORTED_SHORTHANDS.has(property);
}

/**
Expand Down Expand Up @@ -122,40 +115,46 @@ const rule = (primary, _secondaryOptions, context) => {
}

root.walkDecls((decl) => {
if (!isStandardSyntaxDeclaration(decl) || !isStandardSyntaxProperty(decl.prop)) {
return;
}
if (!isStandardSyntaxDeclaration(decl)) return;

const prop = decl.prop;
const value = decl.value;
const { prop, value } = decl;

if (!isStandardSyntaxProperty(prop)) return;

if (!isStandardSyntaxValue(value)) return;

const normalizedProp = vendor.unprefixed(prop.toLowerCase());

if (hasIgnoredCharacters(value) || !isShorthandProperty(normalizedProp)) {
return;
}
if (!isSupportedShorthand(normalizedProp)) return;

/** @type {string[]} */
const valuesToShorthand = [];

valueParser(value).walk((valueNode) => {
if (valueNode.type !== 'word') {
for (const valueNode of valueParser(value).nodes) {
if (isValueDiv(valueNode)) {
return;
}

valuesToShorthand.push(valueParser.stringify(valueNode));
});
// `var()` should be ignored. E.g.
//
// --padding: 20px 5px;
// padding: 0 var(--padding) 0;
//
if (isValueFunction(valueNode) && valueNode.value.toLowerCase() === 'var') {
return;
}

if (isValueWord(valueNode) || isValueFunction(valueNode)) {
valuesToShorthand.push(valueParser.stringify(valueNode));
}
}

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(' ');

Expand Down
9 changes: 9 additions & 0 deletions lib/utils/typeGuards.cjs
View file
Expand Up @@ -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}
Expand Down Expand Up @@ -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;
8 changes: 8 additions & 0 deletions lib/utils/typeGuards.mjs
View file
Expand Up @@ -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}
Expand Down

0 comments on commit f87f784

Please sign in to comment.