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 1 commit
Commits
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
69 changes: 55 additions & 14 deletions lib/rules/jsx-sort-props.js
Expand Up @@ -27,22 +27,54 @@ 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);
const aIsReserved = isReservedPropName(aProp, options.reservedList);
const bIsReserved = isReservedPropName(bProp, options.reservedList);
if ((aIsReserved && bIsReserved) || (!aIsReserved && !bIsReserved)) {
return a.localeCompare(b);
// pass
guliashvili marked this conversation as resolved.
Show resolved Hide resolved
} else if (aIsReserved && !bIsReserved) {
return -1;
} else {
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) || (!aIsCallback && !bIsCallback)) {
// pass
guliashvili marked this conversation as resolved.
Show resolved Hide resolved
} else if (aIsCallback && !bIsCallback) {
return 1;
} else {
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) {
// pass
guliashvili marked this conversation as resolved.
Show resolved Hide resolved
} else if (!a.value) {
return shorthandSign;
} else {
guliashvili marked this conversation as resolved.
Show resolved Hide resolved
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 +111,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 +305,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 +319,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 +333,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
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 ref="ref" key="key" 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