Fix declaration-block-no-redundant-longhand-properties autofix for transition

[jathak]

What steps are needed to reproduce the bug?

Given code like:

a {
  transition-delay: 500ms;
  transition-duration: 250ms;
  transition-timing-function: ease-in-out;
  transition-property: transform, visibility;
}

What Stylelint configuration is needed to reproduce the bug?

{
  "rules": {
    "declaration-block-no-redundant-longhand-properties": true
  }
}

How did you run Stylelint?

https://stylelint.io/demo/#N4Igxg9gJgpiBcICGACYAdAdilAXATkpgM4CWupEmAtLADZICe8KArAAzsC2xA3FjgJEyFKrQCuhUZhYAmDj37Y8hEuUo0KXUpgDm1AGbjMYaSxhJiMajuoRxuJYNUiN1AA74I7mPlzMVYQMIfC4AGhQAN1IyACNSOnJGJQBfEDCQAwSYADkkLjhEGAAPfPc6GAA6MGJidPAqLN0EEAxldBAS3BhMKGIOlgBtARwUDuJ-CsTMXGpITCbqCaIoJHwoDpGAXSw0jPmmgDEQriRcFoArYip62Hc6xDacccmYadwBsZAGbomOsJGL0YUx0swOpH0y16aw2CC+PxgfxAuxAKSAA

Which version of Stylelint are you using?

15.6.0

What did you expect to happen?

No problems to be reported, or, if a declaration-block-no-redundant-longhand-properties error is reported, the autofix should be

a {
  transition: transform 250ms ease-in-out 500ms, visibility 250ms ease-in-out 500ms;
}

What actually happened?

An error of Expected shorthand property "transition" with an incorrect suggested autofix of:

a {
  transition: transform, visibility 250ms ease-in-out 500ms;
}

Does the bug relate to non-standard syntax?

No

Proposal to fix the bug

IMO, this error shouldn't be reported at all for transitions with multiple properties (as reported in #4074), but if it is considered an error, then the autofix at least needs to be changed.

[ybiquitous]

@jathak Thanks for the report with a reproducible demo. I confirm the incorrect behavior.

Using ignoreShorthands option enables us to avoid this problem, although it's just a workaround. For example:

{
  "rules": {
    "declaration-block-no-redundant-longhand-properties": [true, {"ignoreShorthands": ["transition"]}]
  }
}

However, we probably need to provide a custom resolver for the transition shorthand like grid-template to improve user-friendliness:

stylelint/lib/rules/declaration-block-no-redundant-longhand-properties/index.js

Lines 28 to 50 in f7846cb

	const customResolvers = new Map([
	[
	'grid-template',
	(decls) => {
	const areas = decls.get('grid-template-areas')?.value.trim();
	const columns = decls.get('grid-template-columns')?.value.trim();
	const rows = decls.get('grid-template-rows')?.value.trim();
	if (!(areas && columns && rows)) return;
	const splitAreas = [...areas.matchAll(/"[^"]+"/g)].map((x) => x[0]);
	const splitRows = rows.split(' ');
	if (splitAreas.length === 0 || splitRows.length === 0) return;
	if (splitAreas.length !== splitRows.length) return;
	const zipped = splitAreas.map((area, i) => `${area} ${splitRows[i]}`).join(' ');
	return `${zipped} / ${columns}`;
	},
	],
	]);

I've labeled the issue as ready to implement. Please consider contributing if you have time.

There are steps on how to fix a bug in a rule in the Developer guide.

[mattxwang]

😓 sorry for the issue with this autofix (I initially contributed it). Have just submitted #6815 which should directly resolve the reported issue + better follow the CSS spec.

[mattxwang]

Ah, also - one other approach to consider is that we could create a new transition for each transition-property. This has the pro of being more (subjectively) readable, as discussed in #4074 and the original issue; the con is that we're potentially adding multiple nodes to the AST. I initially went ahead with the current approach since it's the easiest to implement; but, if we think the other approach is better, I'm happy to rework #6815 to make it behave that way instead!