declaration-block-no-redundant-longhand-properties needs to be value type aware

[romainmenke]

declaration-block-no-redundant-longhand-properties large operates on value order and doesn't consider value types.

a {
	text-decoration-style: dotted;
	text-decoration-line: red; /* invalid! */
	text-decoration-color: underline; /* invalid! */
}

b {
	text-decoration-style: dotted;
	text-decoration-line: underline;
	text-decoration-color: red;
}

Converted to :

a {
	text-decoration: red dotted underline;
}

b {
	text-decoration: underline dotted red;
}

https://stylelint.io/demo/#N4Igxg9gJgpiBcICGACYAdAdugLjmAHjgLSyQBOSOAlhJsQM44CeANjPClBHjFANxZc+IqRgUqteq2qYOKcn0HZeoshEo06xSKw2cArpljkZc5QF8hmAEZohqkus1TGLdp268BDkU-Eaktpm8kYmIcrChP4SWvS6+gpKWBYgADQgAGbU7ABySAC2cIiEhQAO7AB0YAwM6eB02QDmCCAYmCgo6CDRMMYM3ZwA2lidnd1MbDBmJJCYzW5IxkjkUN2jKAC6KfVzzQBiGgVUrQBWDHT1sGV1iO3jIJPsM4NdIKxUMEzdaRsT7tNZLNGtQmotlqtXt0PvhviAdhYgA

Or even more exotic :

a {
	text-decoration-style: dotted;
	text-decoration-line: ; /* invalid! */
	text-decoration-color: underline red; /* invalid! */
}

Converted to :

a {
	text-decoration: dotted underline red;
}

https://stylelint.io/demo/#N4Igxg9gJgpiBcICGACYAdAdugLjmAHjgLSyQBOSOAlhJsQM44CeANjPClBHjFANxZc+IqRgUqteq2qYOKQdl6iyESjTrFIrNZwCumWORlyU5PooC+IADQgAZtXYA5JAFs4iQu4AO7AHRgDAy24HSOAOYIIBiYKCjoIIT4hgyJnADaWPHxiUxsMCYkkJiRjDhIhkjkUInZKAC6WNZ2JZEAYmpuVNEAVgx0obA+IYixuSD57EXpCSCsVDBMiTb1eSzTssXh1BHllVDVtQhzC-jLIM0glkA

You could argue that in this case the input is bogus, so that the output is unexpected is fine but this does surface the underlying flaw in the implementation.

Longhand to shorthand conversion needs to fully analyze property and value pairs, check that they are valid and effectively calculate something more similar to the computed value for all longhands.

Only when this passes for all known constituents properties would it be safe to combine these.

---

The issue here is not text-decoration itself, but that the implementation is not sufficiently sturdy to be reliable in most cases.

An implementation that is resilient to issues of this nature will be much more complex.

I don't know if this is desirable or worth it.

[romainmenke]

Extra disclosure :

I don't use this rule and actually actively push team members to use longhands over shorthands. I want this rule to work well for those who want to use it, but I am not attached to or invested in it.

[ybiquitous]

@romainmenke Thanks for opening the discussion. This issue is related to #7630 from the point of view of:

this does surface the underlying flaw in the implementation.

[romainmenke]

Yes, indeed related but distinct aspects :)

Edit: there might be other implementation issues, these two are the ones that stand out for me personally.

[Mouvedia]

Regarding value type awareness for autofixes, I knew that the rule lacked it.
I am sure that there are many more rules that are affected as well.
I personally think that it could be a nice enhancement if it doesn't have a significant perf cost.

I don't consider it major for 2 reasons:

• the source of the errors is the user himself and we are not erasing its errors while doing the autofix
• we have the declaration-property-value-no-unknown rule

Would not autofixing because the values are invalid be a breaking change?

I don't use this rule and actually actively push team members to use longhands over shorthands. I want this rule to work well for those who want to use it, but I am not attached to or invested in it.

❤️
That's often my position. You don't have to be a user to care.

[romainmenke]

we have the declaration-property-value-no-unknown rule

I wonder if the tools (csstree) used for declaration-property-value-no-unknown could also help with this rule?

csstree already understands and can validate property value pairs.

Largest concern with such a direction is that csstree isn't as actively maintained as I would want it to be.

[Mouvedia]

Largest concern with such a direction is that csstree isn't as actively maintained as I would want it to be.

I already voiced the fact that at least one of the core maintainers of stylelint should be able to merge PRs on csstree/csstree.
That's off-topic though.

… could also help with this rule?

Again at what cost?

1. Does it have significant perf cost?
2. What about consistency? Why would we enhance only that rule?
3. Is the throw on invalid values before an autofix considered a breaking change?

[romainmenke]

Does it have significant perf cost?

Yes, any extra parsing implies a performance cost.

Why would we enhance only that rule

I see it not as an enhancement but more a potential building block.

If we stick with the text-decoration example:
It is typed as <'text-decoration-line'> || <'text-decoration-thickness'> || <'text-decoration-style'> || <'text-decoration-color'> in https://drafts.csswg.org/css-text-decor-4/#propdef-text-decoration

While text-decoration-line is typed as none | [ underline || overline || line-through || blink ] | spelling-error | grammar-error

Expanding the type from text-decoration through text-decoration-line to none | ... is something that csstree already can do.

Can it do the inverse?
Can it analyze a bag of property/value pairs, organise these by shorthand groups and format as a single value? Or does it have the needed API's so that we can offload any mappings and property specific transforms?

[Mouvedia]

Concerning worthy refactors, I would consider the autofix with optional longhands missing a priority.
This would be real enhancement for users.

Can it analyze a bag of property/value pairs, organise these by shorthand groups and format as a single value? Or does it have the needed API's so that we can offload any mappings and property specific transforms?

Now we are getting somewhere. Offloading the responsibility to a dependency which is well maintained by formatting the raw into a compatible input would be interesting if it had the additional benefit of validating the values; I agree.

Only when this passes for all known constituents properties would it be safe to combine these.

#7609 is relevant in that regard.