Skip to content

Commit

Permalink
[Fix] sort-prop-types: restore autofixing
Browse files Browse the repository at this point in the history
ROSSROSALES authored and ljharb committed Oct 7, 2022
1 parent 5efd774 commit 902d555
Showing 6 changed files with 760 additions and 502 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -14,13 +14,15 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Fixed
* configs: avoid legacy config system error ([#3461][] @ljharb)
* [`jsx-no-target-blank`]: allow ternaries with literals ([#3464][] @akulsr0)
* [`sort-prop-types`]: restore autofixing ([#2574][] @ROSSROSALES)

### Changed
* [Perf] component detection: improve performance by avoiding traversing parents unnecessarily ([#3459][] @golopot)

[#3464]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3464
[#3461]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3461
[#3459]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3459
[#3452]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3452
[#3449]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3449
[#3424]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3429
[#2848]: https://github.com/jsx-eslint/eslint-plugin-react/pull/2848
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -342,7 +342,7 @@ module.exports = [
| | 🔧 | | [react/self-closing-comp](docs/rules/self-closing-comp.md) | Disallow extra closing tags for components without children |
| | | | [react/sort-comp](docs/rules/sort-comp.md) | Enforce component methods order |
| | | | [react/sort-default-props](docs/rules/sort-default-props.md) | Enforce defaultProps declarations alphabetical sorting |
| | | | [react/sort-prop-types](docs/rules/sort-prop-types.md) | Enforce propTypes declarations alphabetical sorting |
| | 🔧 | | [react/sort-prop-types](docs/rules/sort-prop-types.md) | Enforce propTypes declarations alphabetical sorting |
| | | | [react/state-in-constructor](docs/rules/state-in-constructor.md) | Enforce class component state initialization style |
| | | | [react/static-property-placement](docs/rules/static-property-placement.md) | Enforces where React component static properties should be positioned. |
| | | | [react/style-prop-object](docs/rules/style-prop-object.md) | Enforce style prop value is an object |
2 changes: 2 additions & 0 deletions docs/rules/sort-prop-types.md
Original file line number Diff line number Diff line change
@@ -2,6 +2,8 @@

💼 This rule is enabled in the following [configs](https://github.com/jsx-eslint/eslint-plugin-react#shareable-configurations): `all`.

🔧 This rule is automatically fixable using the `--fix` [flag](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) on the command line.

Some developers prefer to sort prop type declarations alphabetically to be able to find necessary declaration easier at the later time. Others feel that it adds complexity and becomes burden to maintain.

## Rule Details
32 changes: 16 additions & 16 deletions lib/rules/sort-prop-types.js
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ const variableUtil = require('../util/variable');
const propsUtil = require('../util/props');
const docsUrl = require('../util/docsUrl');
const propWrapperUtil = require('../util/propWrapper');
// const propTypesSortUtil = require('../util/propTypesSort');
const propTypesSortUtil = require('../util/propTypesSort');
const report = require('../util/report');

// ------------------------------------------------------------------------------
@@ -29,7 +29,7 @@ module.exports = {
recommended: false,
url: docsUrl('sort-prop-types'),
},
// fixable: 'code',
fixable: 'code',

messages,

@@ -106,17 +106,17 @@ module.exports = {
return;
}

// function fix(fixer) {
// return propTypesSortUtil.fixPropTypesSort(
// fixer,
// context,
// declarations,
// ignoreCase,
// requiredFirst,
// callbacksLast,
// sortShapeProp
// );
// }
function fix(fixer) {
return propTypesSortUtil.fixPropTypesSort(
fixer,
context,
declarations,
ignoreCase,
requiredFirst,
callbacksLast,
sortShapeProp
);
}

const callbackPropsLastSeen = new WeakSet();
const requiredPropsFirstSeen = new WeakSet();
@@ -150,7 +150,7 @@ module.exports = {
requiredPropsFirstSeen.add(curr);
report(context, messages.requiredPropsFirst, 'requiredPropsFirst', {
node: curr,
// fix
fix,
});
}
return curr;
@@ -168,7 +168,7 @@ module.exports = {
callbackPropsLastSeen.add(prev);
report(context, messages.callbackPropsLast, 'callbackPropsLast', {
node: prev,
// fix
fix,
});
}
return prev;
@@ -180,7 +180,7 @@ module.exports = {
propsNotSortedSeen.add(curr);
report(context, messages.propsNotSorted, 'propsNotSorted', {
node: curr,
// fix
fix,
});
}
return prev;
42 changes: 38 additions & 4 deletions lib/util/propTypesSort.js
Original file line number Diff line number Diff line change
@@ -116,9 +116,39 @@ function sorter(a, b, context, ignoreCase, requiredFirst, callbacksLast) {
* @param {Boolean=} sortShapeProp whether or not to sort propTypes defined in PropTypes.shape.
* @returns {Object|*|{range, text}} the sort order of the two elements.
*/
const commentnodeMap = new WeakMap(); // all nodes reference WeakMap for start and end range
function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirst, callbacksLast, sortShapeProp) {
function sortInSource(allNodes, source) {
const originalSource = source;
const sourceCode = context.getSourceCode();
for (let i = 0; i < allNodes.length; i++) {
const node = allNodes[i];
let commentAfter = [];
let commentBefore = [];
try {
commentBefore = sourceCode.getCommentsBefore(node);
commentAfter = sourceCode.getCommentsAfter(node);
} catch (e) { /**/ }
if (commentAfter.length === 0 && commentBefore.length === 0) {
commentnodeMap.set(node, { start: node.range[0], end: node.range[1], hasComment: false });
} else {
const firstCommentBefore = commentBefore[0];
if (commentBefore.length === 1) {
commentnodeMap.set(node, { start: firstCommentBefore.range[0], end: node.range[1], hasComment: true });
}
const firstCommentAfter = commentAfter[0];
if (commentAfter.length === 1) {
commentnodeMap.set(node, { start: node.range[0], end: firstCommentAfter.range[1], hasComment: true });
}
if (commentBefore.length === 1 && commentAfter.length === 1) {
commentnodeMap.set(node, {
start: firstCommentBefore.range[0],
end: firstCommentAfter.range[1],
hasComment: true,
});
}
}
}
const nodeGroups = allNodes.reduce((acc, curr) => {
if (curr.type === 'ExperimentalSpreadProperty' || curr.type === 'SpreadElement') {
acc.push([]);
@@ -135,7 +165,11 @@ function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirs

source = nodes.reduceRight((acc, attr, index) => {
const sortedAttr = sortedAttributes[index];
let sortedAttrText = context.getSourceCode().getText(sortedAttr);
const sourceCodeText = sourceCode.getText();
let sortedAttrText = sourceCodeText.substring(
commentnodeMap.get(sortedAttr).start,
commentnodeMap.get(sortedAttr).end
);
if (sortShapeProp && isShapeProp(sortedAttr.value)) {
const shape = getShapeProperties(sortedAttr.value);
if (shape) {
@@ -146,16 +180,16 @@ function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirs
sortedAttrText = attrSource.slice(sortedAttr.range[0], sortedAttr.range[1]);
}
}
return `${acc.slice(0, attr.range[0])}${sortedAttrText}${acc.slice(attr.range[1])}`;
return `${acc.slice(0, commentnodeMap.get(attr).start)}${sortedAttrText}${acc.slice(commentnodeMap.get(attr).end)}`;
}, source);
});
return source;
}

const source = sortInSource(declarations, context.getSourceCode().getText());

const rangeStart = declarations[0].range[0];
const rangeEnd = declarations[declarations.length - 1].range[1];
const rangeStart = commentnodeMap.get(declarations[0]).start;
const rangeEnd = commentnodeMap.get(declarations[declarations.length - 1]).end;
return fixer.replaceTextRange([rangeStart, rangeEnd], source.slice(rangeStart, rangeEnd));
}

1,182 changes: 701 additions & 481 deletions tests/lib/rules/sort-prop-types.js

Large diffs are not rendered by default.

0 comments on commit 902d555

Please sign in to comment.