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 for flush comments containing a comma in selector-max-universal #3729

Closed
ssivanatarajan opened this issue Oct 19, 2018 · 7 comments
Labels
good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule

Comments

@ssivanatarajan
Copy link
Contributor

ssivanatarajan commented Oct 19, 2018

There was a false positive for the selector's list with a commented selector in selector-max-universal rule

Which rule, if any, is the bug related to?

selector-max-universal

What CSS is needed to reproduce the bug?

e.g.

.newbackbtn,/* .searchall, */.calNav{
   -webkit-transform: rotateY(180deg);
   -moz-transform:rotateY(180deg);
   -ms-transform:rotateY(180deg);
   -o-transform:rotateY(180deg);
}

What stylelint configuration is needed to reproduce the bug?

e.g.

{
  "rules": {
    
  "selector-max-universal":0
  }
}

Which version of stylelint are you using?

9.2.1

How are you running stylelint: CLI, PostCSS plugin, Node API?

Node API

Does the bug relate to non-standard syntax (e.g. SCSS, Less etc.)?

NO,

What did you expect to happen?

No warnings to be flagged.

What actually happened (e.g. what warnings or errors did you get)?

The following warnings were flagged:

Expected "*/.calNav" to have no more than 0 universal selectors (selector-max-universal)
@jeddy3 jeddy3 changed the title false positive for Selectors with comment in selector-max-universal rule Fix false positives for flush comments containing a comma in selector-max-universal Oct 19, 2018
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule good first issue is good for newcomers labels Oct 19, 2018
@jeddy3
Copy link
Member

jeddy3 commented Oct 19, 2018

@ssivanatarajan Thanks for the report and for using the template.

This is an interesting bug, and is likely within the selector parser.

It seems it is only triggered when:

  • there is a comma within the comment
  • the comment is flush to the surrounding characters

For example, this will also trigger the bug:

flush/* , */flush {}
1:11 error Expected "*/flush" to have no more than 0 universal selectors (selector-max-universal)

There's a pending PR, #3284, to update the selector parser to the latest version. I suggest we wait for that to be merged to see if it resolves the issue. In the meanwhile, I believe both the following will workaround the issue:

  • remove the comma from within the comment
  • adding a space either side of the comment

@sswebcoder
Copy link
Member

I think what postcss not support comment as part of selector and as result we got selector with comment and with comma. After split selector by comma we got incorrect selectors.

@sswebcoder
Copy link
Member

image

@sswebcoder
Copy link
Member

image

@jeddy3
Copy link
Member

jeddy3 commented Nov 13, 2018

I think what postcss not support comment as part of selector and as result we got selector with comment and with comma.

Do you want to raise an issue upstream in PostCSS to query this behaviour?

@sswebcoder
Copy link
Member

@jeddy3
I will try to debug this and create Issue and PR in postcss-selector-parser.

@hudochenkov
Copy link
Member

Done in #3817

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule
Development

No branches or pull requests

4 participants