Skip to content

Commit f9c22ef

Browse files
ljharbljharb
authored
Merge pull request #1273 from Overload119/master
Add fixer for jsx/sort-props
2 parents e305279 + d66808b commit f9c22ef

File tree

3 files changed

+153
-13
lines changed

3 files changed

+153
-13
lines changed

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
142142
* [react/jsx-no-target-blank](docs/rules/jsx-no-target-blank.md): Prevent usage of unsafe `target='_blank'`
143143
* [react/jsx-no-undef](docs/rules/jsx-no-undef.md): Disallow undeclared variables in JSX
144144
* [react/jsx-pascal-case](docs/rules/jsx-pascal-case.md): Enforce PascalCase for user-defined JSX components
145-
* [react/jsx-sort-props](docs/rules/jsx-sort-props.md): Enforce props alphabetical sorting
145+
* [react/jsx-sort-props](docs/rules/jsx-sort-props.md): Enforce props alphabetical sorting (fixable)
146146
* [react/jsx-space-before-closing](docs/rules/jsx-space-before-closing.md): Validate spacing before closing bracket in JSX (fixable)
147147
* [react/jsx-tag-spacing](docs/rules/jsx-tag-spacing.md): Validate whitespace in and around the JSX opening and closing brackets (fixable)
148148
* [react/jsx-uses-react](docs/rules/jsx-uses-react.md): Prevent React to be incorrectly marked as unused

‎lib/rules/jsx-sort-props.js

+76-3
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,77 @@ function isReservedPropName(name, list) {
4040
return list.indexOf(name) >= 0;
4141
}
4242

43+
function alphabeticalCompare(a, b, ignoreCase) {
44+
if (ignoreCase) {
45+
a = a.toLowerCase();
46+
b = b.toLowerCase();
47+
}
48+
return a.localeCompare(b);
49+
}
50+
51+
/**
52+
* Create an array of arrays where each subarray is composed of attributes
53+
* that are considered sortable.
54+
* @param {Array<JSXSpreadAttribute|JSXAttribute>} attributes
55+
* @return {Array<Array<JSXAttribute>}
56+
*/
57+
function getGroupsOfSortableAttributes(attributes) {
58+
const sortableAttributeGroups = [];
59+
let groupCount = 0;
60+
for (let i = 0; i < attributes.length; i++) {
61+
const lastAttr = attributes[i - 1];
62+
// If we have no groups or if the last attribute was JSXSpreadAttribute
63+
// then we start a new group. Append attributes to the group until we
64+
// come across another JSXSpreadAttribute or exhaust the array.
65+
if (
66+
!lastAttr ||
67+
(lastAttr.type === 'JSXSpreadAttribute' &&
68+
attributes[i].type !== 'JSXSpreadAttribute')
69+
) {
70+
groupCount++;
71+
sortableAttributeGroups[groupCount - 1] = [];
72+
}
73+
if (attributes[i].type !== 'JSXSpreadAttribute') {
74+
sortableAttributeGroups[groupCount - 1].push(attributes[i]);
75+
}
76+
}
77+
return sortableAttributeGroups;
78+
}
79+
80+
const generateFixerFunction = (node, context) => {
81+
const sourceCode = context.getSourceCode();
82+
const attributes = node.attributes.slice(0);
83+
const configuration = context.options[0] || {};
84+
const ignoreCase = configuration.ignoreCase || false;
85+
86+
// Sort props according to the context. Only supports ignoreCase.
87+
// Since we cannot safely move JSXSpreadAttribute (due to potential variable overrides),
88+
// we only consider groups of sortable attributes.
89+
const sortableAttributeGroups = getGroupsOfSortableAttributes(attributes);
90+
const sortedAttributeGroups = sortableAttributeGroups.slice(0).map(group =>
91+
group.slice(0).sort((a, b) =>
92+
alphabeticalCompare(propName(a), propName(b), ignoreCase)
93+
)
94+
);
95+
96+
return function(fixer) {
97+
const fixers = [];
98+
99+
// Replace each unsorted attribute with the sorted one.
100+
sortableAttributeGroups.forEach((sortableGroup, ii) => {
101+
sortableGroup.forEach((attr, jj) => {
102+
const sortedAttr = sortedAttributeGroups[ii][jj];
103+
const sortedAttrText = sourceCode.getText(sortedAttr);
104+
fixers.push(
105+
fixer.replaceTextRange([attr.start, attr.end], sortedAttrText)
106+
);
107+
});
108+
});
109+
110+
return fixers;
111+
};
112+
};
113+
43114
/**
44115
* Checks if the `reservedFirst` option is valid
45116
* @param {Object} context The context of the rule
@@ -88,7 +159,7 @@ module.exports = {
88159
category: 'Stylistic Issues',
89160
recommended: false
90161
},
91-
162+
fixable: 'code',
92163
schema: [{
93164
type: 'object',
94165
properties: {
@@ -168,7 +239,8 @@ module.exports = {
168239
if (!noSortAlphabetically && currentPropName < previousPropName) {
169240
context.report({
170241
node: decl,
171-
message: 'Props should be sorted alphabetically'
242+
message: 'Props should be sorted alphabetically',
243+
fix: generateFixerFunction(node, context)
172244
});
173245
return memo;
174246
}
@@ -228,7 +300,8 @@ module.exports = {
228300
if (!noSortAlphabetically && currentPropName < previousPropName) {
229301
context.report({
230302
node: decl,
231-
message: 'Props should be sorted alphabetically'
303+
message: 'Props should be sorted alphabetically',
304+
fix: generateFixerFunction(node, context)
232305
});
233306
return memo;
234307
}

‎tests/lib/rules/jsx-sort-props.js

+76-9
Original file line numberDiff line numberDiff line change
@@ -153,15 +153,82 @@ ruleTester.run('jsx-sort-props', rule, {
153153
}
154154
],
155155
invalid: [
156-
{code: '<App b a />;', errors: [expectedError]},
157-
{code: '<App {...this.props} b a />;', errors: [expectedError]},
158-
{code: '<App c {...this.props} b a />;', errors: [expectedError]},
159-
{code: '<App a A />;', errors: [expectedError]},
160-
{code: '<App B a />;', options: ignoreCaseArgs, errors: [expectedError]},
161-
{code: '<App B A c />;', options: ignoreCaseArgs, errors: [expectedError]},
162-
{code: '<App c="a" a="c" b="b" />;', errors: 2},
163-
{code: '<App {...this.props} c="a" a="c" b="b" />;', errors: 2},
164-
{code: '<App d="d" b="b" {...this.props} c="a" a="c" />;', errors: 2},
156+
{
157+
code: '<App b a />;',
158+
errors: [expectedError],
159+
output: '<App a b />;'
160+
},
161+
{
162+
code: '<App {...this.props} b a />;',
163+
errors: [expectedError],
164+
output: '<App {...this.props} a b />;'
165+
},
166+
{
167+
code: '<App c {...this.props} b a />;',
168+
errors: [expectedError],
169+
output: '<App c {...this.props} a b />;'
170+
},
171+
{
172+
code: '<App a A />;',
173+
errors: [expectedError],
174+
output: '<App a A />;'
175+
},
176+
{
177+
code: '<App B a />;',
178+
options: ignoreCaseArgs,
179+
errors: [expectedError],
180+
output: '<App a B />;'
181+
},
182+
{
183+
code: '<App B A c />;',
184+
options: ignoreCaseArgs,
185+
errors: [expectedError],
186+
output: '<App A B c />;'
187+
},
188+
{
189+
code: '<App c="a" a="c" b="b" />;',
190+
output: '<App a="c" b="b" c="a" />;',
191+
errors: 2
192+
},
193+
{
194+
code: '<App {...this.props} c="a" a="c" b="b" />;',
195+
output: '<App {...this.props} a="c" b="b" c="a" />;',
196+
errors: 2
197+
},
198+
{
199+
code: '<App d="d" b="b" {...this.props} c="a" a="c" />;',
200+
output: '<App b="b" d="d" {...this.props} a="c" c="a" />;',
201+
errors: 2
202+
},
203+
{
204+
code: [
205+
'<App',
206+
'a={true}',
207+
'z',
208+
'r',
209+
'_onClick={function(){}}',
210+
'onHandle={function(){}}',
211+
'{...this.props}',
212+
'b={false}',
213+
'{...otherProps}>',
214+
' {test}',
215+
'</App>'
216+
].join('\n'),
217+
output: [
218+
'<App',
219+
'_onClick={function(){}}',
220+
'a={true}',
221+
'onHandle={function(){}}',
222+
'r',
223+
'z',
224+
'{...this.props}',
225+
'b={false}',
226+
'{...otherProps}>',
227+
' {test}',
228+
'</App>'
229+
].join('\n'),
230+
errors: 3
231+
},
165232
{
166233
code: '<App a z onFoo onBar />;',
167234
errors: [expectedError],

0 commit comments

Comments
 (0)