From 813e89e53dd8bf0829935c31d92facccaf4ce4dd Mon Sep 17 00:00:00 2001 From: Richard Hallows Date: Sat, 24 Apr 2021 09:18:15 +0100 Subject: [PATCH] Fix autofix removing trailing zeroes in length-zero-no-unit (#5256) --- .../length-zero-no-unit/__tests__/index.js | 19 +- lib/rules/length-zero-no-unit/index.js | 263 ++++++++---------- lib/utils/getAtRuleParams.js | 11 + lib/utils/setAtRuleParams.js | 20 ++ 4 files changed, 160 insertions(+), 153 deletions(-) create mode 100644 lib/utils/getAtRuleParams.js create mode 100644 lib/utils/setAtRuleParams.js diff --git a/lib/rules/length-zero-no-unit/__tests__/index.js b/lib/rules/length-zero-no-unit/__tests__/index.js index 0ca340112e..b94822d73c 100644 --- a/lib/rules/length-zero-no-unit/__tests__/index.js +++ b/lib/rules/length-zero-no-unit/__tests__/index.js @@ -284,6 +284,14 @@ testRule({ line: 1, column: 11, }, + { + code: 'a { top: 0px /* comment */; }', + fixed: 'a { top: 0 /* comment */; }', + + message: messages.rejected, + line: 1, + column: 11, + }, { code: 'a { tOp: 0px; }', fixed: 'a { tOp: 0; }', @@ -318,7 +326,7 @@ testRule({ }, { code: 'a { top: 0.000px; }', - fixed: 'a { top: 0; }', + fixed: 'a { top: 0.000; }', message: messages.rejected, line: 1, @@ -409,6 +417,15 @@ testRule({ line: 1, column: 21, }, + { + code: '@media (min-width: 0px /* comment */) {}', + fixed: '@media (min-width: 0 /* comment */) {}', + + description: 'simple media feature with comment', + message: messages.rejected, + line: 1, + column: 21, + }, { code: '@media screen and (min-width: 0px) {}', fixed: '@media screen and (min-width: 0) {}', diff --git a/lib/rules/length-zero-no-unit/index.js b/lib/rules/length-zero-no-unit/index.js index 3ccc9d1fbf..042b2ec5d0 100644 --- a/lib/rules/length-zero-no-unit/index.js +++ b/lib/rules/length-zero-no-unit/index.js @@ -2,20 +2,22 @@ 'use strict'; -const _ = require('lodash'); -const beforeBlockString = require('../../utils/beforeBlockString'); -const blurComments = require('../../utils/blurComments'); -const hasBlock = require('../../utils/hasBlock'); +const valueParser = require('postcss-value-parser'); + +const atRuleParamIndex = require('../../utils/atRuleParamIndex'); +const declarationValueIndex = require('../../utils/declarationValueIndex'); +const getAtRuleParams = require('../../utils/getAtRuleParams'); +const getDeclarationValue = require('../../utils/getDeclarationValue'); const isCustomProperty = require('../../utils/isCustomProperty'); -const isLessVariable = require('../../utils/isLessVariable'); const isMathFunction = require('../../utils/isMathFunction'); +const isStandardSyntaxAtRule = require('../../utils/isStandardSyntaxAtRule'); const keywordSets = require('../../reference/keywordSets'); const optionsMatches = require('../../utils/optionsMatches'); const report = require('../../utils/report'); const ruleMessages = require('../../utils/ruleMessages'); -const styleSearch = require('style-search'); +const setAtRuleParams = require('../../utils/setAtRuleParams'); +const setDeclarationValue = require('../../utils/setDeclarationValue'); const validateOptions = require('../../utils/validateOptions'); -const valueParser = require('postcss-value-parser'); const ruleName = 'length-zero-no-unit'; @@ -23,180 +25,137 @@ const messages = ruleMessages(ruleName, { rejected: 'Unexpected unit', }); -function rule(actual, secondary, context) { +function rule(primary, secondary, context) { return (root, result) => { - const validOptions = validateOptions(result, ruleName, { actual }); + const validOptions = validateOptions( + result, + ruleName, + { + actual: primary, + }, + { + actual: secondary, + possible: { + ignore: ['custom-properties'], + }, + optional: true, + }, + ); - if (!validOptions) { - return; - } + if (!validOptions) return; - root.walkDecls((decl) => { - if (decl.prop.toLowerCase() === 'line-height') { - return; - } + let needsFix; - const stringValue = blurComments(decl.toString()); - const ignorableIndexes = new Array(stringValue.length).fill(false); - const parsedValue = valueParser(stringValue); + function check(node, nodeIndex, valueNode) { + const { value, sourceIndex } = valueNode; - parsedValue.walk((node, nodeIndex, nodes) => { - if (decl.prop.toLowerCase() === 'font' && node.type === 'div' && node.value === '/') { - const lineHeightNode = nodes[nodeIndex + 1]; - const lineHeightNodeValue = valueParser.stringify(lineHeightNode); + if (isMathFunction(valueNode)) return false; - for (let i = 0; i < lineHeightNodeValue.length; i++) { - ignorableIndexes[lineHeightNode.sourceIndex + i] = true; - } + if (!isWord(valueNode)) return; - return; - } + const numberUnit = valueParser.unit(value); - if (node.type !== 'function') { - return; - } + if (numberUnit === false) return; - const nodeValue = valueParser.stringify(node); - const ignoreFlag = isMathFunction(node); + const { number, unit } = numberUnit; - for (let i = 0; i < nodeValue.length; i++) { - ignorableIndexes[node.sourceIndex + i] = ignoreFlag; - } + if (unit === '') return; - if (isMathFunction) { - return false; - } - }); + if (!isLength(unit)) return; + + if (isFraction(unit)) return; + + if (!isZero(number)) return; - check(stringValue, decl, ignorableIndexes); - }); + if (context.fix) { + valueNode.value = number; + needsFix = true; - root.walkAtRules((atRule) => { - // Ignore Less variables - if (isLessVariable(atRule)) { return; } - const source = hasBlock(atRule) - ? beforeBlockString(atRule, { noRawBefore: true }) - : atRule.toString(); + report({ + index: nodeIndex + sourceIndex + number.length, + message: messages.rejected, + node, + result, + ruleName, + }); + } - check(source, atRule); - }); + function checkAtRule(node) { + if (!isStandardSyntaxAtRule(node)) return; - function check(value, node, ignorableIndexes = []) { - if (optionsMatches(secondary, 'ignore', 'custom-properties') && isCustomProperty(value)) { - return; - } + needsFix = false; - const fixPositions = []; - - styleSearch({ source: value, target: '0' }, (match) => { - const index = match.startIndex; - - // Given a 0 somewhere in the full property value (not in a string, thanks - // to styleSearch) we need to isolate the value that contains the zero. - // To do so, we'll find the last index before the 0 of a character that would - // divide one value in a list from another, and the next index of such a - // character; then we build a substring from those indexes, which we can - // assess. - - // If a single value includes multiple 0's (e.g. 100.01px), we don't want - // each 0 to be treated as a separate value, possibly resulting in multiple - // warnings for the same value (e.g. 0.00px). - // - // This check prevents that from happening: we build and check against a - // Set containing all the indexes that are part of a value already validated. - if (ignorableIndexes[index]) { - return; - } - - const prevValueBreakIndex = _.findLastIndex(value.substr(0, index), (char) => { - return [' ', ',', ')', '(', '#', ':', '\n', '\t'].includes(char); - }); - - // Ignore hex colors - if (value[prevValueBreakIndex] === '#') { - return; - } - - // If no prev break was found, this value starts at 0 - const valueWithZeroStart = prevValueBreakIndex === -1 ? 0 : prevValueBreakIndex + 1; - - const nextValueBreakIndex = _.findIndex(value.substr(valueWithZeroStart), (char) => { - return [' ', ',', ')', '/'].includes(char); - }); - - // If no next break was found, this value ends at the end of the string - const valueWithZeroEnd = - nextValueBreakIndex === -1 ? value.length : nextValueBreakIndex + valueWithZeroStart; - - const valueWithZero = value.slice(valueWithZeroStart, valueWithZeroEnd); - const parsedValue = valueParser.unit(valueWithZero); - - if (!parsedValue || (parsedValue && !parsedValue.unit)) { - return; - } - - if (parsedValue.unit.toLowerCase() === 'fr') { - return; - } - - // Add the indexes to ignorableIndexes so the same value will not - // be checked multiple times. - _.range(valueWithZeroStart, valueWithZeroEnd).forEach((i) => (ignorableIndexes[i] = true)); - - // Only pay attention if the value parses to 0 - // and units with lengths - if ( - Number.parseFloat(valueWithZero) !== 0 || - !keywordSets.lengthUnits.has(parsedValue.unit.toLowerCase()) - ) { - return; - } - - if (context.fix) { - fixPositions.unshift({ - startIndex: valueWithZeroStart, - length: valueWithZeroEnd - valueWithZeroStart, - }); - - return; - } - - report({ - message: messages.rejected, - node, - index: valueWithZeroEnd - parsedValue.unit.length, - result, - ruleName, - }); + const index = atRuleParamIndex(node); + const parsedValue = valueParser(getAtRuleParams(node)); + + parsedValue.walk((valueNode) => { + return check(node, index, valueNode); }); - if (fixPositions.length) { - fixPositions.forEach((fixPosition) => { - if (node.type === 'atrule') { - // Use `-1` for `@` character before each at rule - const realIndex = - fixPosition.startIndex - node.name.length - node.raws.afterName.length - 1; + if (needsFix) { + setAtRuleParams(node, parsedValue.toString()); + } + } + + function checkDecl(node) { + needsFix = false; - node.params = replaceZero(node.params, realIndex, fixPosition.length); - } else { - const realIndex = fixPosition.startIndex - node.prop.length - node.raws.between.length; + const { prop } = node; - node.value = replaceZero(node.value, realIndex, fixPosition.length); - } - }); + if (isLineHeight(prop)) return; + + if (optionsMatches(secondary, 'ignore', 'custom-properties') && isCustomProperty(prop)) + return; + + const index = declarationValueIndex(node); + const parsedValue = valueParser(getDeclarationValue(node)); + + parsedValue.walk((valueNode, valueNodeIndex, valueNodes) => { + if (isLineHeightValue(node, valueNodes, valueNodeIndex)) return; + + return check(node, index, valueNode); + }); + + if (needsFix) { + setDeclarationValue(node, parsedValue.toString()); } } + + root.walkAtRules(checkAtRule); + root.walkDecls(checkDecl); }; } -function replaceZero(input, startIndex, length) { - const stringStart = input.slice(0, startIndex); - const stringEnd = input.slice(startIndex + length); +function isLineHeightValue({ prop }, nodes, index) { + return ( + prop.toLowerCase() === 'font' && + index > 0 && + nodes[index - 1].type === 'div' && + nodes[index - 1].value === '/' + ); +} + +function isLineHeight(prop) { + return prop.toLowerCase() === 'line-height'; +} + +function isWord({ type }) { + return type === 'word'; +} + +function isLength(unit) { + return keywordSets.lengthUnits.has(unit.toLowerCase()); +} + +function isFraction(unit) { + return unit.toLowerCase() === 'fr'; +} - return `${stringStart}0${stringEnd}`; +function isZero(number) { + return Number.parseFloat(number) === 0; } rule.ruleName = ruleName; diff --git a/lib/utils/getAtRuleParams.js b/lib/utils/getAtRuleParams.js new file mode 100644 index 0000000000..ea891dcecc --- /dev/null +++ b/lib/utils/getAtRuleParams.js @@ -0,0 +1,11 @@ +'use strict'; + +const _ = require('lodash'); + +/** + * @param {import('postcss').AtRule} atRule + * @returns {string} + */ +module.exports = function getAtRuleParams(atRule) { + return _.get(atRule, 'raws.params.raw', atRule.params); +}; diff --git a/lib/utils/setAtRuleParams.js b/lib/utils/setAtRuleParams.js new file mode 100644 index 0000000000..7ccfff199a --- /dev/null +++ b/lib/utils/setAtRuleParams.js @@ -0,0 +1,20 @@ +'use strict'; + +const _ = require('lodash'); + +/** @typedef {import('postcss').AtRule} AtRule */ + +/** + * @param {AtRule} atRule + * @param {string} params + * @returns {AtRule} The atRulearation that was passed in. + */ +module.exports = function setAtRuleParams(atRule, params) { + if (_.has(atRule, 'raws.params')) { + _.set(atRule, 'raws.params.raw', params); + } else { + atRule.params = params; + } + + return atRule; +};