Fix inaccurate end positions when postcss-resolve-nested-selector is used in selector-*

[ybiquitous]

What steps are needed to reproduce the bug?

Because the postcss-resolve-nested-selector package returns just a list of strings and loses a raw source position, there are some cases that we cannot calculate Stylelint problems' end positions accurately.

What Stylelint configuration is needed to reproduce the bug?

{
  "rules": {
    "selector-class-pattern": ["^[A-Z]+$", { "resolveNestedSelectors": true }]
  }
}

How did you run Stylelint?

CLI with sylelint *.css

Which version of Stylelint are you using?

HEAD ef37dc54e5e08786750baea0433114b0c56e965a

What did you expect to happen?

A correct end positions for nested selectors should be reported.

What actually happened?

See the following case:

stylelint/lib/rules/selector-class-pattern/index.js

Lines 92 to 98 in ef37dc5

	// TODO: `selector` may be resolved. So, getting its raw value may be pretty hard.
	// It means `endIndex` may be inaccurate (though non-standard selectors).
	//
	// For example, given ".abc { &_x {} }".
	// Then, an expected raw `selector` is "&_x",
	// but, an actual `selector` is ".abc_x".
	const endIndex = index + selector.length;

the endColumn value should be 10 but actually 11:

stylelint/lib/rules/selector-class-pattern/__tests__/index.js

Lines 168 to 175 in ef37dc5

	{
	code: '.A { &__B { }}',
	message: messages.expected('.A__B', '^[A-Z]+$'),
	line: 1,
	column: 6,
	endLine: 1,
	endColumn: 11,
	},

.A { &__B { }}
/*   ↑   ↑
     ↑   ↑
     6   10
*/

Does the bug relate to non-standard syntax?

Yes, it's related to nested selectors

Proposal to fix the bug

I don't have a good idea now.

---

• selector-class-pattern
• selector-max-attribute
• selector-max-class
• selector-max-combinators
• selector-max-id
• selector-max-type
• selector-max-compound-selectors
• selector-max-universal
• selector-max-pseudo-class
• selector-max-specificity
• selector-no-qualifying-type

see also #7482 (comment)

[jeddy3]

I don't have a good idea now.

Nor do I. I've labelled the issue as ready to implement in case any has any ideas.

[mattxwang]

Is one solution contributing upstream / having the package return the raw source positions? The actual library code is only 25 lines/rather small, so hopefully adjusting it wouldn't be too tricky?

[ybiquitous]

Porting the code of postcss-resolve-nested-selector may be a solution. If doing so, it may be helpful for plugin authors to expose the ported code as a utility. See below:

stylelint/docs/developer-guide/plugins.md

Line 314 in 1c386a6

- [postcss-resolve-nested-selector](https://github.com/davidtheclark/postcss-resolve-nested-selector): given a (nested) selector in a PostCSS AST, return an array of resolved selectors.

[ybiquitous]

If possible, I’d like to rely on a PostCSS plugin that implements nesting well because I think maintaining ported code may be too much for us.

For example, postcss-nesting seems to work for the CSS Nesting spec. If need to support non-standard syntaxes like Sass (e.g. &_foo) as before, postcss-nested may be a choice.

EDIT: it’s needed to investigate whether such plugins would fit to Stylelint, though.

[ybiquitous]

I'm revisiting this issue and investigating postcss-nesting and postcss-nested. In short, using these packages instead of postcss-resolve-nested-selector seems difficult because they're a PostCSS plugin and are not a utility to resolve nested selectors. Parsing selectors by the packages should cause performance overhead.

@romainmenke Do you have any good idea?

[romainmenke]

Would it be fine if the selectors are desugared purely with :is()?

.foo, bar {
  &:hover {}
}

gives : :is(.foo, bar):hover

I don't mind adding a package for this under csstools (similar to the specificity package).

[ybiquitous]

Ah, no, we don't want such a :is() optimization here. Instead of postcss-resolve-nested-selector, we want a utility to resolve nested selectors and follow the CSS Nesting spec.

postcss-resolve-nested-selector still works in most cases, but it becomes inappropriate for Stylelint since it supports the identifier concatenation like Sass (e.g. &_foo) which is forbidden in the CSS spec.

I'll dig into the problem more. Thanks for the quick response!

[romainmenke]

Do you have examples rules of where using :is() would give different outcomes?

Because resolving with :is() is the closest to the CSS nesting spec you can get in a static analysis tool.

[ybiquitous]

For example, in the selector-class-pattern code:

stylelint/lib/rules/selector-class-pattern/index.mjs

Lines 64 to 70 in eebb786

	for (const nestedSelector of resolveNestedSelector(selector, ruleNode)) {
	if (!isStandardSyntaxSelector(nestedSelector)) {
	continue;
	}
	parseSelector(nestedSelector, result, ruleNode, (s) => checkSelector(s, ruleNode));
	}

This rule resolves a nested selector as follows, so the :is() transformation is unnecessary here to check if the given pattern matches the resolve selector:

.a {
  .b {}
}
/* ↓ resolved */
.a .b {}

[romainmenke]

It is indeed unnecessary in simple cases but would it cause false positives/negatives if :is() was used?

---

This can be trivially transformed without :is() and the package I have in mind would also skip :is() here :

.a {
  .b {}
}

More problematic :

/* `divdiv` vs. div:is(div) */
div {
  && {}
}

.foo, #bar {
  & {
    /* `&` has specificity of 1 id selector for matches on `.foo` */
  }
}

/*
  `.baz .foo .bar {}`
  but also matches:
  `.baz.foo .bar {}`
  `.foo .baz .bar {}`
*/
.foo .bar {
  .baz & {}
}

/* https://github.com/w3c/csswg-drafts/issues/2881 */
.foo, fooz {
  .bar, baz {
    &:hover, &:focus {}
  }
}

[jeddy3]

Because resolving with :is() is the closest to the CSS nesting spec you can get in a static analysis tool.

That's my understanding, too:
https://drafts.csswg.org/css-nesting/#nest-selector

This change will have a greater impact on rules like no-descending-specificity and selector-max-specificity, but it's a necessary change to bring Stylelint inline with the specifications.

I don't mind adding a package for this under csstools (similar to the specificity package).

That'd be fantastic!

[romainmenke]

I've added a package for this, but there are some things that aren't entirely clear how they should work.

https://github.com/csstools/postcss-plugins/tree/main/packages/selector-resolve-nested

---

If I understand it correctly there are multiple issues here for Stylelint:

• postcss-resolve-nested-selector doesn't follow the CSS nesting specification so it no longer aligns with the current focus of Stylelint
• postcss-resolve-nested-selector takes strings as input and output so the source locations are lost

I think the first issue (spec compliance) is covered with the new package I linked above.
But the second issue is more complicated.

By resolving two selectors and combining their AST's the source locations get mangled anyway. It is fairly trivial to correct the source locations so that the parent selectors point to &.

e.g. .a and &:hover becomes : .a:hover
And .a has the same source locations as &.

It is much harder to keep .a pointing to its rule and the source location in that rule.
That information is lost.

Ideally all selector AST nodes would already be offset by their position in the overall CSS.
Then it wouldn't be needed to correct anything.

[ybiquitous]

@romainmenke Thanks for creating the package so quickly. It's fantastic!

Yes, your understanding is correct, and my question was bad. As you commented, keeping an original source position is almost impossible through resolving a nested selector via a library. Instead, we should save such a position in a rule logic, e.g. selector-class-pattern.

[ybiquitous]

I'd like to use @csstools/selector-resolve-nested soon instead of postcss-resolve-nested-selector, but we may not be able to do that since there are compatibility issues in postcss-resolve-nested-selector (see #7482).

[romainmenke]

One issue I didn't foresee is that rules like selector-max-type do not work well at all when resolving nesting following the specification.

This rule (and others like it?) analyse selectors in a more superficial/syntatical way.

The amount of type selectors in a list of complex selectors is just not something that is ever relevant in CSS for a browser.

The specificity is relevant and the matched elements are also relevant.
But counts of arbitrary bits are not.

This makes it hard to apply standard CSS analogues onto these rules.
We can't resolve nested selectors following the CSS standard and use that result in these rules.

Example :

foo #y, .z bar { foo > & {} }

Is equivalent to :

foo > :is(foo #y, .z bar) {}

This contains 3 type selectors.
It also needs to be exactly this form to have the correct specificity and to match the correct elements.

But a rule like selector-max-type previously would have counted 2 type selectors because it counted these separately.

Maybe rules that work like selector-max-type require a different method of resolving nested selectors? Something that focusses more on splitting into lists even when the end result is a completely different selector.

---

I think a larger effort is needed to redefine which rules should exist for selectors and how they should work.

I am concerned that without a good overview and a clear direction the end result wont be consistent.

[ybiquitous]

@romainmenke Thanks a lot for the further investigation. My understanding of selector-max-type is like this:

{
  "rules": {
    "selector-max-type": 2
  }
}

foo #y, .z bar { foo > & {} } /* original */

foo > :is(foo #y, .z bar) {} /* resolved with :is() */

foo > foo #y, foo > .z bar {} /* resolved without :is() */

Online demo

For now, all the examples are lint-free because the count of type selectors is within 2.
Surely, the resolution with :is() is confusing. 🤔

[romainmenke]

For now, all the examples are lint-free because the count of type selectors is within 2.

This indeed doesn't error, but because of another implementation detail.
Any contents of pseudo's like :is are counted separately.

---

I am going to try and find the time to add a second algorithm to resolve nested selectors without :is. The resulting selectors will not always have the correct specificity and they will not always match the correct elements. But this will make it easier to analyse other aspects of selectors.

Maybe we won't need this after all but I think that it will be easier to reason about all this if you can switch out the algorithms and investigate the different outcomes.

I think we might need both algorithms to cover the existing rules.

[jeddy3]

I think a larger effort is needed to redefine which rules should exist for selectors and how they should work.
I am concerned that without a good overview and a clear direction the end result wont be consistent.

This is the crux of the issue. When we first built these rules, selectors were a different beast:

• preprocessors resolved nesting at build-time
• there were few (if any) functional pseudo-classes
• people mostly used only classes (to limit specificity issues)

So we added rules to help users:

• limit the complexity of their selectors
• enforce methodologies like CSS Modules

For example, we added selector-no-id, which became selector-max-id. Then functional pseudo-classes came along, so we added options like ignoreContextFunctionalPseudoClasses to work around the limitations of the rule.

We then added rules like:

• max-nesting-depth
• selector-max-combinators
• selector-max-compound-selectors
• selector-max-specificity

There's an overlap between these rules as each tries to limit complexity (often naively).

A lot has changed in the past few years, and we should consider revising our selector rules to meet contemporary CSS. The most significant change is browsers support nesting and desugar to :is(), which is one of a handful of functional pseudo-classes with specific specificity behaviour.

This makes it hard to apply standard CSS analogues onto these rules.

Is it fair to say that only selector-max-specificity maps onto a standard CSS analogy?

If so, correctly resolving relative selectors makes sense for this rule. Whereas the other selector-max-* rules are naive and trying to resolve the nesting according to the spec is problematic. One option is that we change these rules so that:

• they apply to the relative selector, i.e. the unresolved selector
• naively count, i.e. include things inside of functional pseudo-classes unless specifically configured not to

This will:

• make it easier for users to understand the behaviour of the rules
• keep the rules useful for simple use cases such as "I don't want any attribute selectors in my code base" can be achieved with selector-max-attribute: 0

Are there other ways we can revisit these rules?

---

As an aside, it's starting to feel like our next major release is an opportunity to modernise quite a few of our rules to make them more useful for modern CSS. There will be tension between pushing forward and backward compatibility, but I believe most of our users will benefit from rules that align with modern CSS practices.

[romainmenke]

I think we might need both algorithms to cover the existing rules.

I've added this in : csstools/postcss-plugins#1267
But I am not really happy with the name for this function.

desugar still implies that it will removing nesting a way that the end product is a valid and equivalent selector.

If anyone has a suggestion, please let me know on that PR.

Aside from the naming I think this new function does help with this issue.
It eliminates nested selectors using the postcss-selector-parser AST.
So you can still walk the output of that function and directly analyse it, even when serializing it would produce nonsense.

div {
  && {}
}

• with :is -> div:is(div)
• without :is -> divdiv (would match <divdiv>, not <div>)

But the AST without :is still has two AST nodes, one for each div.

---

To circle back to the original issue: Fix inaccurate end positions.

By keeping track of the original selector before it is resolved we can accurately report where the problem is.

see :

stylelint/lib/rules/selector-max-specificity/index.mjs

Line 126 in 5c8fda0

const index = selector.first?.sourceIndex ?? 0;

---

Is it fair to say that only selector-max-specificity maps onto a standard CSS analogy?

At first glance, yes :)

naively count

Specificity also isn't a good replacement for "max N".
Because :is(#foo, input div p range img) has a specificity of [1 0 0] and the number of type selectors isn't surfaced.
using :is() as a regular selector here, not as a byproduct of nesting

I think there will always be a use case for placing limits on complexity, which is what those rules do imho.

[romainmenke]

I've added another demo implementation of this second function :
#7509

It is still a breaking change, but the change is much smaller and all of the original intent of the rule remains. That would not have been true if we used the first function for selector-max-type.

Doing this I am more confident that we do need both ways of handling nested selectors.
We should document when to use each function.

[github-actions]

This issue is older than one month. Please ask before opening a pull request, as it may no longer be relevant.

[github-actions]

This issue is older than one month. Please ask before opening a pull request, as it may no longer be relevant.