Add declaration-block-no-redundant-longhand-properties autofix (#6580)

* MVP for failing tests: add autofix * add updated version of test approach on padding * first pass for a complete-ish solution * fix incorrect ordering * add changeset * fix text-decoration longhand ordering * Uses insertion-order for `Set` instead of defining `orderedLonghandSubPropertiesOfShorthandProperties` explicitly * add typedef to reduce type imports * Apply suggestions from code review Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com> * Use `firstDeclNode` everywhere else * Applies @ybiquitous' (wonderful) suggestion Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com> * add TODO comment for text-decoration-thickness --------- Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com> Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
stylelint · Feb 10, 2023 · a38641c · a38641c
1 parent 6f9f41c
commit a38641c
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 21 deletions.
diff --git a/.changeset/pretty-seals-heal.md b/.changeset/pretty-seals-heal.md
@@ -0,0 +1,5 @@
+---
+"stylelint": minor
+---
+
+Added: `declaration-block-no-redundant-longhand-properties` autofix
diff --git a/docs/user-guide/rules.md b/docs/user-guide/rules.md
@@ -250,7 +250,7 @@ Require or disallow quotes with these `quotes` rules.
 
 Disallow redundancy with these `no-redundant` rules.
 
-- [`declaration-block-no-redundant-longhand-properties`](../../lib/rules/declaration-block-no-redundant-longhand-properties/README.md): Disallow redundant longhand properties within declaration-block (Ⓢ).
+- [`declaration-block-no-redundant-longhand-properties`](../../lib/rules/declaration-block-no-redundant-longhand-properties/README.md): Disallow redundant longhand properties within declaration-block (Autofixable) (Ⓢ).
 - [`shorthand-property-no-redundant-values`](../../lib/rules/shorthand-property-no-redundant-values/README.md): Disallow redundant values within shorthand properties (Autofixable).
 
 ### Whitespace inside

diff --git a/lib/reference/properties.js b/lib/reference/properties.js
@@ -19,6 +19,7 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 	[
 		'animation',
 		new Set([
+			// prettier-ignore
 			'animation-name',
 			'animation-duration',
 			'animation-timing-function',
@@ -32,6 +33,7 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 	[
 		'background',
 		new Set([
+			// prettier-ignore
 			'background-image',
 			'background-size',
 			'background-position',
@@ -45,18 +47,19 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 	[
 		'border',
 		new Set([
+			// prettier-ignore
 			'border-top-width',
+			'border-right-width',
 			'border-bottom-width',
 			'border-left-width',
-			'border-right-width',
 			'border-top-style',
+			'border-right-style',
 			'border-bottom-style',
 			'border-left-style',
-			'border-right-style',
 			'border-top-color',
+			'border-right-color',
 			'border-bottom-color',
 			'border-left-color',
-			'border-right-color',
 		]),
 	],
 	[
@@ -91,14 +94,15 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 		new Set([
 			// prettier-ignore
 			'border-top-color',
+			'border-right-color',
 			'border-bottom-color',
 			'border-left-color',
-			'border-right-color',
 		]),
 	],
 	[
 		'border-image',
 		new Set([
+			// prettier-ignore
 			'border-image-source',
 			'border-image-slice',
 			'border-image-width',
@@ -118,6 +122,7 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 	[
 		'border-inline-start',
 		new Set([
+			// prettier-ignore
 			'border-inline-start-width',
 			'border-inline-start-style',
 			'border-inline-start-color',
@@ -135,6 +140,7 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 	[
 		'border-radius',
 		new Set([
+			// prettier-ignore
 			'border-top-right-radius',
 			'border-top-left-radius',
 			'border-bottom-right-radius',
@@ -155,9 +161,9 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 		new Set([
 			// prettier-ignore
 			'border-top-style',
+			'border-right-style',
 			'border-bottom-style',
 			'border-left-style',
-			'border-right-style',
 		]),
 	],
 	[
@@ -216,18 +222,20 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 	[
 		'font',
 		new Set([
+			// prettier-ignore
 			'font-style',
 			'font-variant',
 			'font-weight',
 			'font-stretch',
 			'font-size',
-			'font-family',
 			'line-height',
+			'font-family',
 		]),
 	],
 	[
 		'grid',
 		new Set([
+			// prettier-ignore
 			'grid-template-rows',
 			'grid-template-columns',
 			'grid-template-areas',
@@ -295,14 +303,15 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 		new Set([
 			// prettier-ignore
 			'margin-top',
+			'margin-right',
 			'margin-bottom',
 			'margin-left',
-			'margin-right',
 		]),
 	],
 	[
 		'mask',
 		new Set([
+			// prettier-ignore
 			'mask-image',
 			'mask-mode',
 			'mask-position',
@@ -327,18 +336,20 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 		new Set([
 			// prettier-ignore
 			'padding-top',
+			'padding-right',
 			'padding-bottom',
 			'padding-left',
-			'padding-right',
 		]),
 	],
 	[
 		'text-decoration',
 		new Set([
 			// prettier-ignore
-			'text-decoration-color',
-			'text-decoration-style',
 			'text-decoration-line',
+			'text-decoration-style',
+			'text-decoration-color',
+			// TODO: add support for text-decoration-thickness, Level 4
+			// https://w3c.github.io/csswg-drafts/css-text-decor-4/#text-decoration-width-property
 		]),
 	],
 	[
@@ -352,10 +363,11 @@ const longhandSubPropertiesOfShorthandProperties = new Map([
 	[
 		'transition',
 		new Set([
-			'transition-delay',
-			'transition-duration',
+			// prettier-ignore
 			'transition-property',
+			'transition-duration',
 			'transition-timing-function',
+			'transition-delay',
 		]),
 	],
 ]);

diff --git a/lib/rules/declaration-block-no-redundant-longhand-properties/README.md b/lib/rules/declaration-block-no-redundant-longhand-properties/README.md
@@ -66,6 +66,8 @@ This rule complains when the following shorthand properties can be used:
 
 Flexbox-related properties can be ignored using `ignoreShorthands: ["/flex/"]` (see below).
 
+The [`fix` option](../../../docs/user-guide/options.md#fix) can automatically fix most of the problems reported by this rule.
+
 ## Options
 
 ### `true`

diff --git a/lib/rules/declaration-block-no-redundant-longhand-properties/__tests__/index.js b/lib/rules/declaration-block-no-redundant-longhand-properties/__tests__/index.js
@@ -5,6 +5,7 @@ const { messages, ruleName } = require('..');
 testRule({
 	ruleName,
 	config: [true],
+	fix: true,
 
 	accept: [
 		{
@@ -35,6 +36,7 @@ testRule({
 	reject: [
 		{
 			code: 'a { margin-left: 10px; margin-right: 10px; margin-top: 20px; margin-bottom: 30px; }',
+			fixed: 'a { margin: 20px 10px 30px 10px; }',
 			message: messages.expected('margin'),
 			line: 1,
 			column: 62,
@@ -43,6 +45,7 @@ testRule({
 		},
 		{
 			code: 'a { MARGIN-LEFT: 10px; MARGIN-RIGHT: 10px; margin-top: 20px; margin-bottom: 30px; }',
+			fixed: 'a { margin: 20px 10px 30px 10px; }',
 			message: messages.expected('margin'),
 			line: 1,
 			column: 62,
@@ -51,6 +54,7 @@ testRule({
 		},
 		{
 			code: 'a { MARGIN-LEFT: 10px; MARGIN-RIGHT: 10px; MARGIN-TOP: 20px; MARGIN-BOTTOM: 30px; }',
+			fixed: 'a { margin: 20px 10px 30px 10px; }',
 			message: messages.expected('margin'),
 			line: 1,
 			column: 62,
@@ -59,6 +63,7 @@ testRule({
 		},
 		{
 			code: 'a { padding-left: 10px; padding-right: 10px; padding-top: 20px; padding-bottom: 30px; }',
+			fixed: 'a { padding: 20px 10px 30px 10px; }',
 			message: messages.expected('padding'),
 			line: 1,
 			column: 65,
@@ -67,6 +72,7 @@ testRule({
 		},
 		{
 			code: 'a { font-style: italic; font-variant: normal; font-weight: bold; font-size: .8em; font-family: Arial; line-height: 1.2; font-stretch: normal; }',
+			fixed: 'a { font: italic normal bold normal .8em 1.2 Arial; }',
 			message: messages.expected('font'),
 			line: 1,
 			column: 121,
@@ -75,6 +81,7 @@ testRule({
 		},
 		{
 			code: 'a { border-top-width: 7px; border-top-style: double; border-top-color: green; }',
+			fixed: 'a { border-top: 7px double green; }',
 			message: messages.expected('border-top'),
 			line: 1,
 			column: 54,
@@ -83,6 +90,7 @@ testRule({
 		},
 		{
 			code: 'a { -webkit-transition-delay: 0.5s; -webkit-transition-duration: 2s; -webkit-transition-property: top; -webkit-transition-timing-function: ease; }',
+			fixed: 'a { -webkit-transition: top 2s ease 0.5s; }',
 			message: messages.expected('-webkit-transition'),
 			line: 1,
 			column: 104,
@@ -91,6 +99,7 @@ testRule({
 		},
 		{
 			code: 'a { -WEBKIT-transition-delay: 0.5s; -webKIT-transition-duration: 2s; -webkit-TRANSITION-property: top; -webkit-transition-timing-function: ease; }',
+			fixed: 'a { -webkit-transition: top 2s ease 0.5s; }',
 			message: messages.expected('-webkit-transition'),
 			line: 1,
 			column: 104,
@@ -99,6 +108,7 @@ testRule({
 		},
 		{
 			code: 'a { border-top-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-right-width: 1px; }',
+			fixed: 'a { border-width: 1px 1px 1px 1px; }',
 			message: messages.expected('border-width'),
 		},
 	],

diff --git a/lib/rules/declaration-block-no-redundant-longhand-properties/index.js b/lib/rules/declaration-block-no-redundant-longhand-properties/index.js
@@ -18,12 +18,15 @@ const messages = ruleMessages(ruleName, {
 
 const meta = {
 	url: 'https://stylelint.io/user-guide/rules/declaration-block-no-redundant-longhand-properties',
+	fixable: true,
 };
 
 const IGNORED_VALUES = new Set(['inherit']);
 
+/** @typedef {import('postcss').Declaration} Declaration */
+
 /** @type {import('stylelint').Rule} */
-const rule = (primary, secondaryOptions) => {
+const rule = (primary, secondaryOptions, context) => {
 	return (root, result) => {
 		const validOptions = validateOptions(
 			result,
@@ -42,7 +45,7 @@ const rule = (primary, secondaryOptions) => {
 			return;
 		}
 
-		/** @type {Map<string, string[]>} */
+		/** @type {Map<string, import('stylelint').ShorthandProperties[]>} */
 		const longhandToShorthands = new Map();
 
 		for (const [shorthand, longhandProps] of longhandSubPropertiesOfShorthandProperties.entries()) {
@@ -61,6 +64,8 @@ const rule = (primary, secondaryOptions) => {
 		eachDeclarationBlock(root, (eachDecl) => {
 			/** @type {Map<string, string[]>} */
 			const longhandDeclarations = new Map();
+			/** @type {Map<string, Declaration[]>} */
+			const longhandDeclarationNodes = new Map();
 
 			eachDecl((decl) => {
 				if (IGNORED_VALUES.has(decl.value)) {
@@ -80,21 +85,52 @@ const rule = (primary, secondaryOptions) => {
 				for (const shorthandProperty of shorthandProperties) {
 					const prefixedShorthandProperty = prefix + shorthandProperty;
 					const longhandDeclaration = longhandDeclarations.get(prefixedShorthandProperty) || [];
+					const longhandDeclarationNode =
+						longhandDeclarationNodes.get(prefixedShorthandProperty) || [];
 
 					longhandDeclaration.push(prop);
 					longhandDeclarations.set(prefixedShorthandProperty, longhandDeclaration);
 
-					const shorthandProps = /** @type {Map<string, Set<string>>} */ (
-						longhandSubPropertiesOfShorthandProperties
-					).get(shorthandProperty);
+					longhandDeclarationNode.push(decl);
+					longhandDeclarationNodes.set(prefixedShorthandProperty, longhandDeclarationNode);
+
+					const shorthandProps = longhandSubPropertiesOfShorthandProperties.get(shorthandProperty);
 					const prefixedShorthandData = Array.from(shorthandProps || []).map(
 						(item) => prefix + item,
 					);
 
-					if (!arrayEqual(prefixedShorthandData.sort(), longhandDeclaration.sort())) {
+					const copiedPrefixedShorthandData = [...prefixedShorthandData];
+
+					if (!arrayEqual(copiedPrefixedShorthandData.sort(), longhandDeclaration.sort())) {
 						continue;
 					}
 
+					if (context.fix) {
+						const declNodes = longhandDeclarationNodes.get(prefixedShorthandProperty) || [];
+						const [firstDeclNode] = declNodes;
+
+						if (firstDeclNode) {
+							const transformedDeclarationNodes = new Map(
+								declNodes.map((d) => [d.prop.toLowerCase(), d]),
+							);
+							const resolvedShorthandValue = prefixedShorthandData
+								.map((p) => transformedDeclarationNodes.get(p)?.value.trim())
+								.filter(Boolean)
+								.join(' ');
+
+							const newShorthandDeclarationNode = firstDeclNode.clone({
+								prop: prefixedShorthandProperty,
+								value: resolvedShorthandValue,
+							});
+
+							firstDeclNode.replaceWith(newShorthandDeclarationNode);
+
+							declNodes.forEach((node) => node.remove());
+
+							return;
+						}
+					}
+
 					report({
 						ruleName,
 						result,

diff --git a/types/stylelint/index.d.ts b/types/stylelint/index.d.ts
@@ -424,7 +424,7 @@ declare module 'stylelint' {
 	};
 
 	/** @internal */
-	export type LonghandSubPropertiesOfShorthandProperties = ReadonlyMap<
+	export type ShorthandProperties =
 		| 'animation'
 		| 'background'
 		| 'border'
@@ -459,7 +459,11 @@ declare module 'stylelint' {
 		| 'padding'
 		| 'text-decoration'
 		| 'text-emphasis'
-		| 'transition',
+		| 'transition';
+
+	/** @internal */
+	export type LonghandSubPropertiesOfShorthandProperties = ReadonlyMap<
+		ShorthandProperties,
 		ReadonlySet<string>
 	>;