Skip to content

Commit

Permalink
[Fix] jsx-indent with tabs (fixes jsx-eslint#1057)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kent C. Dodds committed Feb 1, 2017
1 parent c97dd0f commit 2749899
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 74 deletions.
156 changes: 97 additions & 59 deletions lib/rules/jsx-indent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,18 @@ 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 {Boolean} isEndOfTag is used to indent the right thing
* @returns {Function} function to be executed by the fixer
* @private
*/
function getFixerFunction(node, needed) {
function getFixerFunction(node, needed, isEndOfTag) {
return function(fixer) {
var indent = Array(needed + 1).join(indentChar);
return fixer.replaceTextRange(
[node.start - node.loc.start.column, node.start],
indent
);
var rangeToReplace = [node.start - node.loc.start.column, node.start];
if (isEndOfTag) {
rangeToReplace = [node.end - node.loc.end.column, node.end - 1];
}
return fixer.replaceTextRange(rangeToReplace, indent);
};
}

Expand All @@ -93,46 +95,32 @@ 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 {Boolean} isEndOfTag which is used in the fixer function
*/
function report(node, needed, gotten, loc) {
function report(node, needed, gotten, isEndOfTag) {
var msgContext = {
needed: needed,
type: indentType,
characters: needed === 1 ? 'character' : 'characters',
gotten: gotten
};

if (loc) {
context.report({
node: node,
loc: loc,
message: MESSAGE,
data: msgContext,
fix: getFixerFunction(node, needed)
});
} else {
context.report({
node: node,
message: MESSAGE,
data: msgContext,
fix: getFixerFunction(node, needed)
});
}
context.report({
node: node,
message: MESSAGE,
data: msgContext,
fix: getFixerFunction(node, needed, isEndOfTag)
});
}

/**
* Get node indent
* @param {ASTNode} node Node to examine
* @param {Boolean} byLastLine get indent of node's last line
* @param {Boolean} excludeCommas skip comma on start of line
* @return {Number} Indent
* Get the indentation (of the proper indentType) that exists in the source
* @param {String} the source string
* @param {Boolean} whether the line checked should be the last (defaults to the first line)
* @param {Boolean} whether to skip commas in the check (defaults to false)
* @return {Number} the indentation of the indentType that exists on the line
*/
function getNodeIndent(node, byLastLine, excludeCommas) {
byLastLine = byLastLine || false;
excludeCommas = excludeCommas || false;

var src = sourceCode.getText(node, node.loc.start.column + extraColumnStart);
function getIndentFromString(src, byLastLine, excludeCommas) {
var lines = src.split('\n');
if (byLastLine) {
src = lines[lines.length - 1];
Expand All @@ -154,7 +142,24 @@ module.exports = {
}

/**
* Checks node is the first in its own start line. By default it looks by start line.
* Get node indent
* @param {ASTNode} node Node to examine
* @param {Boolean} byLastLine get indent of node's last line
* @param {Boolean} excludeCommas skip comma on start of line
* @return {Number} Indent
*/
function getNodeIndent(node, byLastLine, excludeCommas) {
byLastLine = byLastLine || false;
excludeCommas = excludeCommas || false;

var src = sourceCode.getText(node, node.loc.start.column + extraColumnStart);

return getIndentFromString(src, byLastLine, excludeCommas);
}

/**
* Checks if the node is the first in its own start line. By default it looks by start line.
* One exception is closing tags with preceeding whitespace
* @param {ASTNode} node The node to check
* @return {Boolean} true if its the first in the its start line
*/
Expand All @@ -165,8 +170,9 @@ module.exports = {
} while (token.type === 'JSXText' && /^\s*$/.test(token.value));
var startLine = node.loc.start.line;
var endLine = token ? token.loc.end.line : -1;
var whitespaceOnly = token ? /\n\s*$/.test(token.value) : false;

return startLine !== endLine;
return startLine !== endLine || whitespaceOnly;
}

/**
Expand Down Expand Up @@ -218,41 +224,73 @@ module.exports = {
}
}

/**
* Checks the end of the tag (>) to determine whether it's on its own line
* If so, it verifies the indentation is correct and reports if it is not
* @param {[type]} node [description]
* @param {[type]} startIndent [description]
* @return {[type]} [description]
*/
function checkTagEndIndent(node, startIndent) {
var source = sourceCode.getText(node);
var isTagEndOnOwnLine = /\n\s*\/?>$/.exec(source);
if (isTagEndOnOwnLine) {
var endIndent = getIndentFromString(source, true, false);
if (endIndent !== startIndent) {
report(node, startIndent, endIndent, true);
}
}
}

function getOpeningElementIndent(node) {
var prevToken = sourceCode.getTokenBefore(node);
if (!prevToken) {
return 0;
}
// Use the parent in a list or an array
if (prevToken.type === 'JSXText' || prevToken.type === 'Punctuator' && prevToken.value === ',') {
prevToken = sourceCode.getNodeByRangeIndex(prevToken.start);
prevToken = prevToken.type === 'Literal' ? prevToken.parent : prevToken;
// Use the first non-punctuator token in a conditional expression
} else if (prevToken.type === 'Punctuator' && prevToken.value === ':') {
do {
prevToken = sourceCode.getTokenBefore(prevToken);
} while (prevToken.type === 'Punctuator');
prevToken = sourceCode.getNodeByRangeIndex(prevToken.start);
while (prevToken.parent && prevToken.parent.type !== 'ConditionalExpression') {
prevToken = prevToken.parent;
}
}
prevToken = prevToken.type === 'JSXExpressionContainer' ? prevToken.expression : prevToken;

var parentElementIndent = getNodeIndent(prevToken);
if (prevToken.type === 'JSXElement') {
parentElementIndent = getOpeningElementIndent(prevToken.openingElement);
}

var indent = (
prevToken.loc.start.line === node.loc.start.line ||
isRightInLogicalExp(node) ||
isAlternateInConditionalExp(node)
) ? 0 : indentSize;
return parentElementIndent + indent;
}

return {
JSXOpeningElement: function(node) {
var prevToken = sourceCode.getTokenBefore(node);
if (!prevToken) {
return;
}
// Use the parent in a list or an array
if (prevToken.type === 'JSXText' || prevToken.type === 'Punctuator' && prevToken.value === ',') {
prevToken = sourceCode.getNodeByRangeIndex(prevToken.start);
prevToken = prevToken.type === 'Literal' ? prevToken.parent : prevToken;
// Use the first non-punctuator token in a conditional expression
} else if (prevToken.type === 'Punctuator' && prevToken.value === ':') {
do {
prevToken = sourceCode.getTokenBefore(prevToken);
} while (prevToken.type === 'Punctuator');
prevToken = sourceCode.getNodeByRangeIndex(prevToken.start);
while (prevToken.parent && prevToken.parent.type !== 'ConditionalExpression') {
prevToken = prevToken.parent;
}
}
prevToken = prevToken.type === 'JSXExpressionContainer' ? prevToken.expression : prevToken;

var parentElementIndent = getNodeIndent(prevToken);
var indent = (
prevToken.loc.start.line === node.loc.start.line ||
isRightInLogicalExp(node) ||
isAlternateInConditionalExp(node)
) ? 0 : indentSize;
checkNodesIndent(node, parentElementIndent + indent);
var startIndent = getOpeningElementIndent(node);
checkNodesIndent(node, startIndent);
checkTagEndIndent(node, startIndent);
},
JSXClosingElement: function(node) {
if (!node.parent) {
return;
}
var peerElementIndent = getNodeIndent(node.parent.openingElement);
var peerElementIndent = getOpeningElementIndent(node.parent.openingElement);
checkNodesIndent(node, peerElementIndent);
},
JSXExpressionContainer: function(node) {
Expand Down
64 changes: 49 additions & 15 deletions tests/lib/rules/jsx-indent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,16 @@ ruleTester.run('jsx-indent', rule, {
].join('\n'),
options: [0],
parserOptions: parserOptions
}, {
code: [
' <App>',
'<Foo />',
' </App>'
].join('\n'),
options: [-2],
parserOptions: parserOptions
// }, {
// TODO: should we put effort in making this work?
// who in their right mind would do this?
// code: [
// ' <App>',
// '<Foo />',
// ' </App>'
// ].join('\n'),
// options: [-2],
// parserOptions: parserOptions
}, {
code: [
'<App>',
Expand Down Expand Up @@ -459,6 +461,38 @@ ruleTester.run('jsx-indent', rule, {
options: ['tab'],
parserOptions: parserOptions,
errors: [{message: 'Expected indentation of 1 tab character but found 0.'}]
}, {
code: [
'function MyComponent(props) {',
'\treturn (',
' <div',
'\t\t\tclassName="foo-bar"',
'\t\t\tid="thing"',
' >',
' Hello world!',
' </div>',
'\t)',
'}'
].join('\n'),
output: [
'function MyComponent(props) {',
'\treturn (',
'\t\t<div',
'\t\t\tclassName="foo-bar"',
'\t\t\tid="thing"',
'\t\t>',
'\t\t\tHello world!',
'\t\t</div>',
'\t)',
'}'
].join('\n'),
options: ['tab'],
parserOptions: parserOptions,
errors: [
{message: 'Expected indentation of 2 tab characters but found 0.'},
{message: 'Expected indentation of 2 tab characters but found 0.'},
{message: 'Expected indentation of 2 tab characters but found 0.'}
]
}, {
code: [
'function App() {',
Expand Down Expand Up @@ -505,22 +539,22 @@ 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: [
output: [
'function App() {',
' return (',
' <App>',
' <Foo />',
' </App>',
' );',
'}'
].join('\n'), */
].join('\n'),
options: [2],
parserOptions: parserOptions,
errors: [{message: 'Expected indentation of 4 space characters but found 0.'}]
errors: [
{message: 'Expected indentation of 4 space characters but found 0.'},
{message: 'Expected indentation of 6 space characters but found 2.'},
{message: 'Expected indentation of 4 space characters but found 0.'}
]
}, {
code: [
'<App>',
Expand Down

0 comments on commit 2749899

Please sign in to comment.