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

Add --fix-type support #3165

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

sryze
Copy link

@sryze sryze commented Jan 4, 2022

Add type property to all rules to support usage of eslint's --fix-type option

Fixes #2618. Fixes #2039.

Successor of this PR: #2619

@sryze
Copy link
Author

sryze commented Jan 4, 2022

Rebased

docs: {
description: 'Validate closing bracket location in JSX',
category: 'Stylistic Issues',
recommended: false,
url: docsUrl('jsx-closing-bracket-location'),
},
fixable: 'code',
fixable: 'whitespace',
Copy link
Member

Choose a reason for hiding this comment

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

why is this change correct?

(if it is correct, i'd like to land it separately, before this PR)

Copy link
Author

@sryze sryze Jan 4, 2022

Choose a reason for hiding this comment

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

it was part of the original PR

I'm not 100% sure what this property means because the documentation doesn't mention what is the difference between code and whitespace; possibly whitepsace means that a fix will only change whitespace characters

Copy link
Author

Choose a reason for hiding this comment

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

@ljharb I've reverted the changes to fixable and category in made by the previous PR author as they are not related to --fix-type (purpose of this PR); please have a look, thanks!

Copy link
Author

Choose a reason for hiding this comment

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

looks like this change was correct, because the fix changes only spacing before the bracket (indents + newline), see eslint/eslint#15489 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

@sryze can we make a separate PR that changes all the "code"s to "whitespace"s?

docs: {
description: 'Enforce or disallow spaces inside of curly braces in JSX attributes',
category: 'Stylistic Issues',
recommended: false,
url: docsUrl('jsx-curly-spacing'),
},
fixable: 'code',
fixable: 'whitespace',
Copy link
Member

Choose a reason for hiding this comment

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

same here?

docs: {
description: 'Disallow or enforce spaces around equal signs in JSX attributes',
category: 'Stylistic Issues',
recommended: false,
url: docsUrl('jsx-equals-spacing'),
},
fixable: 'code',
fixable: 'whitespace',
Copy link
Member

Choose a reason for hiding this comment

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

and here?

docs: {
description: 'Ensure proper position of the first property in JSX',
category: 'Stylistic Issues',
recommended: false,
url: docsUrl('jsx-first-prop-new-line'),
},
fixable: 'code',
fixable: 'whitespace',
Copy link
Member

Choose a reason for hiding this comment

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

and here

docs: {
description: 'Validate props indentation in JSX',
category: 'Stylistic Issues',
recommended: false,
url: docsUrl('jsx-indent-props'),
},
fixable: 'code',
fixable: 'whitespace',
Copy link
Member

Choose a reason for hiding this comment

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

and here

docs: {
description: 'Disallow multiple spaces between inline JSX props',
category: 'Stylistic Issues',
recommended: false,
url: docsUrl('jsx-props-no-multi-spaces'),
},
fixable: 'code',
fixable: 'whitespace',
Copy link
Member

Choose a reason for hiding this comment

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

and here

deprecated: true,
docs: {
description: 'Validate spacing before closing bracket in JSX',
category: 'Stylistic Issues',
recommended: false,
url: docsUrl('jsx-space-before-closing'),
},
fixable: 'code',
fixable: 'whitespace',
Copy link
Member

Choose a reason for hiding this comment

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

and here

Comment on lines 26 to 27
category: 'Best Practices',
category: 'Possible Errors',
Copy link
Member

Choose a reason for hiding this comment

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

changing the category is a breaking change; also, this is indeed a best practice, and does not necessarily indicate a problem.

Comment on lines 34 to 35
category: 'Stylistic Issues',
category: 'Possible Errors',
Copy link
Member

Choose a reason for hiding this comment

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

this is a breaking change, and also should be done in a separate PR.

Comment on lines 54 to 55
category: 'Best Practices',
category: 'Possible Errors',
Copy link
Member

Choose a reason for hiding this comment

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

also here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Autofix --fix-type layout [ESLint v5.9.0] Fix types
2 participants