Skip to content

Autofix: jsx-indent: Add support for tabs #608

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

Merged
merged 1 commit into from
Nov 1, 2016
Merged

Autofix: jsx-indent: Add support for tabs #608

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/rules/jsx-indent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This option validates a specific indentation style for JSX.

**Fixable:** This rule is automatically fixable using the `--fix` flag on the command line.
Fixer will fix whitespace and tabs indentation. It won't replace tabs with whitespaces and vice-versa.
Fixer will fix whitespace and tabs indentation.

## Rule Details

Expand Down
24 changes: 9 additions & 15 deletions lib/rules/jsx-indent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ module.exports = {
category: 'Stylistic Issues',
recommended: false
},
// fixer will fix whitespace and tabs indentation.
// It won't replace tabs with whitespaces and vice-versa.
fixable: 'whitespace',
schema: [{
oneOf: [{
Expand Down Expand Up @@ -77,20 +75,16 @@ module.exports = {
* Responsible for fixing the indentation issue fix
* @param {ASTNode} node Node violating the indent rule
* @param {Number} needed Expected indentation character count
* @param {Number} gotten Indentation character count in the actual node/code
* @returns {Function} function to be executed by the fixer
* @private
*/
function getFixerFunction(node, needed, gotten) {
if (needed > gotten) {
var spaces = indentChar.repeat(needed - gotten);
return function(fixer) {
return fixer.insertTextBeforeRange([node.range[0], node.range[0]], spaces);
};
}

function getFixerFunction(node, needed) {
return function(fixer) {
return fixer.removeRange([node.range[0] - (gotten - needed), node.range[0]]);
var indent = Array(needed + 1).join(indentChar);
return fixer.replaceTextRange(
[node.start - node.loc.start.column, node.start],
indent
Copy link
Contributor Author

@jayphelps jayphelps Oct 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

This previous code used String.prototype.repeat() which is ES2015. I can switch back to using that instead of the join() trick I used, if it's a safe to assume repeat is supported.

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively you can just bring in a dep like https://www.npmjs.com/package/string-repeat

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just let me know which way you'd like. to me it seems silly to bring in a lib since the join trick is short, simple, and I think fairly obvious what it does. 😏 aka left-pad

Copy link
Member

Choose a reason for hiding this comment

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

In general, left-pad in no way changes that more deps is better. left-pad was only a problem for people that were already ignoring a decade of best practices and depending on third-party registries for their dependencies.

In this case, keeping your join trick is fine too.

);
};
}

Expand All @@ -99,7 +93,7 @@ module.exports = {
* @param {ASTNode} node Node violating the indent rule
* @param {Number} needed Expected indentation character count
* @param {Number} gotten Indentation character count in the actual node/code
* @param {Object=} loc Error line and column location
* @param {Object} loc Error line and column location
*/
function report(node, needed, gotten, loc) {
var msgContext = {
Expand All @@ -115,14 +109,14 @@ module.exports = {
loc: loc,
message: MESSAGE,
data: msgContext,
fix: getFixerFunction(node, needed, gotten)
fix: getFixerFunction(node, needed)
});
} else {
context.report({
node: node,
message: MESSAGE,
data: msgContext,
fix: getFixerFunction(node, needed, gotten)
fix: getFixerFunction(node, needed)
});
}
}
Expand Down
53 changes: 48 additions & 5 deletions tests/lib/rules/jsx-indent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,11 @@ ruleTester.run('jsx-indent', rule, {
' <Foo />',
'</App>'
].join('\n'),
output: [
'<App>',
'\t<Foo />',
'</App>'
].join('\n'),
options: ['tab'],
parserOptions: parserOptions,
errors: [{message: 'Expected indentation of 1 tab character but found 0.'}]
Expand Down Expand Up @@ -282,6 +287,19 @@ ruleTester.run('jsx-indent', rule, {
' );',
'}'
].join('\n'),
// The detection logic only thinks <App> is indented wrong, not the other
// two lines following. I *think* because it incorrectly uses <App>'s indention
// as the baseline for the next two, instead of the realizing the entire three
// lines are wrong together. See #608
/* output: [
'function App() {',
' return (',
' <App>',
' <Foo />',
' </App>',
' );',
'}'
].join('\n'), */
options: [2],
parserOptions: parserOptions,
errors: [{message: 'Expected indentation of 4 space characters but found 0.'}]
Expand All @@ -291,6 +309,11 @@ ruleTester.run('jsx-indent', rule, {
' {test}',
'</App>'
].join('\n'),
output: [
'<App>',
' {test}',
'</App>'
].join('\n'),
parserOptions: parserOptions,
errors: [
{message: 'Expected indentation of 4 space characters but found 3.'}
Expand All @@ -305,6 +328,15 @@ ruleTester.run('jsx-indent', rule, {
' ))}',
'</App>'
].join('\n'),
output: [
'<App>',
' {options.map((option, index) => (',
' <option key={index} value={option.key}>',
' {option.name}',
' </option>',
' ))}',
'</App>'
].join('\n'),
parserOptions: parserOptions,
errors: [
{message: 'Expected indentation of 12 space characters but found 11.'}
Expand All @@ -315,6 +347,11 @@ ruleTester.run('jsx-indent', rule, {
'{test}',
'</App>'
].join('\n'),
output: [
'<App>',
'\t{test}',
'</App>'
].join('\n'),
parserOptions: parserOptions,
options: ['tab'],
errors: [
Expand All @@ -330,6 +367,15 @@ ruleTester.run('jsx-indent', rule, {
'\t))}',
'</App>'
].join('\n'),
output: [
'<App>',
'\t{options.map((option, index) => (',
'\t\t<option key={index} value={option.key}>',
'\t\t\t{option.name}',
'\t\t</option>',
'\t))}',
'</App>'
].join('\n'),
parserOptions: parserOptions,
options: ['tab'],
errors: [
Expand Down Expand Up @@ -369,10 +415,7 @@ ruleTester.run('jsx-indent', rule, {
errors: [
{message: 'Expected indentation of 2 space characters but found 4.'}
]
}
// Tests for future work. See the comment on line 42-43 in the rule near to fixable: 'whitespace' meta property,
// Right now fixer function doesn't support replacing tabs with whitespaces and vice-versa.
/* , {
}, {
code: [
'<App>\n',
' <Foo />\n',
Expand Down Expand Up @@ -404,5 +447,5 @@ ruleTester.run('jsx-indent', rule, {
errors: [
{message: 'Expected indentation of 2 space characters but found 0.'}
]
}*/]
}]
});