Fix false positives for universal selector and disallowInList in no-duplicate-selectors

[srawlins]

Which issue, if any, is this issue related to?

Fixes #4683
Closes #4795

Is there anything in the PR that needs further explanation?

This code had a pretty big bug in it; it used .includes on a selector string, not a list, so it was just a substring match; the new code certainly takes more time, with more parsing, but is correct. Open to suggestions about how to do improve performance (perhaps cache parsed lists?).

[mattxwang]

Just a note, looks like this collides with #4795, which has a more narrow approach to resolve the issue. Looks like these changes supersede #4795 though! Not super familiar with the problem but can review if needed.

[jeddy3]

@srawlins Thanks for looking into this (and discovering the big bug behind this).

Open to suggestions about how to do improve performance (perhaps cache parsed lists?).

Let's try caching the parsed lists.

You can use npm run benchmark-rule -- no-duplicate-selectors "[true, {\"disallowInList\": true}]" to benchmark the rule.

At the moment master is:

jeddy3@jeddy3 stylelint % npm run benchmark-rule -- no-duplicate-selectors "[true, {\"disallowInList\": true}]"

> stylelint@13.5.0 benchmark-rule /Users/jeddy3/Projects/stylelint
> node scripts/benchmark-rule.js "no-duplicate-selectors" "[true, {\"disallowInList\": true}]"

Warnings: 78
Mean: 91.46247921052631 ms
Deviation: 28.897587464632007 ms

And this branch is:

jeddy3@jeddy3 stylelint % npm run benchmark-rule -- no-duplicate-selectors "[true, {\"disallowInList\": true}]" 

> stylelint@13.5.0 benchmark-rule /Users/jeddy3/Projects/stylelint
> node scripts/benchmark-rule.js "no-duplicate-selectors" "[true, {\"disallowInList\": true}]"

Warnings: 37
Mean: 10107.0616478 ms
Deviation: 508.2475623037317 ms

We don't need to make this branch comparable to master as correctness is better than speed, but it's worth seeing if caching will take a chunk out of this branch's numbers.

[srawlins]

Let's try caching the parsed lists.

Thanks so much for the benchmarking tips! I read the code further and discovered what nodeContextLookup does and how the rule loop uses one giant tree anyhow, so I re-used that for a more proper fix. I also added another test that was failing before this refactor.

[jeddy3]

Excellent. The benchmarks are now comparable.

While reviewing this pull request, @m-allanson and I had to refamiliarise ourselves with nodeContextLookup as it'd be a long while since either of us had used it. I'm glad you were able to make use of it to fix the bug without impacting the performance of the rule.

[m-allanson]

Thanks! LGTM 👍

(I fixed a merge conflict from a recent dep update)

[jeddy3]

• Fixed: no-duplicate-selectors false positives for universal selector and disallowInList (#4809).