Skip to content

Commit

Permalink
[Fix] sort-prop-types Pass all test cases
Browse files Browse the repository at this point in the history
ESlint >3

[Fix] sort-prop-types #3470
  • Loading branch information
ROSSROSALES authored and ljharb committed Nov 16, 2022
1 parent 611b9ee commit b40366d
Show file tree
Hide file tree
Showing 4 changed files with 801 additions and 502 deletions.
2 changes: 2 additions & 0 deletions docs/rules/sort-prop-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 17 additions & 17 deletions lib/rules/sort-prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ 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');

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

const messages = {
requiredPropsFirst: 'Required prop types must be listed before all other prop types',
callbackPropsLast: 'Callback prop types must be listed after all other prop types',
Expand All @@ -29,7 +29,7 @@ module.exports = {
recommended: false,
url: docsUrl('sort-prop-types'),
},
// fixable: 'code',
fixable: 'code',

messages,

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -150,7 +150,7 @@ module.exports = {
requiredPropsFirstSeen.add(curr);
report(context, messages.requiredPropsFirst, 'requiredPropsFirst', {
node: curr,
// fix
fix
});
}
return curr;
Expand All @@ -168,7 +168,7 @@ module.exports = {
callbackPropsLastSeen.add(prev);
report(context, messages.callbackPropsLast, 'callbackPropsLast', {
node: prev,
// fix
fix
});
}
return prev;
Expand All @@ -180,7 +180,7 @@ module.exports = {
propsNotSortedSeen.add(curr);
report(context, messages.propsNotSorted, 'propsNotSorted', {
node: curr,
// fix
fix
});
}
return prev;
Expand Down
35 changes: 31 additions & 4 deletions lib/util/propTypesSort.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,35 @@ 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.
*/
let 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 = [];
let newStart = 0;
let newEnd = 0;
try {
commentBefore = sourceCode.getCommentsBefore(node);
commentAfter = sourceCode.getCommentsAfter(node);
} catch (e) { /**/ };
if (commentAfter.length === 0 || commentBefore.length === 0) {
newStart = node.range[0]
newEnd = node.range[1]
}
const firstCommentBefore = commentBefore[0];
if (commentBefore.length >= 1) {
newStart = firstCommentBefore.range[0]
}
const lastCommentAfter = commentAfter[commentAfter.length - 1];
if (commentAfter.length >= 1) {
newEnd = lastCommentAfter.range[1]
}
commentnodeMap.set(node, { start: newStart, end: newEnd, hasComment: true });
};
const nodeGroups = allNodes.reduce((acc, curr) => {
if (curr.type === 'ExperimentalSpreadProperty' || curr.type === 'SpreadElement') {
acc.push([]);
Expand All @@ -135,7 +161,8 @@ function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirs

source = nodes.reduceRight((acc, attr, index) => {
const sortedAttr = sortedAttributes[index];
let sortedAttrText = context.getSourceCode().getText(sortedAttr);
let 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) {
Expand All @@ -146,16 +173,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));
}

Expand Down
Loading

0 comments on commit b40366d

Please sign in to comment.