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

css-not-sel-list matches on leading/trailing whitespace #168

Closed
edg2s opened this issue Jul 28, 2023 · 4 comments · Fixed by #169
Closed

css-not-sel-list matches on leading/trailing whitespace #168

edg2s opened this issue Jul 28, 2023 · 4 comments · Fixed by #169
Labels
Milestone

Comments

@edg2s
Copy link
Contributor

edg2s commented Jul 28, 2023

The following is a simple not selector, it's not a list:

a:not( .hidden ) {}

but as the regex considers whitespace anywhere to potentially be part of a complex selector, e.g. as in:

a:not(.foo .bar) {}
          ^

we get a false positive.

@edg2s
Copy link
Contributor Author

edg2s commented Jul 28, 2023

I note that other rules account for surrounding whitespace, e.g.

(selector) => /:nth-(child|last-child)\(\s*\d+\s*of/.test(selector),
however it may be worth auditing these newly created rules for other whitespace issues.

edg2s added a commit to wikimedia/stylelint-config-wikimedia that referenced this issue Jul 28, 2023
Due to an upstream bug in doiuse (anandthakker/doiuse#168)
we can't deploy this.
github-merge-queue bot pushed a commit to wikimedia/stylelint-config-wikimedia that referenced this issue Jul 28, 2023
@clshortfuse
Copy link
Collaborator

Thanks for filing the issue as well as the fix.

It seems you're right about the improper regex. Spec states:

Also as usual, white space is allowed around the arguments inside the parentheses of a functional pseudo-class unless otherwise specified.

https://www.w3.org/TR/selectors-4/#pseudo-classes

I'll work on reviewing the PR and seeing if there are any other related issues around the codebase.

@RJWadley
Copy link
Contributor

RJWadley commented Aug 8, 2023

I went through all the new features looking for similar issues. I'm not sure whether or not these ones are spec compliant:

svg-css might have false positives

css-rrggbbaa might not catch all 4-length colors

And for these ones I'm not familiar with the spec, so they might be wrong in subtle ways:
css-relative-colors
css-nth-child-of
css-case-insensitive

@clshortfuse
Copy link
Collaborator

clshortfuse commented Aug 8, 2023

@RJWadley 👋

Mentioning of the colors reminds me of work I had done on clean-css to fix some improper regex there. So we can probably just reuse their Regex instead of trying to figure it out again:

https://github.com/clean-css/clean-css/blob/master/lib/optimizer/level-1/value-optimizers/color.js

I haven't fully checked it, but I did write a thorough color parsing regex as seen here with the compiler source here. Looking back there are notes on how you don't need to close parenthesis according to CSS spec.

Functions aren't the same as selectors in spec, but worth noting:

A functional notation is a type of component value that can represent more complex types or invoke special processing. The syntax starts with the name of the function immediately followed by a left parenthesis (i.e. a ) followed by the argument(s) to the notation followed by a right parenthesis. White space is allowed, but optional, immediately inside the parentheses. Functions can take multiple arguments, which are formatted similarly to a CSS property value.

Some legacy functional notations, such as rgba(), use commas unnecessarily, but generally commas are only used to separate items in a list, or pieces of a grammar that would be ambiguous otherwise. If a comma is used to separate arguments, white space is optional before and after the comma.

https://www.w3.org/TR/css-values-3/#functional-notations

It sounds like some functions can be comma separated. We don't want to validate the CSS, just detect with the most basic parsing. That means even if somebody decides to incorrectly add commas to something like calc(), we shouldn't care.


Summed up, functional notation and CSS selectors can share the same regex if we're just checking by name (rgba, calc, :is). If checking the parameters themselves, we should first ignore whitespace near the parentheses, then do whatever extra if required for the check (eg: counting values, checking for commas, checking for lists, etc.)

Edit: Looking for closing parenthesis and any preceding whitespace isn't required, but I'd be fine with requiring it if it simplifies the check.

@clshortfuse clshortfuse added this to the 6.0.3 milestone Sep 19, 2023
clshortfuse pushed a commit that referenced this issue Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants