Skip to content

Commit 8e5e945

Browse files
ROSSROSALESljharb
ROSSROSALES
authored and
ljharb
committed
jsx-sort-props: Issue#2366 sorted attributes respects comments
1 parent 8306a7b commit 8e5e945

File tree

2 files changed

+321
-29
lines changed

2 files changed

+321
-29
lines changed

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

+95-29
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,14 @@ function isReservedPropName(name, list) {
4646
return list.indexOf(name) >= 0;
4747
}
4848

49+
let attributeMap;
50+
// attributeMap = [endrange, true||false if comment in between nodes exists, it needs to be sorted to end]
51+
52+
function shouldSortToEnd(node) {
53+
const attr = attributeMap.get(node);
54+
return !!attr && !!attr[1];
55+
}
56+
4957
function contextCompare(a, b, options) {
5058
let aProp = propName(a);
5159
let bProp = propName(b);
@@ -98,6 +106,17 @@ function contextCompare(a, b, options) {
98106
return 0;
99107
}
100108

109+
if (options.commentbetween) {
110+
const aSortToEnd = shouldSortToEnd(a);
111+
const bSortToEnd = shouldSortToEnd(b);
112+
if (aSortToEnd && !bSortToEnd) {
113+
return 1;
114+
}
115+
if (!aSortToEnd && bSortToEnd) {
116+
return -1;
117+
}
118+
}
119+
101120
const actualLocale = options.locale === 'auto' ? undefined : options.locale;
102121

103122
if (options.ignoreCase) {
@@ -118,32 +137,77 @@ function contextCompare(a, b, options) {
118137
* Create an array of arrays where each subarray is composed of attributes
119138
* that are considered sortable.
120139
* @param {Array<JSXSpreadAttribute|JSXAttribute>} attributes
140+
* @param {Object} context The context of the rule
121141
* @return {Array<Array<JSXAttribute>>}
122142
*/
123-
function getGroupsOfSortableAttributes(attributes) {
143+
function getGroupsOfSortableAttributes(attributes, context) {
144+
const sourceCode = context.getSourceCode();
145+
124146
const sortableAttributeGroups = [];
125147
let groupCount = 0;
148+
function addtoSortableAttributeGroups(attribute) {
149+
sortableAttributeGroups[groupCount - 1].push(attribute);
150+
}
151+
126152
for (let i = 0; i < attributes.length; i++) {
153+
const attribute = attributes[i];
154+
const nextAttribute = attributes[i + 1];
155+
const attributeline = attribute.loc.start.line;
156+
const comment = sourceCode.getCommentsAfter(attribute);
127157
const lastAttr = attributes[i - 1];
158+
159+
const attrIsSpread = attribute.type === 'JSXSpreadAttribute';
160+
128161
// If we have no groups or if the last attribute was JSXSpreadAttribute
129162
// then we start a new group. Append attributes to the group until we
130163
// come across another JSXSpreadAttribute or exhaust the array.
131164
if (
132165
!lastAttr
133-
|| (lastAttr.type === 'JSXSpreadAttribute'
134-
&& attributes[i].type !== 'JSXSpreadAttribute')
166+
|| (lastAttr.type === 'JSXSpreadAttribute' && !attrIsSpread)
135167
) {
136168
groupCount += 1;
137169
sortableAttributeGroups[groupCount - 1] = [];
138170
}
139-
if (attributes[i].type !== 'JSXSpreadAttribute') {
140-
sortableAttributeGroups[groupCount - 1].push(attributes[i]);
171+
if (!attrIsSpread) {
172+
if (comment.length === 0) {
173+
attributeMap.set(attribute, [attribute.range[1], false]);
174+
addtoSortableAttributeGroups(attribute);
175+
} else {
176+
const firstComment = comment[0];
177+
const commentline = firstComment.loc.start.line;
178+
if (comment.length === 1) {
179+
if (attributeline + 1 === commentline && nextAttribute) {
180+
attributeMap.set(attribute, [nextAttribute.range[1], true]);
181+
addtoSortableAttributeGroups(attribute);
182+
i++;
183+
} else if (attributeline === commentline) {
184+
if (firstComment.type === 'Block') {
185+
attributeMap.set(attribute, [nextAttribute.range[1], true]);
186+
i++;
187+
} else {
188+
attributeMap.set(attribute, [firstComment.range[1], false]);
189+
}
190+
addtoSortableAttributeGroups(attribute);
191+
}
192+
} else if (comment.length > 1 && attributeline + 1 === comment[1].loc.start.line && nextAttribute) {
193+
const commentNextAttribute = sourceCode.getCommentsAfter(nextAttribute);
194+
attributeMap.set(attribute, [nextAttribute.range[1], true]);
195+
if (
196+
commentNextAttribute.length === 1
197+
&& nextAttribute.loc.start.line === commentNextAttribute[0].loc.start.line
198+
) {
199+
attributeMap.set(attribute, [commentNextAttribute[0].range[1], true]);
200+
}
201+
addtoSortableAttributeGroups(attribute);
202+
i++;
203+
}
204+
}
141205
}
142206
}
143207
return sortableAttributeGroups;
144208
}
145209

146-
const generateFixerFunction = (node, context, reservedList) => {
210+
function generateFixerFunction(node, context, reservedList) {
147211
const sourceCode = context.getSourceCode();
148212
const attributes = node.attributes.slice(0);
149213
const configuration = context.options[0] || {};
@@ -155,6 +219,7 @@ const generateFixerFunction = (node, context, reservedList) => {
155219
const noSortAlphabetically = configuration.noSortAlphabetically || false;
156220
const reservedFirst = configuration.reservedFirst || false;
157221
const locale = configuration.locale || 'auto';
222+
const commentbetween = configuration.commentbetween || true;
158223

159224
// Sort props according to the context. Only supports ignoreCase.
160225
// Since we cannot safely move JSXSpreadAttribute (due to potential variable overrides),
@@ -169,8 +234,9 @@ const generateFixerFunction = (node, context, reservedList) => {
169234
reservedFirst,
170235
reservedList,
171236
locale,
237+
commentbetween,
172238
};
173-
const sortableAttributeGroups = getGroupsOfSortableAttributes(attributes);
239+
const sortableAttributeGroups = getGroupsOfSortableAttributes(attributes, context);
174240
const sortedAttributeGroups = sortableAttributeGroups
175241
.slice(0)
176242
.map((group) => group.slice(0).sort((a, b) => contextCompare(a, b, options)));
@@ -179,13 +245,13 @@ const generateFixerFunction = (node, context, reservedList) => {
179245
const fixers = [];
180246
let source = sourceCode.getText();
181247

182-
// Replace each unsorted attribute with the sorted one.
183248
sortableAttributeGroups.forEach((sortableGroup, ii) => {
184249
sortableGroup.forEach((attr, jj) => {
185250
const sortedAttr = sortedAttributeGroups[ii][jj];
186-
const sortedAttrText = sourceCode.getText(sortedAttr);
251+
const sortedAttrText = source.substring(sortedAttr.range[0], attributeMap.get(sortedAttr)[0]);
252+
const attrrangeEnd = attributeMap.get(attr)[0];
187253
fixers.push({
188-
range: [attr.range[0], attr.range[1]],
254+
range: [attr.range[0], attrrangeEnd],
189255
text: sortedAttrText,
190256
});
191257
});
@@ -202,7 +268,7 @@ const generateFixerFunction = (node, context, reservedList) => {
202268

203269
return fixer.replaceTextRange([rangeStart, rangeEnd], source.substr(rangeStart, rangeEnd - rangeStart));
204270
};
205-
};
271+
}
206272

207273
/**
208274
* Checks if the `reservedFirst` option is valid
@@ -331,15 +397,17 @@ module.exports = {
331397
const noSortAlphabetically = configuration.noSortAlphabetically || false;
332398
const reservedFirst = configuration.reservedFirst || false;
333399
const reservedFirstError = validateReservedFirstConfig(context, reservedFirst);
334-
let reservedList = Array.isArray(reservedFirst) ? reservedFirst : RESERVED_PROPS_LIST;
400+
const reservedList = Array.isArray(reservedFirst) ? reservedFirst : RESERVED_PROPS_LIST;
335401
const locale = configuration.locale || 'auto';
336402

337403
return {
404+
Program() {
405+
attributeMap = new WeakMap();
406+
},
407+
338408
JSXOpeningElement(node) {
339409
// `dangerouslySetInnerHTML` is only "reserved" on DOM components
340-
if (reservedFirst && !jsxUtil.isDOMComponent(node)) {
341-
reservedList = reservedList.filter((prop) => prop !== 'dangerouslySetInnerHTML');
342-
}
410+
const nodeReservedList = reservedFirst && !jsxUtil.isDOMComponent(node) ? reservedList.filter((prop) => prop !== 'dangerouslySetInnerHTML') : reservedList;
343411

344412
node.attributes.reduce((memo, decl, idx, attrs) => {
345413
if (decl.type === 'JSXSpreadAttribute') {
@@ -352,8 +420,6 @@ module.exports = {
352420
const currentValue = decl.value;
353421
const previousIsCallback = isCallbackPropName(previousPropName);
354422
const currentIsCallback = isCallbackPropName(currentPropName);
355-
const previousIsMultiline = isMultilineProp(memo);
356-
const currentIsMultiline = isMultilineProp(decl);
357423

358424
if (ignoreCase) {
359425
previousPropName = previousPropName.toLowerCase();
@@ -366,14 +432,14 @@ module.exports = {
366432
return memo;
367433
}
368434

369-
const previousIsReserved = isReservedPropName(previousPropName, reservedList);
370-
const currentIsReserved = isReservedPropName(currentPropName, reservedList);
435+
const previousIsReserved = isReservedPropName(previousPropName, nodeReservedList);
436+
const currentIsReserved = isReservedPropName(currentPropName, nodeReservedList);
371437

372438
if (previousIsReserved && !currentIsReserved) {
373439
return decl;
374440
}
375441
if (!previousIsReserved && currentIsReserved) {
376-
reportNodeAttribute(decl, 'listReservedPropsFirst', node, context, reservedList);
442+
reportNodeAttribute(decl, 'listReservedPropsFirst', node, context, nodeReservedList);
377443

378444
return memo;
379445
}
@@ -386,7 +452,7 @@ module.exports = {
386452
}
387453
if (previousIsCallback && !currentIsCallback) {
388454
// Encountered a non-callback prop after a callback prop
389-
reportNodeAttribute(memo, 'listCallbacksLast', node, context, reservedList);
455+
reportNodeAttribute(memo, 'listCallbacksLast', node, context, nodeReservedList);
390456

391457
return memo;
392458
}
@@ -397,7 +463,7 @@ module.exports = {
397463
return decl;
398464
}
399465
if (!currentValue && previousValue) {
400-
reportNodeAttribute(decl, 'listShorthandFirst', node, context, reservedList);
466+
reportNodeAttribute(decl, 'listShorthandFirst', node, context, nodeReservedList);
401467

402468
return memo;
403469
}
@@ -408,33 +474,33 @@ module.exports = {
408474
return decl;
409475
}
410476
if (currentValue && !previousValue) {
411-
reportNodeAttribute(memo, 'listShorthandLast', node, context, reservedList);
477+
reportNodeAttribute(memo, 'listShorthandLast', node, context, nodeReservedList);
412478

413479
return memo;
414480
}
415481
}
416482

483+
const previousIsMultiline = isMultilineProp(memo);
484+
const currentIsMultiline = isMultilineProp(decl);
417485
if (multiline === 'first') {
418486
if (previousIsMultiline && !currentIsMultiline) {
419487
// Exiting the multiline prop section
420488
return decl;
421489
}
422490
if (!previousIsMultiline && currentIsMultiline) {
423491
// Encountered a non-multiline prop before a multiline prop
424-
reportNodeAttribute(decl, 'listMultilineFirst', node, context, reservedList);
492+
reportNodeAttribute(decl, 'listMultilineFirst', node, context, nodeReservedList);
425493

426494
return memo;
427495
}
428-
}
429-
430-
if (multiline === 'last') {
496+
} else if (multiline === 'last') {
431497
if (!previousIsMultiline && currentIsMultiline) {
432498
// Entering the multiline prop section
433499
return decl;
434500
}
435501
if (previousIsMultiline && !currentIsMultiline) {
436502
// Encountered a non-multiline prop after a multiline prop
437-
reportNodeAttribute(memo, 'listMultilineLast', node, context, reservedList);
503+
reportNodeAttribute(memo, 'listMultilineLast', node, context, nodeReservedList);
438504

439505
return memo;
440506
}
@@ -448,7 +514,7 @@ module.exports = {
448514
: previousPropName > currentPropName
449515
)
450516
) {
451-
reportNodeAttribute(decl, 'sortPropsByAlpha', node, context, reservedList);
517+
reportNodeAttribute(decl, 'sortPropsByAlpha', node, context, nodeReservedList);
452518

453519
return memo;
454520
}

0 commit comments

Comments
 (0)