Skip to content

Commit

Permalink
Refactor to migrate unit-disallowed-list to new media query parser (#…
Browse files Browse the repository at this point in the history 
…6886)

* Refactor to migrate `unit-disallowed-list` to new media query parser

* Apply suggestions from code review

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

* apply suggestions from code review

* increase test coverage

* fix

* Update lib/rules/unit-disallowed-list/index.js

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

* cleanup

* ignore urange productions in unicode-range description

---------

Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
  • Loading branch information
@romainmenke @ybiquitous
romainmenke and ybiquitous committed Jun 5, 2023
1 parent 54b7376 commit f1ace61
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 95 deletions.
31 changes: 29 additions & 2 deletions lib/rules/unit-disallowed-list/__tests__/index.js
View file
Expand Up @@ -5,7 +5,7 @@ const { messages, ruleName } = require('..');
testRule({
ruleName,

config: ['px', 'vmin'],
config: ['px', 'vmin', 'f'],

accept: [
{
Expand Down Expand Up @@ -118,6 +118,9 @@ testRule({
code: '@media (min-width: 10em)\n and (max-width: 20em) {}',
description: 'complex @media',
},
{
code: '@font-face { unicode-range: U+0100-024F; }',
},
],

reject: [
Expand Down Expand Up @@ -231,7 +234,7 @@ testRule({
line: 1,
column: 38,
endLine: 1,
endColumn: 41,
endColumn: 40,
},
{
code: '@media (min-width: 13px) {}',
Expand Down Expand Up @@ -260,6 +263,24 @@ testRule({
endLine: 1,
endColumn: 24,
},
{
code: '@font-face { color: U+0100-024F; }',
message: messages.rejected('F'),
description: 'Unicode range value in something other than `unicode-range`',
line: 1,
column: 31,
endLine: 1,
endColumn: 32,
},
{
code: 'a { unicode-range: U+0100-024F; }',
message: messages.rejected('F'),
description: 'Unicode range value in something other than `@font-face`',
line: 1,
column: 30,
endLine: 1,
endColumn: 31,
},
],
});

Expand Down Expand Up @@ -402,6 +423,12 @@ testRule({
{
code: 'a { margin: calc(50% - 100px) }',
},
{
code: 'a { margin: min(calc(50px * 2)) }',
},
{
code: 'a { margin: calc(max(50px)) }',
},
{
code: 'a { transform: translate(100px, 100px) }',
},
Expand Down
237 changes: 146 additions & 91 deletions lib/rules/unit-disallowed-list/index.js
View file
@@ -1,16 +1,26 @@
'use strict';

const { TokenType, tokenize, NumberType } = require('@csstools/css-tokenizer');
const {
isTokenNode,
isFunctionNode,
parseCommaSeparatedListOfComponentValues,
isSimpleBlockNode,
} = require('@csstools/css-parser-algorithms');
const { parseFromTokens, isMediaFeature } = require('@csstools/media-query-list-parser');

const atRuleParamIndex = require('../../utils/atRuleParamIndex');
const declarationValueIndex = require('../../utils/declarationValueIndex');
const getDimension = require('../../utils/getDimension');
const mediaParser = require('postcss-media-query-parser').default;
const getAtRuleParams = require('../../utils/getAtRuleParams');
const getDeclarationValue = require('../../utils/getDeclarationValue');
const hasDimension = require('../../utils/hasDimension');
const optionsMatches = require('../../utils/optionsMatches');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateObjectWithArrayProps = require('../../utils/validateObjectWithArrayProps');
const validateOptions = require('../../utils/validateOptions');
const valueParser = require('postcss-value-parser');
const { isRegExp, isString } = require('../../utils/validateTypes');
const isUnicodeRangeDescriptor = require('../../utils/isUnicodeRangeDescriptor');

const ruleName = 'unit-disallowed-list';

Expand All @@ -22,21 +32,6 @@ const meta = {
url: 'https://stylelint.io/user-guide/rules/unit-disallowed-list',
};

/**
* a function to retrieve only the media feature name
* could be externalized in an utils function if needed in other code
*
* @param {import('postcss-media-query-parser').Child} mediaFeatureNode
* @returns {string | undefined}
*/
const getMediaFeatureName = (mediaFeatureNode) => {
const value = mediaFeatureNode.value.toLowerCase();

const match = /((?:-?\w*)*)/.exec(value);

return match ? match[1] : undefined;
};

/** @type {import('stylelint').Rule<string | string[]>} */
const rule = (primary, secondaryOptions) => {
return (root, result) => {
Expand Down Expand Up @@ -65,29 +60,52 @@ const rule = (primary, secondaryOptions) => {
const primaryValues = [primary].flat();

/**
* @param {import('postcss').Node} node
* @param {number} nodeIndex
* @param {import('postcss-value-parser').Node} valueNode
* @param {string | undefined} input
* @param {Record<string, unknown>} options
* Ignore wrong units within `url` function
* Ignore units within function that match `ignoreFunctions` option
*
* @param {import('@csstools/css-parser-algorithms').ComponentValue} componentValue
* @returns {boolean}
*/
function componentValueIsIgnored(componentValue) {
if (!isFunctionNode(componentValue)) {
return false;
}

const name = componentValue.getName().toLowerCase();

return name === 'url' || optionsMatches(secondaryOptions, 'ignoreFunctions', name);
}

/**
* @template {import('postcss').AtRule | import('postcss').Declaration} T
* @param {T} node
* @param {(node: T) => number} getIndex
* @param {import('@csstools/css-parser-algorithms').ComponentValue} componentValue
* @param {string} input
* @param {Record<string, unknown> | undefined} options
* @returns {void}
*/
function check(node, nodeIndex, valueNode, input, options) {
const { number, unit } = getDimension(valueNode);
function check(node, getIndex, componentValue, input, options) {
if (!isTokenNode(componentValue)) return;

if (componentValue.value[0] !== TokenType.Dimension) return;

const [, , , endIndex, { unit }] = componentValue.value;

// There is not unit or it is not configured as a problem
if (!number || !unit || !primaryValues.includes(unit.toLowerCase())) {
const lowerCaseUnit = unit.toLowerCase();

if (!primaryValues.includes(lowerCaseUnit)) {
return;
}

// The unit has an ignore option for the specific input
if (optionsMatches(options, unit.toLowerCase(), input)) {
return;
}
if (options && optionsMatches(options, lowerCaseUnit, input)) return;

const startIndex = getIndex(node) + (endIndex + 1) - unit.length;

report({
index: nodeIndex + valueNode.sourceIndex + number.length,
endIndex: nodeIndex + valueNode.sourceEndIndex,
index: startIndex,
endIndex: startIndex + unit.length,
message: messages.rejected,
messageArgs: [unit],
node,
Expand All @@ -96,75 +114,112 @@ const rule = (primary, secondaryOptions) => {
});
}

/**
* @template {import('postcss').AtRule} T
* @param {T} node
* @param {string} value
* @param {(node: T) => number} getIndex
* @returns {void}
*/
function checkMedia(node, value, getIndex) {
mediaParser(node.params).walk(/^media-feature$/i, (mediaFeatureNode) => {
const mediaName = getMediaFeatureName(mediaFeatureNode);
const parentValue = mediaFeatureNode.parent.value;

valueParser(value).walk((valueNode) => {
// Ignore all non-word valueNode and
// the values not included in the parentValue string
if (valueNode.type !== 'word' || !parentValue.includes(valueNode.value)) {
return;
root.walkAtRules(/^media$/i, (atRule) => {
const params = getAtRuleParams(atRule);

if (!hasDimension(params)) return;

parseFromTokens(tokenizeWithoutPercentageTokens(params)).forEach((mediaQuery) => {
/** @type {{ mediaFeatureName: string | undefined }} */
const state = {
mediaFeatureName: undefined,
};

mediaQuery.walk((entry) => {
if (!entry.state) return;

if (isMediaFeature(entry.node)) {
entry.state.mediaFeatureName = entry.node.getName().toLowerCase();
}

if (!entry.state.mediaFeatureName) return;

if (!isTokenNode(entry.node)) return;

check(
node,
getIndex(node),
valueNode,
mediaName,
secondaryOptions ? secondaryOptions.ignoreMediaFeatureNames : {},
atRule,
atRuleParamIndex,
entry.node,
entry.state.mediaFeatureName,
secondaryOptions?.ignoreMediaFeatureNames,
);
});
}, state);
});
}
});
root.walkDecls((decl) => {
if (isUnicodeRangeDescriptor(decl)) return;

const value = getDeclarationValue(decl);

if (!hasDimension(value)) return;

parseCommaSeparatedListOfComponentValues(tokenize({ css: value }))
.flatMap((x) => x)
.forEach((componentValue) => {
if (isTokenNode(componentValue)) {
check(
decl,
declarationValueIndex,
componentValue,
decl.prop,
secondaryOptions?.ignoreProperties,
);

/**
* @template {import('postcss').Declaration} T
* @param {T} node
* @param {string} value
* @param {(node: T) => number} getIndex
* @returns {void}
*/
function checkDecl(node, value, getIndex) {
// make sure multiplication operations (*) are divided - not handled
// by postcss-value-parser
value = value.replace(/\*/g, ',');

valueParser(value).walk((valueNode) => {
const valueLowerCase = valueNode.value.toLowerCase();

// Ignore wrong units within `url` function
if (
valueNode.type === 'function' &&
(valueLowerCase === 'url' ||
optionsMatches(secondaryOptions, 'ignoreFunctions', valueLowerCase))
) {
return false;
}

check(
node,
getIndex(node),
valueNode,
node.prop,
secondaryOptions ? secondaryOptions.ignoreProperties : {},
);
});
}
return;
}

if (!isFunctionNode(componentValue) && !isSimpleBlockNode(componentValue)) return;

const state = {
ignored: componentValueIsIgnored(componentValue),
};

componentValue.walk((entry) => {
if (!entry.state) return;

if (entry.state.ignored) return;

root.walkAtRules(/^media$/i, (atRule) => checkMedia(atRule, atRule.params, atRuleParamIndex));
root.walkDecls((decl) => checkDecl(decl, decl.value, declarationValueIndex));
if (isTokenNode(entry.node)) {
check(
decl,
declarationValueIndex,
entry.node,
decl.prop,
secondaryOptions?.ignoreProperties,
);

return;
}

if (componentValueIsIgnored(entry.node)) {
entry.state.ignored = true;
}
}, state);
});
});
};
};

/**
* @param {string} value
* @returns {Array<import('@csstools/css-tokenizer').CSSToken>}
*/
function tokenizeWithoutPercentageTokens(value) {
return tokenize({ css: value }).map((x) => {
if (x[0] !== TokenType.Percentage) return x;

// Percentage values are not valid in media queries, so we can't parse them and get something valid back.
// Dimension tokens with a unit of "%" work just fine.
return [
TokenType.Dimension,
x[1],
x[2],
x[3],
{ value: x[4].value, unit: '%', type: NumberType.Number },
];
});
}

rule.primaryOptionArray = true;

rule.ruleName = ruleName;
Expand Down
4 changes: 2 additions & 2 deletions lib/rules/unit-no-unknown/index.js
View file
Expand Up @@ -13,6 +13,7 @@ const atRuleParamIndex = require('../../utils/atRuleParamIndex');
const declarationValueIndex = require('../../utils/declarationValueIndex');
const getAtRuleParams = require('../../utils/getAtRuleParams');
const getDeclarationValue = require('../../utils/getDeclarationValue');
const hasDimension = require('../../utils/hasDimension');
const isStandardSyntaxAtRule = require('../../utils/isStandardSyntaxAtRule');
const isStandardSyntaxDeclaration = require('../../utils/isStandardSyntaxDeclaration');
const isStandardSyntaxValue = require('../../utils/isStandardSyntaxValue');
Expand All @@ -35,7 +36,6 @@ const meta = {
url: 'https://stylelint.io/user-guide/rules/unit-no-unknown',
};

const HAS_DIMENSION_LIKE_VALUES = /\d[\w-]/;
const RESOLUTION_MEDIA_FEATURE_NAME = /^(?:min-|max-)?resolution$/i;

/** @type {import('stylelint').Rule} */
Expand Down Expand Up @@ -63,7 +63,7 @@ const rule = (primary, secondaryOptions) => {
* @param {string} value
*/
const tokenizeIfValueMightContainUnknownUnits = (value) => {
if (!HAS_DIMENSION_LIKE_VALUES.test(value)) return;
if (!hasDimension(value)) return;

const tokens = tokenize({ css: value });
const hasUnknownUnits = tokens.some((token) => {
Expand Down
13 changes: 13 additions & 0 deletions lib/utils/hasDimension.js
View file
@@ -0,0 +1,13 @@
'use strict';

const HAS_DIMENSION_LIKE_VALUES = /\d[%\w-]/;

/**
* Check if a value contains any dimension-like values.
*
* @param {string} value
* @returns {boolean}
*/
module.exports = function hasDimension(value) {
return HAS_DIMENSION_LIKE_VALUES.test(value);
};

0 comments on commit f1ace61

Please sign in to comment.