Add selector-max-compound-selectors rule

[dryoma]

Implements #1141 Please close/delete the #1166

Benchmark results:

> npm run benchmark-rule selector-max-depth 3

Warnings: 279
Mean: 66.98629777142861 ms
Deviation: 10.578904153861743 ms

[alexander-akait]

I think usage regex not good solution, you can use postcss-selector-parser https://github.com/postcss/postcss-selector-parser

[dryoma]

I was thinking of that, but there are several arguments against that:

1. postcss-selector-parser uses regexes too;

2. The docs and the API differ from the version used by stylelint (e.g, walkTags in the docs, but eachTag in the actual code of v1.3.3)

3. It's much slower:

   npm run benchmark-rule selector-max-depth 3
   
   Warnings: 279
   Mean: 152.12407722222224 ms
   Deviation: 27.9745142792245 ms
   

   Maybe it was me who used it all wrong (though the api is quite simple - just traversing stuff).

[alexander-akait]

@dryoma currently only available eachTag, next version added waklTags, yes use regexp, but change bugs in one package more simple, than each rule

[dryoma]

Sorry if I misunderstood the docs, but is using postcss-selector-parser mandatory? Cause from the docs I concluded, that is a recomendation, especially for those who are not experienced with regular expressions.

I personally wouldn't want to implement an additional layer of abstraction, especially since it is 2.5 times slower, and the rule is not similar to other rules (so no common bugs in core logics).

[alexander-akait]

@dryoma problems not performance, we support non-standard syntax and postcss-selector-parser good work with this and if we catch bug we change code inside package and it is allow fix bugs in all rules, where used postcss-selector-parser, also read regex some difficult, than use library, it is get many times to fix bug inside rule.

[davidtheclark]

I don't think the docs are misleading at all. They do not say "Do anything necessary to get the fastest possible performance, at the expense of anything else." They just say "Try out ways to speed up rules, and use benchmarking to test if they're working." Given some stats, it's a judgment call to determine how the final code should look.

[jeddy3]

Any final thought on this before I apply the changes?

Nope. I agree with the conclusion.

Are the docs going to be changed then?

Yes, I think that'll be best. Can you create a separate issue for that please?

[jeddy3]

Oops @davidtheclark, I missed your message. I agree with you.

There's no harm in updating this sentence though:

"Depending on the rule, you may also want to use postcss-value-parser and/or postcss-selector-parser, which are easier and more accurate than most people's guesses at regular expressions."

To be more about "we need to use standard parsers whenever possible... because there are significant gains in doing so". :)

[davidtheclark]

Sure.

[dryoma]

To be more about "we need to use standard parsers whenever possible... because there are significant gains in doing so".

Yep, I meant that part.

Thanks everyone for talking this out.

[alexander-akait]

@dryoma I think the rule is complete 👍

[davidtheclark]

Sorry to just come up with this idea --- but might it be more clear to word all this as targeting the number of combinators: so the rule name could be selector-max-combinators, and the description: Limit the number of combinators in selectors.

If those words are equally or more accurate, I'd prefer to use them, because "depth" is vague and unspecified, but the quantity of combinators is precise.

cc/ @stylelint/core

[alexander-akait]

@davidtheclark I think it is two difference rules, first about count selectors, second about count combinators selectors. Here we speak about first, i think.

[davidtheclark]

This rule counts compound selectors, right @dryoma? — which is the same thing as counting combinators and adding one.

[dryoma]

This rule counts compound selectors, right

Tunrs out that - yes, now after the change discussed above it would count compound selectors. So selector-max-combinators for simplicity (instead of selector-max-compound-selectors)?

[mehcode]

@dryoma / @davidtheclark

I'd say that selector-max-depth (even if it isn't as accurate as selector-max-combinators) means a lot more to the average CSS developer.

[jeddy3]

If those words are equally or more accurate, I'd prefer to use them, because "depth" is vague and unspecified, but the quantity of combinators is precise. cc/ @stylelint/core

Agreed. Lets go selector-max-combinators. We should focus this rule down to limiting the number of combinators (be it descendant, sibling etc..). It's both precise and tackles the use case in mind. I think max-depth is no long relevant and misrepresents the behaviour.

[dryoma]

@jeddy3 In fact, I'd prefer to go with selector-max-compound-selectors. Compound selector is a standardised definition and in my opinion conveys the rule's idea better.

[jeddy3]

I'd prefer to go with selector-more-compound-selectors

I find the selector terminology a little hard to grok (so bare with me), but doesn't that name imply that simple selectors aren't included in the count?

For example, I think the following complex selector:

div div.class + div  > div ~ div {}

Contains:

• 6 simple selectors
  • 5 of which are type selectors
  • 1 of which is a class selector
• 1 compound selector (made up of one of the type selectors and the class selector)
• 4 combinators
  • 1 of which is the descendant combinator
  • 1 of which is the the next-sibling combinator
  • 1 of which is the child combinator
  • 1 of which is the following-sibling combinator

So, I think there's perhaps scope (in the future) to write a number of *-max-* rules that limit these things e.g. selector-max-simple-selectors, selector-max-compound-selectors, selector-max-type-selectors etc... But I think the behaviour we've described above is limiting the number of combinators, and would be be most useful limiting rule. So, selector-max-combinators seems to apply here.

[dryoma]

The docs might indeed be misguiding there, but a compound selector can consist of just one simple selector. The only limit in the definition is how many type/universal selectors are allowed in a compound selector and where they are allowed. Moreover,

A combinator is punctuation that represents a particular kind of relationship between the compound selectors on either side.

Which, if your suggestion were correct, would mean that simple selectors are not allowed as operands for combinators.

So this

a div.class + a  > a ~ a {}

contains 5 compound selectors, 4 of which consist of just one simple selector. The number of those compound selectors is controlled by the rule.

[jeddy3]

Which, if your suggestion were correct, would mean that simple selectors are not allowed as operands for combinators.

Crikey, I think you're right.

"compound selector is a sequence of simple selectors"

I think this bit has confused me all this time, as I thought of a "sequence" as 2 or more things. But I realise now that's not correct.

selector-max-compound-selectors works for me :)

[davidtheclark]

selector-max-compound-selectors is fine with me, too. Thanks for hashing that out, team.

[dryoma]

All the changes discussed prior are applied (postcss-selector-parser, rule rename).

[coveralls]

Coverage increased (+0.01%) to 98.043% when pulling b48dd35 on dryoma:rule/selector-max-depth into d3b3e41 on stylelint:master.

[jeddy3]

LGTM.

A second pair of eyes would be good before merge.

@dryoma Thanks for making all the changes :)

[davidtheclark]

Thanks, @dryoma! Nice work.

[jeddy3]

Thanks, @dryoma! Nice work.

Agreed. It's an excellent rule!

Thanks also for sticking with us as we trashed-out how to name this rule unambiguously :)

[dryoma]

Great, thanks for merging this, and for all the comments! Glad I could contribute to such an awesome tool project :)

[felixfbecker]

Is it possible to ignore certain cases, for example allow rules to override styles under certain top-level classes like theme-dark?