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.

Sign up for GitHub

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

Jump to bottom

Fix trailing zeroes being removed by autofix in length-zero-no-unit #5256

Merged
merged 2 commits into from Apr 24, 2021
Merged

Fix trailing zeroes being removed by autofix in length-zero-no-unit #5256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bdaf27d
Refactor length-zero-no-unit to remove styleSearch
jeddy3 Apr 21, 2021
041f7be
Refactor amends
jeddy3 Apr 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion lib/rules/length-zero-no-unit/__tests__/index.js
View file
Open in desktop
Expand Up @@ -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; }',
Expand Down Expand Up @@ -318,7 +326,7 @@ testRule({
},
{
code: 'a { top: 0.000px; }',
fixed: 'a { top: 0; }',
fixed: 'a { top: 0.000; }',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have been outside the scope of this rule in favour of number-no-trailing-zeros.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. 👍🏼

So, this change may be a bug fix, not a refactoring?
(maybe, this PR title is confusing... 🤔 )


message: messages.rejected,
line: 1,
Expand Down Expand Up @@ -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) {}',
Expand Down
263 changes: 111 additions & 152 deletions lib/rules/length-zero-no-unit/index.js
View file
Open in desktop
Expand Up @@ -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,
},
);
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved

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;
Expand Down
11 changes: 11 additions & 0 deletions lib/utils/getAtRuleParams.js
View file
Open in desktop
@@ -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);
};
20 changes: 20 additions & 0 deletions lib/utils/setAtRuleParams.js
View file
Open in desktop
@@ -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;
};
ybiquitous marked this conversation as resolved.
Show resolved Hide resolved