From f47deef684e5d5538776dd11be8b2e0724094e9a Mon Sep 17 00:00:00 2001 From: Duhamel Guerrero Date: Sat, 5 Feb 2022 21:26:38 -0500 Subject: [PATCH] [New] `jsx-sort-props`: support multiline prop groups Fixes #3170. --- CHANGELOG.md | 2 + docs/rules/jsx-sort-props.md | 37 ++++ lib/rules/jsx-sort-props.js | 113 +++++++++-- tests/lib/rules/jsx-sort-props.js | 306 ++++++++++++++++++++++++++++++ 4 files changed, 438 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 161d7aabfe..55b58a197c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange * [`jsx-curly-brace-presence`]: add "propElementValues" config option ([#3191][] @ljharb) * add [`iframe-missing-sandbox`] rule ([#2753][] @tosmolka @ljharb) * [`no-did-mount-set-state`], [`no-did-update-set-state`]: no-op with react >= 16.3 ([#1754][] @ljharb) +* [`jsx-sort-props`]: support multiline prop groups ([#3198][] @duhamelgm) ### Fixed * [`prop-types`], `propTypes`: add support for exported type inference ([#3163][] @vedadeepta) @@ -31,6 +32,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange * [Docs] [`forbid-foreign-prop-types`]: document `allowInPropTypes` option ([#1815][] @ljharb) * [Refactor] [`jsx-sort-default-props`]: remove unnecessary code ([#1817][] @ljharb) +[#3198]: https://github.com/yannickcr/eslint-plugin-react/pull/3198 [#3195]: https://github.com/yannickcr/eslint-plugin-react/pull/3195 [#3191]: https://github.com/yannickcr/eslint-plugin-react/pull/3191 [#3190]: https://github.com/yannickcr/eslint-plugin-react/pull/3190 diff --git a/docs/rules/jsx-sort-props.md b/docs/rules/jsx-sort-props.md index 49c876ed7e..b0c98fe23f 100644 --- a/docs/rules/jsx-sort-props.md +++ b/docs/rules/jsx-sort-props.md @@ -29,6 +29,7 @@ Examples of **correct** code for this rule: "callbacksLast": , "shorthandFirst": , "shorthandLast": , + "multiline": "ignore" | "first" | "last", "ignoreCase": , "noSortAlphabetically": , "reservedFirst": |>, @@ -70,6 +71,42 @@ When `true`, short hand props must be listed after all other props (unless `call ``` +### `multiline` + +Enforced sorting for multiline props + +* `ignore`: Multiline props will not be taken in consideration for sorting. + +* `first`: Multiline props must be listed before all other props (unless `shorthandFirst` is set), but still respecting the alphabetical order. + +* `last`: Multiline props must be listed after all other props (unless either `callbacksLast` or `shorthandLast` are set), but still respecting the alphabetical order. + +Defaults to `ignore`. + +```jsx +// 'jsx-sort-props': [1, { multiline: 'first' }] + + +// 'jsx-sort-props': [1, { multiline: 'last' }] + +``` + ### `noSortAlphabetically` When `true`, alphabetical order is **not** enforced: diff --git a/lib/rules/jsx-sort-props.js b/lib/rules/jsx-sort-props.js index 6c864bdea1..533f0b3e19 100644 --- a/lib/rules/jsx-sort-props.js +++ b/lib/rules/jsx-sort-props.js @@ -6,6 +6,7 @@ 'use strict'; const propName = require('jsx-ast-utils/propName'); +const includes = require('array-includes'); const docsUrl = require('../util/docsUrl'); const jsxUtil = require('../util/jsx'); const report = require('../util/report'); @@ -18,6 +19,10 @@ function isCallbackPropName(name) { return /^on[A-Z]/.test(name); } +function isMultilineProp(node) { + return node.loc.start.line !== node.loc.end.line; +} + const messages = { noUnreservedProps: 'A customized reserved first list must only contain a subset of React reserved props. Remove: {{unreservedWords}}', listIsEmpty: 'A customized reserved first list must not be empty', @@ -25,6 +30,8 @@ const messages = { listCallbacksLast: 'Callbacks must be listed after all other props', listShorthandFirst: 'Shorthand props must be listed before all other props', listShorthandLast: 'Shorthand props must be listed after all other props', + listMultilineFirst: 'Multiline props must be listed before all other props', + listMultilineLast: 'Multiline props must be listed after all other props', sortPropsByAlpha: 'Props should be sorted alphabetically', }; @@ -75,6 +82,18 @@ function contextCompare(a, b, options) { } } + if (options.multiline !== 'ignore') { + const multilineSign = options.multiline === 'first' ? -1 : 1; + const aIsMultiline = isMultilineProp(a); + const bIsMultiline = isMultilineProp(b); + if (aIsMultiline && !bIsMultiline) { + return multilineSign; + } + if (!aIsMultiline && bIsMultiline) { + return -multilineSign; + } + } + if (options.noSortAlphabetically) { return 0; } @@ -127,6 +146,7 @@ const generateFixerFunction = (node, context, reservedList) => { const callbacksLast = configuration.callbacksLast || false; const shorthandFirst = configuration.shorthandFirst || false; const shorthandLast = configuration.shorthandLast || false; + const multiline = configuration.multiline || 'ignore'; const noSortAlphabetically = configuration.noSortAlphabetically || false; const reservedFirst = configuration.reservedFirst || false; @@ -138,6 +158,7 @@ const generateFixerFunction = (node, context, reservedList) => { callbacksLast, shorthandFirst, shorthandLast, + multiline, noSortAlphabetically, reservedFirst, reservedList, @@ -213,6 +234,34 @@ function validateReservedFirstConfig(context, reservedFirst) { } } +const reportedNodeAttributes = new WeakMap(); +/** + * Check if the current node attribute has already been reported with the same error type + * if that's the case then we don't report a new error + * otherwise we report the error + * @param {Object} nodeAttribute The node attribute to be reported + * @param {string} errorType The error type to be reported + * @param {Object} node The parent node for the node attribute + * @param {Object} context The context of the rule + * @param {Array} reservedList The list of reserved props + */ +function reportNodeAttribute(nodeAttribute, errorType, node, context, reservedList) { + const errors = reportedNodeAttributes.get(nodeAttribute) || []; + + if (includes(errors, errorType)) { + return; + } + + errors.push(errorType); + + reportedNodeAttributes.set(nodeAttribute, errors); + + report(context, messages[errorType], errorType, { + node: nodeAttribute.name, + fix: generateFixerFunction(node, context, reservedList), + }); +} + module.exports = { meta: { docs: { @@ -241,6 +290,11 @@ module.exports = { shorthandLast: { type: 'boolean', }, + // Whether multiline properties should be listed first or last + multiline: { + enum: ['ignore', 'first', 'last'], + default: 'ignore', + }, ignoreCase: { type: 'boolean', }, @@ -262,6 +316,7 @@ module.exports = { const callbacksLast = configuration.callbacksLast || false; const shorthandFirst = configuration.shorthandFirst || false; const shorthandLast = configuration.shorthandLast || false; + const multiline = configuration.multiline || 'ignore'; const noSortAlphabetically = configuration.noSortAlphabetically || false; const reservedFirst = configuration.reservedFirst || false; const reservedFirstError = validateReservedFirstConfig(context, reservedFirst); @@ -285,6 +340,8 @@ module.exports = { const currentValue = decl.value; const previousIsCallback = isCallbackPropName(previousPropName); const currentIsCallback = isCallbackPropName(currentPropName); + const previousIsMultiline = isMultilineProp(memo); + const currentIsMultiline = isMultilineProp(decl); if (ignoreCase) { previousPropName = previousPropName.toLowerCase(); @@ -304,10 +361,8 @@ module.exports = { return decl; } if (!previousIsReserved && currentIsReserved) { - report(context, messages.listReservedPropsFirst, 'listReservedPropsFirst', { - node: decl.name, - fix: generateFixerFunction(node, context, reservedList), - }); + reportNodeAttribute(decl, 'listReservedPropsFirst', node, context, reservedList); + return memo; } } @@ -319,10 +374,8 @@ module.exports = { } if (previousIsCallback && !currentIsCallback) { // Encountered a non-callback prop after a callback prop - report(context, messages.listCallbacksLast, 'listCallbacksLast', { - node: memo.name, - fix: generateFixerFunction(node, context, reservedList), - }); + reportNodeAttribute(memo, 'listCallbacksLast', node, context, reservedList); + return memo; } } @@ -332,10 +385,8 @@ module.exports = { return decl; } if (!currentValue && previousValue) { - report(context, messages.listShorthandFirst, 'listShorthandFirst', { - node: memo.name, - fix: generateFixerFunction(node, context, reservedList), - }); + reportNodeAttribute(decl, 'listShorthandFirst', node, context, reservedList); + return memo; } } @@ -345,10 +396,34 @@ module.exports = { return decl; } if (currentValue && !previousValue) { - report(context, messages.listShorthandLast, 'listShorthandLast', { - node: memo.name, - fix: generateFixerFunction(node, context, reservedList), - }); + reportNodeAttribute(memo, 'listShorthandLast', node, context, reservedList); + + return memo; + } + } + + if (multiline === 'first') { + if (previousIsMultiline && !currentIsMultiline) { + // Exiting the multiline prop section + return decl; + } + if (!previousIsMultiline && currentIsMultiline) { + // Encountered a non-multiline prop before a multiline prop + reportNodeAttribute(decl, 'listMultilineFirst', node, context, reservedList); + + return memo; + } + } + + if (multiline === 'last') { + if (!previousIsMultiline && currentIsMultiline) { + // Entering the multiline prop section + return decl; + } + if (previousIsMultiline && !currentIsMultiline) { + // Encountered a non-multiline prop after a multiline prop + reportNodeAttribute(memo, 'listMultilineLast', node, context, reservedList); + return memo; } } @@ -361,10 +436,8 @@ module.exports = { : previousPropName > currentPropName ) ) { - report(context, messages.sortPropsByAlpha, 'sortPropsByAlpha', { - node: decl.name, - fix: generateFixerFunction(node, context, reservedList), - }); + reportNodeAttribute(decl, 'sortPropsByAlpha', node, context, reservedList); + return memo; } diff --git a/tests/lib/rules/jsx-sort-props.js b/tests/lib/rules/jsx-sort-props.js index e6dd0aa0fc..519d143947 100644 --- a/tests/lib/rules/jsx-sort-props.js +++ b/tests/lib/rules/jsx-sort-props.js @@ -44,6 +44,14 @@ const expectedShorthandLastError = { messageId: 'listShorthandLast', type: 'JSXIdentifier', }; +const expectedMultilineFirstError = { + messageId: 'listMultilineFirst', + type: 'JSXIdentifier', +}; +const expectedMultilineLastError = { + messageId: 'listMultilineLast', + type: 'JSXIdentifier', +}; const expectedReservedFirstError = { messageId: 'listReservedPropsFirst', type: 'JSXIdentifier', @@ -95,6 +103,21 @@ const reservedFirstWithShorthandLast = [ ]; const reservedFirstAsEmptyArrayArgs = [{ reservedFirst: [] }]; const reservedFirstAsInvalidArrayArgs = [{ reservedFirst: ['notReserved'] }]; +const multilineFirstArgs = [{ multiline: 'first' }]; +const multilineAndShorthandFirstArgs = [ + { + multiline: 'first', + shorthandFirst: true, + }, +]; +const multilineLastArgs = [{ multiline: 'last' }]; +const multilineAndShorthandAndCallbackLastArgs = [ + { + multiline: 'last', + shorthandLast: true, + callbacksLast: true, + }, +]; ruleTester.run('jsx-sort-props', rule, { valid: parsers.all([ @@ -130,6 +153,97 @@ ruleTester.run('jsx-sort-props', rule, { code: ';', options: shorthandAndCallbackLastArgs, }, + // Sorting multiline props before others + { + code: ` + + `, + options: multilineFirstArgs, + }, + { + code: ` + + `, + options: multilineFirstArgs, + }, + { + code: ` + + `, + options: multilineAndShorthandFirstArgs, + }, + // Sorting multiline props after others + { + code: ` + + `, + options: multilineLastArgs, + }, + { + code: ` + + `, + options: multilineLastArgs, + }, + { + code: ` + ( + 1 + )} + e + f + onClick={() => ({ + gG: 1, + })} + /> + `, + options: multilineAndShorthandAndCallbackLastArgs, + }, // noSortAlphabetically { code: ';', options: noSortAlphabeticallyArgs }, { code: ';', options: noSortAlphabeticallyArgs }, @@ -494,6 +608,198 @@ ruleTester.run('jsx-sort-props', rule, { output: ';', options: reservedFirstAndCallbacksLastArgs, errors: [expectedCallbackError], + // multiline first + }, + { + code: ` + + `, + options: multilineFirstArgs, + errors: [expectedMultilineFirstError], + output: ` + + `, + }, + { + code: ` + + `, + options: multilineAndShorthandFirstArgs, + errors: [expectedMultilineFirstError, expectedShorthandFirstError], + output: ` + + `, + }, + // multiline last + { + code: ` + + `, + options: multilineLastArgs, + errors: [expectedMultilineLastError], + output: ` + + `, + }, + { + code: ` + ({ + c: 1 + })} + d="dD" + e={() => ({ + eE: 1 + })} + f + /> + `, + options: multilineAndShorthandAndCallbackLastArgs, + errors: [ + { + messageId: 'listShorthandLast', + line: 6, + }, + { + messageId: 'listCallbacksLast', + line: 8, + }, + ], + output: ` + ({ + eE: 1 + })} + b + f + onClick={() => ({ + c: 1 + })} + /> + `, + }, + { + code: ` + + `, + options: [ + { + multiline: 'last', + shorthandFirst: true, + callbacksLast: true, + reservedFirst: true, + ignoreCase: true, + }, + ], + output: ` + + `, + errors: [ + { + messageId: 'listMultilineLast', + line: 4, + }, + { + messageId: 'listReservedPropsFirst', + line: 12, + }, + { + messageId: 'listShorthandFirst', + line: 13, + }, + { + messageId: 'listCallbacksLast', + line: 19, + }, + ], }, ]), });