Skip to content

Commit b40366d

Browse files
ROSSROSALESljharb
authored andcommitted
[Fix] sort-prop-types Pass all test cases
ESlint >3 [Fix] sort-prop-types jsx-eslint#3470
1 parent 611b9ee commit b40366d

File tree

4 files changed

+801
-502
lines changed

4 files changed

+801
-502
lines changed

docs/rules/sort-prop-types.md

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

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

5+
🔧 This rule is automatically fixable using the `--fix` [flag](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) on the command line.
6+
57
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.
68

79
## Rule Details

lib/rules/sort-prop-types.js

+17-17
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ const variableUtil = require('../util/variable');
88
const propsUtil = require('../util/props');
99
const docsUrl = require('../util/docsUrl');
1010
const propWrapperUtil = require('../util/propWrapper');
11-
// const propTypesSortUtil = require('../util/propTypesSort');
11+
const propTypesSortUtil = require('../util/propTypesSort');
1212
const report = require('../util/report');
1313

1414
// ------------------------------------------------------------------------------
1515
// Rule Definition
1616
// ------------------------------------------------------------------------------
17-
17+
1818
const messages = {
1919
requiredPropsFirst: 'Required prop types must be listed before all other prop types',
2020
callbackPropsLast: 'Callback prop types must be listed after all other prop types',
@@ -29,7 +29,7 @@ module.exports = {
2929
recommended: false,
3030
url: docsUrl('sort-prop-types'),
3131
},
32-
// fixable: 'code',
32+
fixable: 'code',
3333

3434
messages,
3535

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

109-
// function fix(fixer) {
110-
// return propTypesSortUtil.fixPropTypesSort(
111-
// fixer,
112-
// context,
113-
// declarations,
114-
// ignoreCase,
115-
// requiredFirst,
116-
// callbacksLast,
117-
// sortShapeProp
118-
// );
119-
// }
109+
function fix(fixer) {
110+
return propTypesSortUtil.fixPropTypesSort(
111+
fixer,
112+
context,
113+
declarations,
114+
ignoreCase,
115+
requiredFirst,
116+
callbacksLast,
117+
sortShapeProp
118+
);
119+
}
120120

121121
const callbackPropsLastSeen = new WeakSet();
122122
const requiredPropsFirstSeen = new WeakSet();
@@ -150,7 +150,7 @@ module.exports = {
150150
requiredPropsFirstSeen.add(curr);
151151
report(context, messages.requiredPropsFirst, 'requiredPropsFirst', {
152152
node: curr,
153-
// fix
153+
fix
154154
});
155155
}
156156
return curr;
@@ -168,7 +168,7 @@ module.exports = {
168168
callbackPropsLastSeen.add(prev);
169169
report(context, messages.callbackPropsLast, 'callbackPropsLast', {
170170
node: prev,
171-
// fix
171+
fix
172172
});
173173
}
174174
return prev;
@@ -180,7 +180,7 @@ module.exports = {
180180
propsNotSortedSeen.add(curr);
181181
report(context, messages.propsNotSorted, 'propsNotSorted', {
182182
node: curr,
183-
// fix
183+
fix
184184
});
185185
}
186186
return prev;

lib/util/propTypesSort.js

+31-4
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,35 @@ function sorter(a, b, context, ignoreCase, requiredFirst, callbacksLast) {
116116
* @param {Boolean=} sortShapeProp whether or not to sort propTypes defined in PropTypes.shape.
117117
* @returns {Object|*|{range, text}} the sort order of the two elements.
118118
*/
119+
let commentnodeMap = new WeakMap(); // all nodes reference WeakMap for start and end range
119120
function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirst, callbacksLast, sortShapeProp) {
120121
function sortInSource(allNodes, source) {
121122
const originalSource = source;
123+
const sourceCode = context.getSourceCode();
124+
for (let i = 0; i < allNodes.length; i++) {
125+
const node = allNodes[i];
126+
let commentAfter = [];
127+
let commentBefore = [];
128+
let newStart = 0;
129+
let newEnd = 0;
130+
try {
131+
commentBefore = sourceCode.getCommentsBefore(node);
132+
commentAfter = sourceCode.getCommentsAfter(node);
133+
} catch (e) { /**/ };
134+
if (commentAfter.length === 0 || commentBefore.length === 0) {
135+
newStart = node.range[0]
136+
newEnd = node.range[1]
137+
}
138+
const firstCommentBefore = commentBefore[0];
139+
if (commentBefore.length >= 1) {
140+
newStart = firstCommentBefore.range[0]
141+
}
142+
const lastCommentAfter = commentAfter[commentAfter.length - 1];
143+
if (commentAfter.length >= 1) {
144+
newEnd = lastCommentAfter.range[1]
145+
}
146+
commentnodeMap.set(node, { start: newStart, end: newEnd, hasComment: true });
147+
};
122148
const nodeGroups = allNodes.reduce((acc, curr) => {
123149
if (curr.type === 'ExperimentalSpreadProperty' || curr.type === 'SpreadElement') {
124150
acc.push([]);
@@ -135,7 +161,8 @@ function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirs
135161

136162
source = nodes.reduceRight((acc, attr, index) => {
137163
const sortedAttr = sortedAttributes[index];
138-
let sortedAttrText = context.getSourceCode().getText(sortedAttr);
164+
let sourceCodeText = sourceCode.getText();
165+
let sortedAttrText = sourceCodeText.substring(commentnodeMap.get(sortedAttr).start, commentnodeMap.get(sortedAttr).end);
139166
if (sortShapeProp && isShapeProp(sortedAttr.value)) {
140167
const shape = getShapeProperties(sortedAttr.value);
141168
if (shape) {
@@ -146,16 +173,16 @@ function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirs
146173
sortedAttrText = attrSource.slice(sortedAttr.range[0], sortedAttr.range[1]);
147174
}
148175
}
149-
return `${acc.slice(0, attr.range[0])}${sortedAttrText}${acc.slice(attr.range[1])}`;
176+
return `${acc.slice(0, commentnodeMap.get(attr).start)}${sortedAttrText}${acc.slice(commentnodeMap.get(attr).end)}`;
150177
}, source);
151178
});
152179
return source;
153180
}
154181

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

157-
const rangeStart = declarations[0].range[0];
158-
const rangeEnd = declarations[declarations.length - 1].range[1];
184+
const rangeStart = commentnodeMap.get(declarations[0]).start;
185+
const rangeEnd = commentnodeMap.get(declarations[declarations.length - 1]).end;
159186
return fixer.replaceTextRange([rangeStart, rangeEnd], source.slice(rangeStart, rangeEnd));
160187
}
161188

0 commit comments

Comments
 (0)