Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Fix] jsx-sort-props: sorted attributes now respect comments #3358

Merged
merged 1 commit into from Aug 24, 2022
Merged

[Fix] jsx-sort-props: sorted attributes now respect comments #3358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [`display-name`], component detection: fix false positive for HOF returning only nulls and literals ([#3305][] @golopot)
* [`jsx-no-target-blank`]: False negative when rel attribute is assigned using ConditionalExpression ([#3332][] @V2dha)
* [`jsx-no-leaked-render`]: autofix nested "&&" logical expressions ([#3353][] @hduprat)
* [`jsx-sort-props`]: sorted attributes now respect comments ([#3358][] @ROSSROSALES)

### Changed
* [Refactor] [`jsx-indent-props`]: improved readability of the checkNodesIndent function ([#3315][] @caroline223)
Expand Down Expand Up @@ -50,6 +51,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
[#3362]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3362
[#3361]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3361
[#3359]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3359
[#3358]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3358
[#3355]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3355
[#3353]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3353
[#3351]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3351
Expand Down
122 changes: 93 additions & 29 deletions lib/rules/jsx-sort-props.js
View file
Open in desktop
Expand Up @@ -46,10 +46,27 @@ function isReservedPropName(name, list) {
return list.indexOf(name) >= 0;
}

let attributeMap;
// attributeMap = [endrange, true||false if comment in between nodes exists, it needs to be sorted to end]

function shouldSortToEnd(node) {
const attr = attributeMap.get(node);
return !!attr && !!attr[1];
}

function contextCompare(a, b, options) {
let aProp = propName(a);
let bProp = propName(b);

const aSortToEnd = shouldSortToEnd(a);
const bSortToEnd = shouldSortToEnd(b);
if (aSortToEnd && !bSortToEnd) {
return 1;
}
if (!aSortToEnd && bSortToEnd) {
return -1;
}

if (options.reservedFirst) {
const aIsReserved = isReservedPropName(aProp, options.reservedList);
const bIsReserved = isReservedPropName(bProp, options.reservedList);
Expand Down Expand Up @@ -118,32 +135,79 @@ function contextCompare(a, b, options) {
* Create an array of arrays where each subarray is composed of attributes
* that are considered sortable.
* @param {Array<JSXSpreadAttribute|JSXAttribute>} attributes
* @param {Object} context The context of the rule
* @return {Array<Array<JSXAttribute>>}
*/
function getGroupsOfSortableAttributes(attributes) {
function getGroupsOfSortableAttributes(attributes, context) {
const sourceCode = context.getSourceCode();

const sortableAttributeGroups = [];
let groupCount = 0;
function addtoSortableAttributeGroups(attribute) {
sortableAttributeGroups[groupCount - 1].push(attribute);
}

for (let i = 0; i < attributes.length; i++) {
const attribute = attributes[i];
const nextAttribute = attributes[i + 1];
const attributeline = attribute.loc.start.line;
let comment = [];
try {
comment = sourceCode.getCommentsAfter(attribute);
} catch (e) { /**/ }
const lastAttr = attributes[i - 1];
const attrIsSpread = attribute.type === 'JSXSpreadAttribute';

// If we have no groups or if the last attribute was JSXSpreadAttribute
// then we start a new group. Append attributes to the group until we
// come across another JSXSpreadAttribute or exhaust the array.
if (
!lastAttr
|| (lastAttr.type === 'JSXSpreadAttribute'
&& attributes[i].type !== 'JSXSpreadAttribute')
|| (lastAttr.type === 'JSXSpreadAttribute' && !attrIsSpread)
) {
groupCount += 1;
sortableAttributeGroups[groupCount - 1] = [];
}
if (attributes[i].type !== 'JSXSpreadAttribute') {
sortableAttributeGroups[groupCount - 1].push(attributes[i]);
if (!attrIsSpread) {
if (comment.length === 0) {
attributeMap.set(attribute, [attribute.range[1], false]);
addtoSortableAttributeGroups(attribute);
} else {
const firstComment = comment[0];
const commentline = firstComment.loc.start.line;
if (comment.length === 1) {
if (attributeline + 1 === commentline && nextAttribute) {
attributeMap.set(attribute, [nextAttribute.range[1], true]);
addtoSortableAttributeGroups(attribute);
i += 1;
} else if (attributeline === commentline) {
if (firstComment.type === 'Block') {
attributeMap.set(attribute, [nextAttribute.range[1], true]);
i += 1;
} else {
attributeMap.set(attribute, [firstComment.range[1], false]);
}
addtoSortableAttributeGroups(attribute);
}
} else if (comment.length > 1 && attributeline + 1 === comment[1].loc.start.line && nextAttribute) {
const commentNextAttribute = sourceCode.getCommentsAfter(nextAttribute);
attributeMap.set(attribute, [nextAttribute.range[1], true]);
if (
commentNextAttribute.length === 1
&& nextAttribute.loc.start.line === commentNextAttribute[0].loc.start.line
) {
attributeMap.set(attribute, [commentNextAttribute[0].range[1], true]);
}
addtoSortableAttributeGroups(attribute);
i += 1;
}
}
}
}
return sortableAttributeGroups;
}

const generateFixerFunction = (node, context, reservedList) => {
function generateFixerFunction(node, context, reservedList) {
const sourceCode = context.getSourceCode();
const attributes = node.attributes.slice(0);
const configuration = context.options[0] || {};
Expand All @@ -170,7 +234,7 @@ const generateFixerFunction = (node, context, reservedList) => {
reservedList,
locale,
};
const sortableAttributeGroups = getGroupsOfSortableAttributes(attributes);
const sortableAttributeGroups = getGroupsOfSortableAttributes(attributes, context);
const sortedAttributeGroups = sortableAttributeGroups
.slice(0)
.map((group) => group.slice(0).sort((a, b) => contextCompare(a, b, options)));
Expand All @@ -179,13 +243,13 @@ const generateFixerFunction = (node, context, reservedList) => {
const fixers = [];
let source = sourceCode.getText();

// Replace each unsorted attribute with the sorted one.
sortableAttributeGroups.forEach((sortableGroup, ii) => {
sortableGroup.forEach((attr, jj) => {
const sortedAttr = sortedAttributeGroups[ii][jj];
const sortedAttrText = sourceCode.getText(sortedAttr);
const sortedAttrText = source.substring(sortedAttr.range[0], attributeMap.get(sortedAttr)[0]);
const attrrangeEnd = attributeMap.get(attr)[0];
fixers.push({
range: [attr.range[0], attr.range[1]],
range: [attr.range[0], attrrangeEnd],
text: sortedAttrText,
});
});
Expand All @@ -202,7 +266,7 @@ const generateFixerFunction = (node, context, reservedList) => {

return fixer.replaceTextRange([rangeStart, rangeEnd], source.substr(rangeStart, rangeEnd - rangeStart));
};
};
}

/**
* Checks if the `reservedFirst` option is valid
Expand Down Expand Up @@ -331,15 +395,17 @@ module.exports = {
const noSortAlphabetically = configuration.noSortAlphabetically || false;
const reservedFirst = configuration.reservedFirst || false;
const reservedFirstError = validateReservedFirstConfig(context, reservedFirst);
let reservedList = Array.isArray(reservedFirst) ? reservedFirst : RESERVED_PROPS_LIST;
const reservedList = Array.isArray(reservedFirst) ? reservedFirst : RESERVED_PROPS_LIST;
const locale = configuration.locale || 'auto';

return {
Program() {
attributeMap = new WeakMap();
},

JSXOpeningElement(node) {
// `dangerouslySetInnerHTML` is only "reserved" on DOM components
if (reservedFirst && !jsxUtil.isDOMComponent(node)) {
reservedList = reservedList.filter((prop) => prop !== 'dangerouslySetInnerHTML');
}
const nodeReservedList = reservedFirst && !jsxUtil.isDOMComponent(node) ? reservedList.filter((prop) => prop !== 'dangerouslySetInnerHTML') : reservedList;

node.attributes.reduce((memo, decl, idx, attrs) => {
if (decl.type === 'JSXSpreadAttribute') {
Expand All @@ -352,8 +418,6 @@ module.exports = {
const currentValue = decl.value;
const previousIsCallback = isCallbackPropName(previousPropName);
const currentIsCallback = isCallbackPropName(currentPropName);
const previousIsMultiline = isMultilineProp(memo);
const currentIsMultiline = isMultilineProp(decl);

if (ignoreCase) {
previousPropName = previousPropName.toLowerCase();
Expand All @@ -366,14 +430,14 @@ module.exports = {
return memo;
}

const previousIsReserved = isReservedPropName(previousPropName, reservedList);
const currentIsReserved = isReservedPropName(currentPropName, reservedList);
const previousIsReserved = isReservedPropName(previousPropName, nodeReservedList);
const currentIsReserved = isReservedPropName(currentPropName, nodeReservedList);

if (previousIsReserved && !currentIsReserved) {
return decl;
}
if (!previousIsReserved && currentIsReserved) {
reportNodeAttribute(decl, 'listReservedPropsFirst', node, context, reservedList);
reportNodeAttribute(decl, 'listReservedPropsFirst', node, context, nodeReservedList);

return memo;
}
Expand All @@ -386,7 +450,7 @@ module.exports = {
}
if (previousIsCallback && !currentIsCallback) {
// Encountered a non-callback prop after a callback prop
reportNodeAttribute(memo, 'listCallbacksLast', node, context, reservedList);
reportNodeAttribute(memo, 'listCallbacksLast', node, context, nodeReservedList);

return memo;
}
Expand All @@ -397,7 +461,7 @@ module.exports = {
return decl;
}
if (!currentValue && previousValue) {
reportNodeAttribute(decl, 'listShorthandFirst', node, context, reservedList);
reportNodeAttribute(decl, 'listShorthandFirst', node, context, nodeReservedList);

return memo;
}
Expand All @@ -408,33 +472,33 @@ module.exports = {
return decl;
}
if (currentValue && !previousValue) {
reportNodeAttribute(memo, 'listShorthandLast', node, context, reservedList);
reportNodeAttribute(memo, 'listShorthandLast', node, context, nodeReservedList);

return memo;
}
}

const previousIsMultiline = isMultilineProp(memo);
const currentIsMultiline = isMultilineProp(decl);
if (multiline === 'first') {
if (previousIsMultiline && !currentIsMultiline) {
// Exiting the multiline prop section
return decl;
}
if (!previousIsMultiline && currentIsMultiline) {
// Encountered a non-multiline prop before a multiline prop
reportNodeAttribute(decl, 'listMultilineFirst', node, context, reservedList);
reportNodeAttribute(decl, 'listMultilineFirst', node, context, nodeReservedList);

return memo;
}
}

if (multiline === 'last') {
} else if (multiline === 'last') {
if (!previousIsMultiline && currentIsMultiline) {
// Entering the multiline prop section
return decl;
}
if (previousIsMultiline && !currentIsMultiline) {
// Encountered a non-multiline prop after a multiline prop
reportNodeAttribute(memo, 'listMultilineLast', node, context, reservedList);
reportNodeAttribute(memo, 'listMultilineLast', node, context, nodeReservedList);

return memo;
}
Expand All @@ -448,7 +512,7 @@ module.exports = {
: previousPropName > currentPropName
)
) {
reportNodeAttribute(decl, 'sortPropsByAlpha', node, context, reservedList);
reportNodeAttribute(decl, 'sortPropsByAlpha', node, context, nodeReservedList);

return memo;
}
Expand Down