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

Disable --fix for sort-prop-types #2505

Merged
merged 1 commit into from Nov 28, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions docs/rules/jsx-sort-default-props.md
Expand Up @@ -2,8 +2,6 @@

Some developers prefer to sort `defaultProps` declarations alphabetically to be able to find necessary declarations easier at a later time. Others feel that it adds complexity and becomes a burden to maintain.

**Fixable:** This rule is automatically fixable using the `--fix` flag on the command line.

## Rule Details

This rule checks all components and verifies that all `defaultProps` declarations are sorted alphabetically. A spread attribute resets the verification. The default configuration of the rule is case-sensitive.
Expand Down
3 changes: 0 additions & 3 deletions docs/rules/sort-prop-types.md
Expand Up @@ -2,9 +2,6 @@

Some developers prefer to sort propTypes declarations alphabetically to be able to find necessary declaration easier at the later time. Others feel that it adds complexity and becomes burden to maintain.

**Fixable:** This rule is automatically fixable using the `--fix` flag on the command line.


## Rule Details

This rule checks all components and verifies that all propTypes declarations are sorted alphabetically. A spread attribute resets the verification. The default configuration of the rule is case-sensitive.
Expand Down
14 changes: 7 additions & 7 deletions lib/rules/jsx-sort-default-props.js
Expand Up @@ -8,7 +8,7 @@
const variableUtil = require('../util/variable');
const docsUrl = require('../util/docsUrl');
const propWrapperUtil = require('../util/propWrapper');
const propTypesSortUtil = require('../util/propTypesSort');
// const propTypesSortUtil = require('../util/propTypesSort');

// ------------------------------------------------------------------------------
// Rule Definition
Expand All @@ -23,7 +23,7 @@ module.exports = {
url: docsUrl('jsx-sort-default-props')
},

fixable: 'code',
// fixable: 'code',

schema: [{
type: 'object',
Expand Down Expand Up @@ -100,9 +100,9 @@ module.exports = {
* @returns {void}
*/
function checkSorted(declarations) {
function fix(fixer) {
return propTypesSortUtil.fixPropTypesSort(fixer, context, declarations, ignoreCase);
}
// function fix(fixer) {
// return propTypesSortUtil.fixPropTypesSort(fixer, context, declarations, ignoreCase);
// }

declarations.reduce((prev, curr, idx, decls) => {
if (/Spread(?:Property|Element)$/.test(curr.type)) {
Expand All @@ -120,8 +120,8 @@ module.exports = {
if (currentPropName < prevPropName) {
context.report({
node: curr,
message: 'Default prop types declarations should be sorted alphabetically',
fix
message: 'Default prop types declarations should be sorted alphabetically'
// fix
});

return prev;
Expand Down
38 changes: 19 additions & 19 deletions lib/rules/sort-prop-types.js
Expand Up @@ -8,7 +8,7 @@ const variableUtil = require('../util/variable');
const propsUtil = require('../util/props');
const docsUrl = require('../util/docsUrl');
const propWrapperUtil = require('../util/propWrapper');
const propTypesSortUtil = require('../util/propTypesSort');
// const propTypesSortUtil = require('../util/propTypesSort');

// ------------------------------------------------------------------------------
// Rule Definition
Expand All @@ -23,7 +23,7 @@ module.exports = {
url: docsUrl('sort-prop-types')
},

fixable: 'code',
// fixable: 'code',

schema: [{
type: 'object',
Expand Down Expand Up @@ -98,17 +98,17 @@ module.exports = {
return;
}

function fix(fixer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we keep this as commented code, and do something like const fix = undefined; or something, so that all the rest of the code passing fix doesn't have to change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it might be easier to make this easy to revert to restore the missing code when desired. I tend to not like to keep large swaths of commented out code around, but I can do that if desired.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally i'd agree, but in this case i think it'd be preferable.

return propTypesSortUtil.fixPropTypesSort(
fixer,
context,
declarations,
ignoreCase,
requiredFirst,
callbacksLast,
sortShapeProp
);
}
// function fix(fixer) {
// return propTypesSortUtil.fixPropTypesSort(
// fixer,
// context,
// declarations,
// ignoreCase,
// requiredFirst,
// callbacksLast,
// sortShapeProp
// );
// }

declarations.reduce((prev, curr, idx, decls) => {
if (curr.type === 'ExperimentalSpreadProperty' || curr.type === 'SpreadElement') {
Expand Down Expand Up @@ -136,8 +136,8 @@ module.exports = {
// Encountered a non-required prop after a required prop
context.report({
node: curr,
message: 'Required prop types must be listed before all other prop types',
fix
message: 'Required prop types must be listed before all other prop types'
// fix
});
return curr;
}
Expand All @@ -152,8 +152,8 @@ module.exports = {
// Encountered a non-callback prop after a callback prop
context.report({
node: prev,
message: 'Callback prop types must be listed after all other prop types',
fix
message: 'Callback prop types must be listed after all other prop types'
// fix
});
return prev;
}
Expand All @@ -162,8 +162,8 @@ module.exports = {
if (!noSortAlphabetically && currentPropName < prevPropName) {
context.report({
node: curr,
message: 'Prop types declarations should be sorted alphabetically',
fix
message: 'Prop types declarations should be sorted alphabetically'
// fix
});
return prev;
}
Expand Down