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.

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

Make all jsx-prop-sort lints fixable #2250

Merged
merged 2 commits into from
Apr 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 51 additions & 16 deletions lib/rules/jsx-sort-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,48 @@ function isReservedPropName(name, list) {
return list.indexOf(name) >= 0;
}

function propNameCompare(a, b, options) {
if (options.ignoreCase) {
a = a.toLowerCase();
b = b.toLowerCase();
}
function contextCompare(a, b, options) {
let aProp = propName(a);
let bProp = propName(b);

if (options.reservedFirst) {
const aIsReserved = isReservedPropName(a, options.reservedList);
const bIsReserved = isReservedPropName(b, options.reservedList);
if ((aIsReserved && bIsReserved) || (!aIsReserved && !bIsReserved)) {
return a.localeCompare(b);
} else if (aIsReserved && !bIsReserved) {
const aIsReserved = isReservedPropName(aProp, options.reservedList);
const bIsReserved = isReservedPropName(bProp, options.reservedList);
if (aIsReserved && !bIsReserved) {
return -1;
} else if (!aIsReserved && bIsReserved) {
return 1;
guliashvili marked this conversation as resolved.
Show resolved Hide resolved
}
}

if (options.callbacksLast) {
const aIsCallback = isCallbackPropName(aProp);
const bIsCallback = isCallbackPropName(bProp);
if (aIsCallback && !bIsCallback) {
return 1;
} else if (!aIsCallback && bIsCallback) {
return -1;
guliashvili marked this conversation as resolved.
Show resolved Hide resolved
}
}

if (options.shorthandFirst || options.shorthandLast) {
const shorthandSign = options.shorthandFirst ? -1 : 1;
if (!a.value && b.value) {
return shorthandSign;
} else if (a.value && !b.value) {
return -shorthandSign;
}
return 1;
}
return a.localeCompare(b);

if (options.noSortAlphabetically) {
return 0;
}

if (options.ignoreCase) {
aProp = aProp.toLowerCase();
bProp = bProp.toLowerCase();
}
return aProp.localeCompare(bProp);
}

/**
Expand Down Expand Up @@ -79,15 +105,21 @@ const generateFixerFunction = (node, context, reservedList) => {
const attributes = node.attributes.slice(0);
const configuration = context.options[0] || {};
const ignoreCase = configuration.ignoreCase || false;
const callbacksLast = configuration.callbacksLast || false;
const shorthandFirst = configuration.shorthandFirst || false;
const shorthandLast = configuration.shorthandLast || false;
const noSortAlphabetically = configuration.noSortAlphabetically || false;
const reservedFirst = configuration.reservedFirst || false;

// Sort props according to the context. Only supports ignoreCase.
// Since we cannot safely move JSXSpreadAttribute (due to potential variable overrides),
// we only consider groups of sortable attributes.
const options = {ignoreCase, callbacksLast, shorthandFirst, shorthandLast,
noSortAlphabetically, reservedFirst, reservedList};
const sortableAttributeGroups = getGroupsOfSortableAttributes(attributes);
const sortedAttributeGroups = sortableAttributeGroups.slice(0).map(group =>
group.slice(0).sort((a, b) =>
propNameCompare(propName(a), propName(b), {ignoreCase, reservedFirst, reservedList})
contextCompare(a, b, options)
)
);

Expand Down Expand Up @@ -267,7 +299,8 @@ module.exports = {
// Encountered a non-callback prop after a callback prop
context.report({
node: memo,
message: 'Callbacks must be listed after all other props'
message: 'Callbacks must be listed after all other props',
fix: generateFixerFunction(node, context, reservedList)
});
return memo;
}
Expand All @@ -280,7 +313,8 @@ module.exports = {
if (!currentValue && previousValue) {
context.report({
node: memo,
message: 'Shorthand props must be listed before all other props'
message: 'Shorthand props must be listed before all other props',
fix: generateFixerFunction(node, context, reservedList)
});
return memo;
}
Expand All @@ -293,7 +327,8 @@ module.exports = {
if (currentValue && !previousValue) {
context.report({
node: memo,
message: 'Shorthand props must be listed after all other props'
message: 'Shorthand props must be listed after all other props',
fix: generateFixerFunction(node, context, reservedList)
});
return memo;
}
Expand Down
46 changes: 32 additions & 14 deletions tests/lib/rules/jsx-sort-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -336,59 +336,75 @@ ruleTester.run('jsx-sort-props', rule, {
{
code: '<App key="key" b c="c" />',
errors: [expectedShorthandLastError],
options: reservedFirstWithShorthandLast
options: reservedFirstWithShorthandLast,
output: '<App key="key" c="c" b />'
},
{
code: '<App ref="ref" key="key" isShorthand veryLastAttribute="yes" />',
errors: [expectedError, expectedShorthandLastError],
options: reservedFirstWithShorthandLast
options: reservedFirstWithShorthandLast,
output: '<App key="key" ref="ref" veryLastAttribute="yes" isShorthand />'
},
{
code: '<App a z onFoo onBar />;',
errors: [expectedError],
options: callbacksLastArgs
options: callbacksLastArgs,
output: '<App a z onBar onFoo />;'
},
{
code: '<App a onBar onFoo z />;',
errors: [expectedCallbackError],
options: callbacksLastArgs
options: callbacksLastArgs,
output: '<App a z onBar onFoo />;'
},
{
code: '<App a="a" b />;',
errors: [expectedShorthandFirstError],
options: shorthandFirstArgs
options: shorthandFirstArgs,
output: '<App b a="a" />;'
},
{
code: '<App z x a="a" />;',
errors: [expectedError],
options: shorthandFirstArgs
options: shorthandFirstArgs,
output: '<App x z a="a" />;'
},
{
code: '<App b a="a" />;',
errors: [expectedShorthandLastError],
options: shorthandLastArgs
options: shorthandLastArgs,
output: '<App a="a" b />;'
},
{
code: '<App a="a" onBar onFoo z x />;',
errors: [shorthandAndCallbackLastArgs],
options: shorthandLastArgs
options: shorthandLastArgs,
output: '<App a="a" onBar onFoo x z />;'
},
{
code: '<App b a />;',
errors: [expectedError],
options: sortAlphabeticallyArgs,
output: '<App a b />;'
},
{code: '<App b a />;', errors: [expectedError], options: sortAlphabeticallyArgs},
// reservedFirst
{
code: '<App a key={1} />',
options: reservedFirstAsBooleanArgs,
errors: [expectedReservedFirstError]
errors: [expectedReservedFirstError],
output: '<App key={1} a />'
},
{
code: '<div a dangerouslySetInnerHTML={{__html: "EPR"}} />',
options: reservedFirstAsBooleanArgs,
errors: [expectedReservedFirstError]
errors: [expectedReservedFirstError],
output: '<div dangerouslySetInnerHTML={{__html: "EPR"}} a />'
},
{
code: '<App ref="r" key={2} b />',
options: reservedFirstAsBooleanArgs,
errors: [expectedError]
errors: [expectedError],
output: '<App key={2} ref="r" b />'
},
{
code: '<App key={2} b a />',
Expand All @@ -411,12 +427,14 @@ ruleTester.run('jsx-sort-props', rule, {
{
code: '<App key={3} children={<App />} />',
options: reservedFirstAsArrayArgs,
errors: [expectedError]
errors: [expectedError],
output: '<App children={<App />} key={3} />'
},
{
code: '<App z ref="r" />',
options: reservedFirstWithNoSortAlphabeticallyArgs,
errors: [expectedReservedFirstError]
errors: [expectedReservedFirstError],
output: '<App ref="r" z />'
},
{
code: '<App key={4} />',
Expand Down