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.

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

Fix false positives regression in named-grid-areas-no-invalid #6041

Closed
ai opened this issue Apr 26, 2022 · 3 comments · Fixed by #6042
Closed

Fix false positives regression in named-grid-areas-no-invalid #6041

ai opened this issue Apr 26, 2022 · 3 comments · Fixed by #6042
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule

Comments

@ai
Copy link
Member

ai commented Apr 26, 2022

What steps are needed to reproduce the bug?

$ pnpm add stylelint@14.8.0
$ echo "body { grid-template: 1fr / auto 1fr auto }" > test.css
$ echo '{ "rules": { "named-grid-areas-no-invalid": true } }' > .stylelintrc
$ pnpm stylelint test.css

test.css
 1:23  ✖  Expected same number of cell tokens in    named-grid-areas-no-invalid
          each string

What Stylelint configuration is needed to reproduce the bug?

{
  "rules": {
    "named-grid-areas-no-invalid": true
  }
}

How did you run Stylelint?

stylelint test.css

Which version of Stylelint are you using?

14.8.0

What did you expect to happen?

No error

What actually happened?

False positive error:

 1:23  ✖  Expected same number of cell tokens in    named-grid-areas-no-invalid
          each string

Does the bug relate to non-standard syntax?

No

Proposal to fix the bug

I saw this error just after update to 14.8 so it could be a regression

@ai ai changed the title False positive for named-grid-areas-no-invalid in Stylelint 14.8 Regression 14.8: false positive for named-grid-areas-no-invalid Apr 26, 2022
@ybiquitous ybiquitous added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule labels Apr 26, 2022
@ybiquitous
Copy link
Member

@ai Thank you for reporting the issue and using the template. I can confirm the regression also on the demo.

I've labeled the issue as ready to implement. Please consider contributing if you have time.

There are steps on how to fix a bug in a rule in the Developer guide.

@ybiquitous
Copy link
Member

ybiquitous commented Apr 26, 2022

@jeddy3 jeddy3 changed the title Regression 14.8: false positive for named-grid-areas-no-invalid Fix false positives regression in named-grid-areas-no-invalid Apr 27, 2022
@jeddy3 jeddy3 added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Apr 27, 2022
@ybiquitous
Copy link
Member

@jeddy3 Thanks for creating the quick fix #6042. I believe the fix is enough to suppress the regression. 👍🏼


I've looked into the real cause of the regression. See the change below of the isRectangular() utility.

Previous:

function isRectangular(areas) {
return areas.every((row, _i, arr) => row.length === arr[0].length);
}

Current:

function isRectangular(areas) {
const firstArea = areas[0];
if (firstArea === undefined) return false;
return areas.every((row) => row.length === firstArea.length);
}

When the area argument is an empty array,

  • the previous version returned true
  • the current version returns false instead

I was careless with the behavior of the Array.prototype.every() method. MDN says about the Array method:

every acts like the "for all" quantifier in mathematics. In particular, for an empty array, it returns true.

In summary, I believe the current behavior of isRectangular() and the #6042 change (that checks array.length before calling isRectangular) is reasonable.

@ybiquitous ybiquitous mentioned this issue Apr 28, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

3 participants