Skip to content

Commit

Permalink
Fix false positives for line-height, and for fr units in `length-…
Browse files Browse the repository at this point in the history
…zero-no-unit` (#4394)

* Ignore some properties with 0 value (#4250)

* Fix prettier issue

* Update tests

* Remove array with grid props

* Add logic to process font decl, update tests
  • Loading branch information
fanich37 authored and hudochenkov committed Nov 9, 2019
1 parent b8f872b commit 1c415f5
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 2 deletions.
131 changes: 131 additions & 0 deletions lib/rules/length-zero-no-unit/__tests__/index.js
Expand Up @@ -183,6 +183,65 @@ testRule(rule, {
code: 'a { margin : 0; }',
description: 'space before and after colon',
},
{
code: 'a { line-height : 0px; }',
description: 'ignore line-height property',
},
{
code: 'a { lIne-hEight: 0px; }',
},
{
code: 'a { grid-auto-columns: 0fr; }',
description: 'ignore some grid properties with `fr`',
},
{
code: 'a { grid-Auto-cOlUmns: 0fr; }',
description: 'ignore some grid properties with `fr`',
},
{
code: 'a { grid-template-columns: repeat(10, 0fr); }',
description: 'ignore some grid properties with `fr`',
},
{
code: 'a { Grid-TEMplate-cOlumns : repeat( 10, 0fr ); }',
description: 'ignore some grid properties with `fr`',
},
{
code: 'a { grid-template-rows: 0fr 1fr; }',
description: 'ignore some grid properties with `fr`',
},
{
code: 'a { grId-tEMplate-Rows:0fr 1fr; }',
description: 'ignore some grid properties with `fr`',
},
{
code: 'a { font: normal normal 400 16px/0px cursive; }',
description: 'ignore line-height in font declaration',
},
{
code: 'a { font: normal normal 400 16px/0 cursive; }',
description: 'ignore line-height in font declaration',
},
{
code: 'a { font: normal normal 400 0 / 0px cursive; }',
description: 'ignore line-height in font declaration',
},
{
code: 'a { font: normal normal 400 16px / 0px cursive; }',
description: 'ignore line-height in font declaration',
},
{
code: 'a { font: normal normal 400 16px/ 0px cursive; }',
description: 'ignore line-height in font declaration',
},
{
code: 'a { font: normal normal 400 16px /0px cursive; }',
description: 'ignore line-height in font declaration',
},
{
code: 'a { font: normal normal 400 1.2em cursive; }',
description: 'do not fail if no line-height in font declaration',
},
],

reject: [
Expand Down Expand Up @@ -400,6 +459,78 @@ testRule(rule, {
line: 1,
column: 11,
},
{
code: 'a { grid-template-columns: 0px 0fr 1fr };',
fixed: 'a { grid-template-columns: 0 0fr 1fr };',

message: messages.rejected,
line: 1,
column: 29,
},
{
code: 'a { grid-template-colUMns: 0px 0fr 1fr };',
fixed: 'a { grid-template-colUMns: 0 0fr 1fr };',

message: messages.rejected,
line: 1,
column: 29,
},
{
code: 'a { grid-template-rows: 40px 4fr 0px; };',
fixed: 'a { grid-template-rows: 40px 4fr 0; };',

message: messages.rejected,
line: 1,
column: 35,
},
{
code: 'a { grid-auto-columns: 0px };',
fixed: 'a { grid-auto-columns: 0 };',

message: messages.rejected,
line: 1,
column: 25,
},
{
code: 'a { grid-template-columns: repeat(2, 50px 0px) 100px; };',
fixed: 'a { grid-template-columns: repeat(2, 50px 0) 100px; };',

message: messages.rejected,
line: 1,
column: 44,
},
{
code: 'a { font: normal normal 400 0px / 0px cursive; }',
fixed: 'a { font: normal normal 400 0 / 0px cursive; }',

message: messages.rejected,
line: 1,
column: 30,
},
{
code: 'a { font: normal normal 400 0px /0px cursive; }',
fixed: 'a { font: normal normal 400 0 /0px cursive; }',

message: messages.rejected,
line: 1,
column: 30,
},
{
code: 'a { font: normal normal 400 0px/0px cursive; }',
fixed: 'a { font: normal normal 400 0/0px cursive; }',

message: messages.rejected,
line: 1,
column: 30,
},
{
code: 'a { font: normal normal 400 0px/ 0px cursive; }',
fixed: 'a { font: normal normal 400 0/ 0px cursive; }',

message: messages.rejected,
line: 1,
column: 30,
},
],
});

Expand Down
23 changes: 21 additions & 2 deletions lib/rules/length-zero-no-unit/index.js
Expand Up @@ -29,11 +29,26 @@ const rule = function(actual, secondary, context) {
}

root.walkDecls((decl) => {
if (decl.prop.toLowerCase() === 'line-height') {
return;
}

const stringValue = blurComments(decl.toString());
const ignorableIndexes = new Array(stringValue.length).fill(false);
const parsedValue = valueParser(stringValue);

parsedValue.walk((node) => {
parsedValue.walk((node, nodeIndex) => {
if (decl.prop.toLowerCase() === 'font' && node.type === 'div' && node.value === '/') {
const lineHeightNode = parsedValue.nodes[nodeIndex + 1];
const lineHeightNodeValue = valueParser.stringify(lineHeightNode);

for (let i = 0; i < lineHeightNodeValue.length; i++) {
ignorableIndexes[lineHeightNode.sourceIndex + i] = true;
}

return;
}

if (node.type !== 'function') {
return;
}
Expand Down Expand Up @@ -97,7 +112,7 @@ const rule = function(actual, secondary, context) {
const valueWithZeroStart = prevValueBreakIndex === -1 ? 0 : prevValueBreakIndex + 1;

const nextValueBreakIndex = _.findIndex(value.substr(valueWithZeroStart), (char) => {
return [' ', ',', ')'].indexOf(char) !== -1;
return [' ', ',', ')', '/'].indexOf(char) !== -1;
});

// If no next break was found, this value ends at the end of the string
Expand All @@ -111,6 +126,10 @@ const rule = function(actual, secondary, context) {
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));
Expand Down

0 comments on commit 1c415f5

Please sign in to comment.