Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect location for matching violating times in time-min-milliseconds #6319

Merged
merged 10 commits into from Sep 9, 2022
56 changes: 56 additions & 0 deletions lib/rules/time-min-milliseconds/__tests__/index.js
Expand Up @@ -371,6 +371,25 @@ testRule({
endLine: 1,
endColumn: 55,
},
{
code: 'a { animation: foo 0.01s ease 0.8s, bar 0.8s ease 0.01s; }',
warnings: [
{
message: messages.expected(MIN_VALUE),
line: 1,
column: 20,
endLine: 1,
endColumn: 25,
},
{
message: messages.expected(MIN_VALUE),
line: 1,
column: 51,
endLine: 1,
endColumn: 56,
},
],
},
],
});

Expand Down Expand Up @@ -466,54 +485,91 @@ testRule({
message: messages.expected(MIN_VALUE),
line: 1,
column: 26,
endLine: 1,
endColumn: 32,
},
{
code: 'a { -webkit-transition-duration: 0.009s; }',
message: messages.expected(MIN_VALUE),
line: 1,
column: 34,
endLine: 1,
endColumn: 40,
},
{
code: 'a { transition-duration: 80ms; }',
message: messages.expected(MIN_VALUE),
line: 1,
column: 26,
endLine: 1,
endColumn: 30,
},
{
code: 'a { transition: foo 0.008s linear; }',
message: messages.expected(MIN_VALUE),
line: 1,
column: 21,
endLine: 1,
endColumn: 27,
},
{
code: 'a { -webkit-transition: foo 0.008s linear; }',
message: messages.expected(MIN_VALUE),
line: 1,
column: 29,
endLine: 1,
endColumn: 35,
},
{
code: 'a { animation-duration: 0.009s; }',
message: messages.expected(MIN_VALUE),
line: 1,
column: 25,
endLine: 1,
endColumn: 31,
},
{
code: 'a { -webkit-animation-duration: 0.009s; }',
message: messages.expected(MIN_VALUE),
line: 1,
column: 33,
endLine: 1,
endColumn: 39,
},
{
code: 'a { animation-duration: 80ms; }',
message: messages.expected(MIN_VALUE),
line: 1,
column: 25,
endLine: 1,
endColumn: 29,
},
{
code: 'a { animation: foo 0.8s ease 0.2s, bar 20ms ease 0.2s; }',
message: messages.expected(MIN_VALUE),
line: 1,
column: 40,
endLine: 1,
endColumn: 44,
},
{
code: 'a { animation: foo 0.01s ease 0.8s, bar 20ms ease 0.01s; }',
warnings: [
{
message: messages.expected(MIN_VALUE),
line: 1,
column: 20,
endLine: 1,
endColumn: 25,
},
{
message: messages.expected(MIN_VALUE),
line: 1,
column: 41,
endLine: 1,
endColumn: 45,
},
],
},
],
});
91 changes: 40 additions & 51 deletions lib/rules/time-min-milliseconds/index.js
Expand Up @@ -3,13 +3,14 @@
const declarationValueIndex = require('../../utils/declarationValueIndex');
const { longhandTimeProperties, shorthandTimeProperties } = require('../../reference/properties');
const optionsMatches = require('../../utils/optionsMatches');
const postcss = require('postcss');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateOptions = require('../../utils/validateOptions');
const valueParser = require('postcss-value-parser');
const vendor = require('../../utils/vendor');
const { isNumber } = require('../../utils/validateTypes');
const getDeclarationValue = require('../../utils/getDeclarationValue');
const getDimension = require('../../utils/getDimension');

const ruleName = 'time-min-milliseconds';

Expand Down Expand Up @@ -52,57 +53,45 @@ const rule = (primary, secondaryOptions) => {
root.walkDecls((decl) => {
const propertyName = vendor.unprefixed(decl.prop.toLowerCase());
const propertyValue = decl.value;
const parsedValue = valueParser(getDeclarationValue(decl));
let timeValueCount = 0;

parsedValue.walk((node) => {
const { value, sourceIndex } = node;
const dimension = getDimension(node);

if (
longhandTimeProperties.has(propertyName) &&
!isIgnoredProperty(propertyName) &&
!isAcceptableTime(dimension)
) {
complain(decl, 0, propertyValue.length);
}

if (
longhandTimeProperties.has(propertyName) &&
!isIgnoredProperty(propertyName) &&
!isAcceptableTime(propertyValue)
) {
complain(decl, 0, propertyValue.length);
}
if (!shorthandTimeProperties.has(propertyName)) return;

if (shorthandTimeProperties.has(propertyName)) {
const valueListList = postcss.list.comma(propertyValue);

for (const valueListString of valueListList) {
const valueList = postcss.list.space(valueListString);

if (ignoreDelay) {
// Check only duration time values
const duration = getDuration(valueList);

if (duration && !isAcceptableTime(duration)) {
complain(decl, propertyValue.indexOf(duration), duration.length);
}
} else {
// Check all time values
for (const value of valueList) {
if (!isAcceptableTime(value)) {
complain(decl, propertyValue.indexOf(value), value.length);
}
}
}
}
}
timeValueCount = calcTimeValueCount(dimension, value, timeValueCount);

if (isAcceptableTime(dimension) || (ignoreDelay && timeValueCount !== 1)) return;

complain(decl, sourceIndex, value.length);
});
});

/**
* Get the duration within an `animation` or `transition` shorthand property value.
*
* @param {string[]} valueList
* @returns {string | undefined}
* @param {import('postcss-value-parser').Dimension | {unit: null, number: null}} dimension
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved
* @param {string} value
* @param {number} valueTimeCount
* @returns {number}
*/
function getDuration(valueList) {
for (const value of valueList) {
const parsedTime = valueParser.unit(value);
function calcTimeValueCount(dimension, value, valueTimeCount) {
const { unit } = dimension;

if (!parsedTime) continue;
if (unit !== null) valueTimeCount++;

// The first numeric value in an animation shorthand is the duration.
return value;
}
if (value === ',') valueTimeCount = 0;

return undefined;
return valueTimeCount;
}

/**
Expand All @@ -118,27 +107,27 @@ const rule = (primary, secondaryOptions) => {
}

/**
* @param {string} time
* @param {import('postcss-value-parser').Dimension | {unit: null, number: null}} valueNode
* @returns {boolean}
*/
function isAcceptableTime(time) {
const parsedTime = valueParser.unit(time);
function isAcceptableTime(valueNode) {
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved
const { unit, number } = valueNode;

if (!parsedTime) return true;
if (unit === null || number === null) return true;

const numTime = Number(parsedTime.number);
const numTime = Number(number);

if (numTime <= 0) {
return true;
}

const unit = parsedTime.unit.toLowerCase();
const _unit = unit.toLowerCase();
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved

if (unit === 'ms' && numTime < minimum) {
if (_unit === 'ms' && numTime < minimum) {
return false;
}

if (unit === 's' && numTime * 1000 < minimum) {
if (_unit === 's' && numTime * 1000 < minimum) {
return false;
}

Expand Down