Adds threshold options for properties-order (#88)

* Adds emptyLineMinimumPropertyThreshold option - Partial implementation - Acceptance tests are passing - Rejection tests are failing * Resolves threshold setting problems with unspecified props * Adds docs for emptyLineMinimumPropertyThreshold option - Includes a few extra test absolutely verifying that the documented example is correct * Makes threshold setting opt-in * Adds test cases for integration of threshold setting and unspecified * Updates documentation for threshold option * Resolves merge conflicts * Restore tests now I'm no longer debugging 🤦 * Update rules/properties-order/README.md Fixes a docs link Co-Authored-By: Aleks Hudochenkov <aleks@hudochenkov.com> * Update rules/properties-order/README.md Co-Authored-By: Aleks Hudochenkov <aleks@hudochenkov.com> * Update rules/properties-order/README.md Co-Authored-By: Aleks Hudochenkov <aleks@hudochenkov.com> * Update rules/properties-order/README.md Co-Authored-By: Aleks Hudochenkov <aleks@hudochenkov.com> * Update rules/properties-order/README.md Co-Authored-By: Aleks Hudochenkov <aleks@hudochenkov.com> * Tweak emptyLineMinimumPropertyThreshold docs per suggestions * Remove `allNodesData` property from checkEmptyLineBefore * Attempts to make the docs clearer for the emptyLineMinimumPropertyThreshold config * Fix propsCount in checkNode.js * Moves propsCount outside of the loop * Concatenate long lines in properties-order README
hudochenkov · Aug 20, 2019 · d23db37 · d23db37
1 parent b003219
commit d23db37
Show file tree

Hide file tree

Showing 6 changed files with 1,026 additions and 12 deletions.
diff --git a/rules/properties-order/README.md b/rules/properties-order/README.md
@@ -18,7 +18,8 @@ This rule ignores variables (`$sass`, `@less`, `--custom-property`).
 * Options
 * Optional secondary options
 	* [`unspecified: "top"|"bottom"|"bottomAlphabetical"|"ignore"`](#unspecified-topbottombottomalphabeticalignore)
-	* [`emptyLineBeforeUnspecified: "always"|"never"`](#emptyLineBeforeUnspecified-alwaysnever)
+	* [`emptyLineBeforeUnspecified: "always"|"never"|"threshold"`](#emptyLineBeforeUnspecified-alwaysneverthreshold)
+	* [`emptyLineMinimumPropertyThreshold: <number>`](#emptylineminimumpropertythreshold-number)
 	* [`disableFix: true`](#disablefix-true)
 * [Autofixing caveats](#autofixing-caveats)
 
@@ -34,7 +35,7 @@ Within an order array, you can include
 * group objects with these properties:
 	* `order: "flexible"`: If property isn't set (the default), the properties in this group must come in the order specified. If `"flexible"`, the properties can be in any order as long as they are grouped correctly.
 	* `properties (array of strings)`: The properties in this group.
-	* `emptyLineBefore ("always"|"never")`: If `always`, this group must be separated from other properties by an empty newline. If emptyLineBefore is `never`, the group must have no empty lines separating it from other properties. By default this property isn't set. Rule will check empty lines between properties _only_. However, shared-line comments ignored by rule. Shared-line comment is a comment on the same line as declaration before this comment.
+	* `emptyLineBefore ("always"|"never"|"threshold")`: If `always`, this group must be separated from other properties by an empty newline. If emptyLineBefore is `never`, the group must have no empty lines separating it from other properties. By default this property isn't set. Rule will check empty lines between properties _only_. However, shared-line comments ignored by rule. Shared-line comment is a comment on the same line as declaration before this comment. For `threshold`, refer to the [`emptyLineMinimumPropertyThreshold` documentation](#emptylineminimumpropertythreshold-number).
 	* `noEmptyLineBetween`: If `true`, properties within group should not have empty lines between them.
 	* `groupName`: An optional name for the group. This will be used in error messages.
 
@@ -595,11 +596,14 @@ a {
 }
 ```
 
-### `emptyLineBeforeUnspecified: "always"|"never"`
+### `emptyLineBeforeUnspecified: "always"|"never"|"threshold"`
 
 Default behavior does not enforce the presence of an empty line before an unspecified block of properties.
 
-If `"always"`, the unspecified group must be separated from other properties by an empty newline. If `"never"`, the unspecified group must have no empty lines separating it from other properties.
+If `"always"`, the unspecified group must be separated from other properties by an empty newline. 
+If `"never"`, the unspecified group must have no empty lines separating it from other properties.
+
+For `"threshold"`, see the [`emptyLineMinimumPropertyThreshold` documentation](#emptylineminimumpropertythreshold-number) for more information.
 
 Given:
 
@@ -637,13 +641,139 @@ a {
 }
 ```
 
+### `emptyLineMinimumPropertyThreshold: <number>`
+
+If a group is configured with `emptyLineBefore: 'threshold'`, the empty line behaviour toggles based on the number of properties in the rule.
+
+When the configured minimum property threshold is reached, empty lines are **inserted**.  When the number of properties is **less than** the minimum property threshold, empty lines are **removed**.
+
+ _e.g. threshold set to **3**, and there are **5** properties in total, then groups set to `'threshold'` will have an empty line inserted._
+
+The same behaviour is applied to unspecified groups when `emptyLineBeforeUnspecified: "threshold"`
+
+Given:
+
+```js
+[
+    [
+        {
+            emptyLineBefore: 'threshold',
+            properties: ['display'],
+        },
+        {
+            emptyLineBefore: 'threshold',
+            properties: ['height', 'width'],
+        },
+        {
+            emptyLineBefore: 'always',
+            properties: ['border'],
+        },
+        {
+            emptyLineBefore: 'never',
+            properties: ['transform'],
+        },
+    ],
+    {
+       unspecified: 'bottom',
+       emptyLineBeforeUnspecified: 'threshold',
+       emptyLineMinimumPropertyThreshold: 4,
+    }
+]
+```
+
+The following patterns are considered warnings:
+
+```css
+a {
+    display: block;
+
+	height: 1px;
+    width: 2px;
+    color: blue;
+}
+
+a {
+    display: block;
+
+    height: 1px;
+    width: 2px;
+    border: 0;
+    color: blue;
+}
+
+a {
+    display: block;
+
+    height: 1px;
+    width: 2px;
+    border: 0;
+
+    transform: none;
+    color: blue;
+}
+```
+
+The following patterns are *not* considered warnings:
+
+```css
+a {
+    display: block;
+    height: 1px;
+    width: 2px;
+}
+
+a {
+    display: block;
+
+    height: 1px;
+    width: 2px;
+
+    border: 0;
+}
+
+a {
+    display: block;
+
+    height: 1px;
+    width: 2px;
+
+    border: 0;
+    transform: none;
+}
+
+a {
+    display: block;
+    height: 1px;
+
+    border: 0;
+}
+
+a {
+    border: 0;
+    transform: none;
+    color: blue;
+}
+
+a {
+    display: block;
+
+    height: 1px;
+    width: 2px;
+
+    border: 0;
+    transform: none;
+
+    color: blue;
+}
+```
+
 ### `disableFix: true`
 
 Disable autofixing. Autofixing is enabled by default if it's enabled in stylelint configuration.
 
 ## Autofixing caveats
 
-Properties will be grouped together, if other node types between them (except comments). They will be groupped with the first found property. E. g.:
+Properties will be grouped together, if other node types between them (except comments). They will be grouped with the first found property. E.g.:
 
 ```css
 /* Before: */

diff --git a/rules/properties-order/checkEmptyLineBefore.js b/rules/properties-order/checkEmptyLineBefore.js
@@ -4,7 +4,12 @@ const addEmptyLineBefore = require('./addEmptyLineBefore');
 const hasEmptyLineBefore = require('./hasEmptyLineBefore');
 const removeEmptyLinesBefore = require('./removeEmptyLinesBefore');
 
-module.exports = function checkEmptyLineBefore(firstPropData, secondPropData, sharedInfo) {
+module.exports = function checkEmptyLineBefore(
+	firstPropData,
+	secondPropData,
+	sharedInfo,
+	propsCount
+) {
 	const firstPropIsUnspecified = !firstPropData.orderData;
 	const secondPropIsUnspecified = !secondPropData.orderData;
 
@@ -22,6 +27,9 @@ module.exports = function checkEmptyLineBefore(firstPropData, secondPropData, sh
 		firstPropSeparatedGroup !== secondPropSeparatedGroup && !secondPropIsUnspecified;
 	const startOfUnspecifiedGroup = !firstPropIsUnspecified && secondPropIsUnspecified;
 
+	// Line threshold logic
+	const belowEmptyLineThreshold = propsCount < sharedInfo.emptyLineMinimumPropertyThreshold;
+
 	if (betweenGroupsInSpecified || startOfUnspecifiedGroup) {
 		// Get an array of just the property groups, remove any solo properties
 		const groups = _.reject(sharedInfo.expectation, _.isString);
@@ -32,7 +40,16 @@ module.exports = function checkEmptyLineBefore(firstPropData, secondPropData, sh
 			  _.get(groups[secondPropSeparatedGroup - 2], 'emptyLineBefore')
 			: sharedInfo.emptyLineBeforeUnspecified;
 
-		if (!hasEmptyLineBefore(secondPropData.node) && emptyLineBefore === 'always') {
+		// Threshold logic
+		const emptyLineThresholdInsertLines =
+			!belowEmptyLineThreshold && emptyLineBefore === 'threshold';
+		const emptyLineThresholdRemoveLines =
+			belowEmptyLineThreshold && emptyLineBefore === 'threshold';
+
+		if (
+			!hasEmptyLineBefore(secondPropData.node) &&
+			(emptyLineBefore === 'always' || emptyLineThresholdInsertLines)
+		) {
 			if (sharedInfo.isFixEnabled) {
 				addEmptyLineBefore(secondPropData.node, sharedInfo.context.newline);
 			} else {
@@ -43,7 +60,10 @@ module.exports = function checkEmptyLineBefore(firstPropData, secondPropData, sh
 					ruleName: sharedInfo.ruleName,
 				});
 			}
-		} else if (hasEmptyLineBefore(secondPropData.node) && emptyLineBefore === 'never') {
+		} else if (
+			hasEmptyLineBefore(secondPropData.node) &&
+			(emptyLineBefore === 'never' || emptyLineThresholdRemoveLines)
+		) {
 			if (sharedInfo.isFixEnabled) {
 				removeEmptyLinesBefore(secondPropData.node, sharedInfo.context.newline);
 			} else {

diff --git a/rules/properties-order/checkNode.js b/rules/properties-order/checkNode.js
@@ -10,6 +10,7 @@ const createFlatOrder = require('./createFlatOrder');
 module.exports = function checkNode(node, sharedInfo, originalNode) {
 	// First, check order
 	const allPropData = getAllPropData(node);
+	const propsCount = node.nodes.filter(item => utils.isProperty(item)).length;
 
 	if (!sharedInfo.isFixEnabled) {
 		allPropData.forEach(checkEveryPropForOrder);
@@ -91,7 +92,7 @@ module.exports = function checkNode(node, sharedInfo, originalNode) {
 			return;
 		}
 
-		checkEmptyLineBefore(previousNodeData, nodeData, sharedInfo);
+		checkEmptyLineBefore(previousNodeData, nodeData, sharedInfo, propsCount);
 	});
 
 	function checkEveryPropForOrder(propData, index, listOfProps) {

diff --git a/rules/properties-order/index.js b/rules/properties-order/index.js
@@ -11,7 +11,7 @@ const messages = stylelint.utils.ruleMessages(ruleName, {
 	expected: (first, second, groupName) =>
 		`Expected "${first}" to come before "${second}"${groupName ? ` in group "${groupName}"` : ''}`,
 	expectedEmptyLineBefore: property => `Expected an empty line before property "${property}"`,
-	rejectedEmptyLineBefore: property => `Unexpected an empty line before property "${property}"`,
+	rejectedEmptyLineBefore: property => `Unexpected empty line before property "${property}"`,
 });
 
 const rule = function(expectation, options, context = {}) {
@@ -27,8 +27,9 @@ const rule = function(expectation, options, context = {}) {
 				actual: options,
 				possible: {
 					unspecified: ['top', 'bottom', 'ignore', 'bottomAlphabetical'],
-					emptyLineBeforeUnspecified: ['always', 'never'],
+					emptyLineBeforeUnspecified: ['always', 'never', 'threshold'],
 					disableFix: _.isBoolean,
+					emptyLineMinimumPropertyThreshold: _.isNumber,
 				},
 				optional: true,
 			}
@@ -41,6 +42,11 @@ const rule = function(expectation, options, context = {}) {
 		// By default, ignore unspecified properties
 		const unspecified = _.get(options, 'unspecified', 'ignore');
 		const emptyLineBeforeUnspecified = _.get(options, 'emptyLineBeforeUnspecified', '');
+		const emptyLineMinimumPropertyThreshold = _.get(
+			options,
+			'emptyLineMinimumPropertyThreshold',
+			0
+		);
 		const disableFix = _.get(options, 'disableFix', false);
 		const isFixEnabled = context.fix && !disableFix;
 
@@ -51,6 +57,7 @@ const rule = function(expectation, options, context = {}) {
 			expectation,
 			unspecified,
 			emptyLineBeforeUnspecified,
+			emptyLineMinimumPropertyThreshold,
 			messages,
 			ruleName,
 			result,