Fix autofix removing trailing zeroes in length-zero-no-unit (#5256)

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) {}',

lib/rules/length-zero-no-unit/index.js

@@ -2,201 +2,160 @@
 
 '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';
 
 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;

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);
+};

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;
+};