From dd949bdcf4cb63436a03ce3472b8b934b4087f9d Mon Sep 17 00:00:00 2001 From: kawaguchi Date: Sun, 4 Sep 2022 15:42:06 +0900 Subject: [PATCH 1/9] Fix incorrect location for matching violating times in `time-min-milliseconds` --- .../time-min-milliseconds/__tests__/index.js | 38 +++++++++++ lib/rules/time-min-milliseconds/index.js | 65 +++++++++---------- 2 files changed, 69 insertions(+), 34 deletions(-) diff --git a/lib/rules/time-min-milliseconds/__tests__/index.js b/lib/rules/time-min-milliseconds/__tests__/index.js index 10fea8fc2b..54169991f1 100644 --- a/lib/rules/time-min-milliseconds/__tests__/index.js +++ b/lib/rules/time-min-milliseconds/__tests__/index.js @@ -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, + }, + ], + }, ], }); @@ -515,5 +534,24 @@ testRule({ line: 1, column: 40, }, + { + 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, + }, + ], + }, ], }); diff --git a/lib/rules/time-min-milliseconds/index.js b/lib/rules/time-min-milliseconds/index.js index d91378935d..c5e1657f47 100644 --- a/lib/rules/time-min-milliseconds/index.js +++ b/lib/rules/time-min-milliseconds/index.js @@ -3,13 +3,13 @@ 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 ruleName = 'time-min-milliseconds'; @@ -62,47 +62,44 @@ const rule = (primary, secondaryOptions) => { } 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); - } - } - } - } + const parsedValue = valueParser(getDeclarationValue(decl)); + let isValueTimeCount = 0; + + parsedValue.walk(({ value, sourceIndex }) => { + isValueTimeCount = valueTimeCheck(value, isValueTimeCount); + + if (isAcceptableTime(value) || (ignoreDelay && isValueTimeCount !== 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 {string} value + * @param {number} valueTimeCount + * @returns {number} */ - function getDuration(valueList) { - for (const value of valueList) { - const parsedTime = valueParser.unit(value); + function valueTimeCheck(value, valueTimeCount) { + if (isTime(value)) valueTimeCount++; - if (!parsedTime) continue; + if (value === ',') valueTimeCount = 0; - // The first numeric value in an animation shorthand is the duration. - return value; - } + return valueTimeCount; + } + + /** + * @param {string} time + * @returns {boolean} + */ + function isTime(time) { + const parsedTime = valueParser.unit(time); + + if (!parsedTime) return false; + + const unit = parsedTime.unit.toLowerCase(); - return undefined; + return unit === 'ms' || unit === 's'; } /** From 78493969d8594ce590d1efe01aa1a8a2a3bd56db Mon Sep 17 00:00:00 2001 From: kawaguchi Date: Sun, 4 Sep 2022 15:43:51 +0900 Subject: [PATCH 2/9] Fix `time-min-milliseconds` end positions --- .../time-min-milliseconds/__tests__/index.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/rules/time-min-milliseconds/__tests__/index.js b/lib/rules/time-min-milliseconds/__tests__/index.js index 54169991f1..24249c220f 100644 --- a/lib/rules/time-min-milliseconds/__tests__/index.js +++ b/lib/rules/time-min-milliseconds/__tests__/index.js @@ -485,54 +485,72 @@ 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; }', From 8e736648bdc380b708a19e54674f6233b4442cc3 Mon Sep 17 00:00:00 2001 From: kawaguchi Date: Tue, 6 Sep 2022 22:48:42 +0900 Subject: [PATCH 3/9] Fix: Refactor `isAcceptableTime` --- lib/rules/time-min-milliseconds/index.js | 45 ++++++++++++++---------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/lib/rules/time-min-milliseconds/index.js b/lib/rules/time-min-milliseconds/index.js index c5e1657f47..d034072f75 100644 --- a/lib/rules/time-min-milliseconds/index.js +++ b/lib/rules/time-min-milliseconds/index.js @@ -10,6 +10,7 @@ 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'; @@ -53,22 +54,30 @@ const rule = (primary, secondaryOptions) => { const propertyName = vendor.unprefixed(decl.prop.toLowerCase()); const propertyValue = decl.value; - if ( - longhandTimeProperties.has(propertyName) && - !isIgnoredProperty(propertyName) && - !isAcceptableTime(propertyValue) - ) { - complain(decl, 0, propertyValue.length); - } + const parsedValue = valueParser(getDeclarationValue(decl)); + + parsedValue.walk((node) => { + const dimension = getDimension(node); + + if ( + longhandTimeProperties.has(propertyName) && + !isIgnoredProperty(propertyName) && + !isAcceptableTime(dimension) + ) { + complain(decl, 0, propertyValue.length); + } + }); if (shorthandTimeProperties.has(propertyName)) { - const parsedValue = valueParser(getDeclarationValue(decl)); let isValueTimeCount = 0; - parsedValue.walk(({ value, sourceIndex }) => { + parsedValue.walk((node) => { + const { value, sourceIndex } = node; + const dimension = getDimension(node); + isValueTimeCount = valueTimeCheck(value, isValueTimeCount); - if (isAcceptableTime(value) || (ignoreDelay && isValueTimeCount !== 1)) return; + if (isAcceptableTime(dimension) || (ignoreDelay && isValueTimeCount !== 1)) return; complain(decl, sourceIndex, value.length); }); @@ -115,27 +124,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) { + 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(); - 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; } From f9d3746d54e3b19c18ef392116d912bdbd3394cc Mon Sep 17 00:00:00 2001 From: kawaguchi Date: Tue, 6 Sep 2022 22:50:36 +0900 Subject: [PATCH 4/9] Fix: Refactor --- lib/rules/time-min-milliseconds/index.js | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/rules/time-min-milliseconds/index.js b/lib/rules/time-min-milliseconds/index.js index d034072f75..27ff797163 100644 --- a/lib/rules/time-min-milliseconds/index.js +++ b/lib/rules/time-min-milliseconds/index.js @@ -53,10 +53,11 @@ const rule = (primary, secondaryOptions) => { root.walkDecls((decl) => { const propertyName = vendor.unprefixed(decl.prop.toLowerCase()); const propertyValue = decl.value; - const parsedValue = valueParser(getDeclarationValue(decl)); + let isValueTimeCount = 0; parsedValue.walk((node) => { + const { value, sourceIndex } = node; const dimension = getDimension(node); if ( @@ -66,22 +67,15 @@ const rule = (primary, secondaryOptions) => { ) { complain(decl, 0, propertyValue.length); } - }); - - if (shorthandTimeProperties.has(propertyName)) { - let isValueTimeCount = 0; - parsedValue.walk((node) => { - const { value, sourceIndex } = node; - const dimension = getDimension(node); + if (!shorthandTimeProperties.has(propertyName)) return; - isValueTimeCount = valueTimeCheck(value, isValueTimeCount); + isValueTimeCount = valueTimeCheck(value, isValueTimeCount); - if (isAcceptableTime(dimension) || (ignoreDelay && isValueTimeCount !== 1)) return; + if (isAcceptableTime(dimension) || (ignoreDelay && isValueTimeCount !== 1)) return; - complain(decl, sourceIndex, value.length); - }); - } + complain(decl, sourceIndex, value.length); + }); }); /** From 7be15a3876678231bddc68c4ffff406b734d8259 Mon Sep 17 00:00:00 2001 From: kawaguchi Date: Tue, 6 Sep 2022 23:28:24 +0900 Subject: [PATCH 5/9] Fix: Refactor `valueTimeCheck` --- lib/rules/time-min-milliseconds/index.js | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/lib/rules/time-min-milliseconds/index.js b/lib/rules/time-min-milliseconds/index.js index 27ff797163..3c369ffde5 100644 --- a/lib/rules/time-min-milliseconds/index.js +++ b/lib/rules/time-min-milliseconds/index.js @@ -70,7 +70,7 @@ const rule = (primary, secondaryOptions) => { if (!shorthandTimeProperties.has(propertyName)) return; - isValueTimeCount = valueTimeCheck(value, isValueTimeCount); + isValueTimeCount = valueTimeCheck(dimension, value, isValueTimeCount); if (isAcceptableTime(dimension) || (ignoreDelay && isValueTimeCount !== 1)) return; @@ -79,32 +79,21 @@ const rule = (primary, secondaryOptions) => { }); /** + * @param {import('postcss-value-parser').Dimension | {unit: null, number: null}} dimension * @param {string} value * @param {number} valueTimeCount * @returns {number} */ - function valueTimeCheck(value, valueTimeCount) { - if (isTime(value)) valueTimeCount++; + function valueTimeCheck(dimension, value, valueTimeCount) { + const { unit } = dimension; + + if (unit !== null) valueTimeCount++; if (value === ',') valueTimeCount = 0; return valueTimeCount; } - /** - * @param {string} time - * @returns {boolean} - */ - function isTime(time) { - const parsedTime = valueParser.unit(time); - - if (!parsedTime) return false; - - const unit = parsedTime.unit.toLowerCase(); - - return unit === 'ms' || unit === 's'; - } - /** * @param {string} propertyName * @returns {boolean} From b6d20ad0f46d58223b6b1308803eba42dcf53592 Mon Sep 17 00:00:00 2001 From: kawaguchi Date: Wed, 7 Sep 2022 00:56:09 +0900 Subject: [PATCH 6/9] Fix: Refactor `isValueTimeCount` -> `timeValueCount` --- lib/rules/time-min-milliseconds/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/time-min-milliseconds/index.js b/lib/rules/time-min-milliseconds/index.js index 3c369ffde5..1e2c352772 100644 --- a/lib/rules/time-min-milliseconds/index.js +++ b/lib/rules/time-min-milliseconds/index.js @@ -54,7 +54,7 @@ const rule = (primary, secondaryOptions) => { const propertyName = vendor.unprefixed(decl.prop.toLowerCase()); const propertyValue = decl.value; const parsedValue = valueParser(getDeclarationValue(decl)); - let isValueTimeCount = 0; + let timeValueCount = 0; parsedValue.walk((node) => { const { value, sourceIndex } = node; @@ -70,9 +70,9 @@ const rule = (primary, secondaryOptions) => { if (!shorthandTimeProperties.has(propertyName)) return; - isValueTimeCount = valueTimeCheck(dimension, value, isValueTimeCount); + timeValueCount = valueTimeCheck(dimension, value, timeValueCount); - if (isAcceptableTime(dimension) || (ignoreDelay && isValueTimeCount !== 1)) return; + if (isAcceptableTime(dimension) || (ignoreDelay && timeValueCount !== 1)) return; complain(decl, sourceIndex, value.length); }); From c71fb9f63eb80e22ccd90b81875ce1890288332e Mon Sep 17 00:00:00 2001 From: kawaguchi Date: Wed, 7 Sep 2022 00:57:19 +0900 Subject: [PATCH 7/9] Fix: Refactor `valueTimeCheck` -> `calcTimeValueCount` --- lib/rules/time-min-milliseconds/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/time-min-milliseconds/index.js b/lib/rules/time-min-milliseconds/index.js index 1e2c352772..afa93778e4 100644 --- a/lib/rules/time-min-milliseconds/index.js +++ b/lib/rules/time-min-milliseconds/index.js @@ -70,7 +70,7 @@ const rule = (primary, secondaryOptions) => { if (!shorthandTimeProperties.has(propertyName)) return; - timeValueCount = valueTimeCheck(dimension, value, timeValueCount); + timeValueCount = calcTimeValueCount(dimension, value, timeValueCount); if (isAcceptableTime(dimension) || (ignoreDelay && timeValueCount !== 1)) return; @@ -84,7 +84,7 @@ const rule = (primary, secondaryOptions) => { * @param {number} valueTimeCount * @returns {number} */ - function valueTimeCheck(dimension, value, valueTimeCount) { + function calcTimeValueCount(dimension, value, valueTimeCount) { const { unit } = dimension; if (unit !== null) valueTimeCount++; From 6695f8b8464c82eba5fba98db89eac280364bcd0 Mon Sep 17 00:00:00 2001 From: kawaguchi Date: Sun, 4 Sep 2022 15:42:06 +0900 Subject: [PATCH 8/9] Fix: Refactor --- lib/rules/time-min-milliseconds/index.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/rules/time-min-milliseconds/index.js b/lib/rules/time-min-milliseconds/index.js index afa93778e4..6834d71c80 100644 --- a/lib/rules/time-min-milliseconds/index.js +++ b/lib/rules/time-min-milliseconds/index.js @@ -79,7 +79,7 @@ const rule = (primary, secondaryOptions) => { }); /** - * @param {import('postcss-value-parser').Dimension | {unit: null, number: null}} dimension + * @param {{unit: string | null, number: string | null}} dimension * @param {string} value * @param {number} valueTimeCount * @returns {number} @@ -107,11 +107,11 @@ const rule = (primary, secondaryOptions) => { } /** - * @param {import('postcss-value-parser').Dimension | {unit: null, number: null}} valueNode + * @param {import('postcss-value-parser').Dimension | {unit: null, number: null}} dimension * @returns {boolean} */ - function isAcceptableTime(valueNode) { - const { unit, number } = valueNode; + function isAcceptableTime(dimension) { + const { unit, number } = dimension; if (unit === null || number === null) return true; @@ -121,13 +121,13 @@ const rule = (primary, secondaryOptions) => { return true; } - const _unit = unit.toLowerCase(); + const timeUnit = unit.toLowerCase(); - if (_unit === 'ms' && numTime < minimum) { + if (timeUnit === 'ms' && numTime < minimum) { return false; } - if (_unit === 's' && numTime * 1000 < minimum) { + if (timeUnit === 's' && numTime * 1000 < minimum) { return false; } From 7a8d66592b139ddc277af034e2bc72db65f2b7e2 Mon Sep 17 00:00:00 2001 From: Masafumi Koba <473530+ybiquitous@users.noreply.github.com> Date: Thu, 8 Sep 2022 08:48:21 +0900 Subject: [PATCH 9/9] Add changelog entry --- .changeset/old-tools-care.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/old-tools-care.md diff --git a/.changeset/old-tools-care.md b/.changeset/old-tools-care.md new file mode 100644 index 0000000000..a366f50729 --- /dev/null +++ b/.changeset/old-tools-care.md @@ -0,0 +1,5 @@ +--- +"stylelint": patch +--- + +Fixed: `time-min-milliseconds` incorrect location for matching violating times