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 1e7fbe5
Show file tree
Hide file tree
Showing 2 changed files with 183 additions and 86 deletions.
193 changes: 133 additions & 60 deletions lib/rules/jsx-indent.js
View file
Expand Up @@ -73,18 +73,16 @@ module.exports = {

/**
* Responsible for fixing the indentation issue fix
* @param {ASTNode} node Node violating the indent rule
* @param {Boolean} rangeToReplace is used to specify the range
* to replace with the correct indentation.
* @param {Number} needed Expected indentation character count
* @returns {Function} function to be executed by the fixer
* @private
*/
function getFixerFunction(node, needed) {
function getFixerFunction(rangeToReplace, needed) {
return function(fixer) {
var indent = Array(needed + 1).join(indentChar);
return fixer.replaceTextRange(
[node.start - node.loc.start.column, node.start],
indent
);
return fixer.replaceTextRange(rangeToReplace, indent);
};
}

Expand All @@ -93,46 +91,38 @@ 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 {Array} rangeToReplace is used in the fixer.
* Defaults to the indent of the start of the node
* @param {Object} loc Error line and column location (defaults to node.loc
*/
function report(node, needed, gotten, loc) {
function report(node, needed, gotten, rangeToReplace, loc) {
var msgContext = {
needed: needed,
type: indentType,
characters: needed === 1 ? 'character' : 'characters',
gotten: gotten
};
rangeToReplace = rangeToReplace || [node.start - node.loc.start.column, node.start];

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,
loc: loc || node.loc,
message: MESSAGE,
data: msgContext,
fix: getFixerFunction(rangeToReplace, needed)
});
}

/**
* 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} src the source string
* @param {Boolean} byLastLine whether the line checked should be the last
* Defaults to the first line
* @param {Boolean} excludeCommas 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 +144,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 +172,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 +226,78 @@ 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 {ASTNode} node The node to check
* @param {Number} startIndent The indentation of the start of the tag
*/
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) {
var rangeToReplace = [node.end - node.loc.end.column, node.end - 1];
report(node, startIndent, endIndent, rangeToReplace);
}
}
}

/**
* Gets what the JSXOpeningElement's indentation should be
* @param {ASTNode} node The JSXOpeningElement
* @return {Number} the number of indentation characters it should have
*/
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 All @@ -261,6 +306,34 @@ module.exports = {
}
var parentNodeIndent = getNodeIndent(node.parent);
checkNodesIndent(node, parentNodeIndent + indentSize);
},
Literal: function(node) {
if (!node.parent || (node.parent.type !== 'JSXElement' && node.parent.type !== 'JSXExpressionContainer')) {
return;
}
var parentElementIndent = getOpeningElementIndent(node.parent.openingElement);
var expectedIndent = parentElementIndent + indentSize;
var source = sourceCode.getText(node);
var lines = source.split('\n');
var currentIndex = 0;
lines.forEach(function(line, lineNumber) {
if (line.trim()) {
var lineIndent = getIndentFromString(line);
if (lineIndent !== expectedIndent) {
var lineStart = source.indexOf(line, currentIndex);
var lineIndentStart = line.search(/\S/);
var lineIndentEnd = lineStart + lineIndentStart;
var rangeToReplace = [node.start + lineStart, node.start + lineIndentEnd];
var locLine = lineNumber + node.loc.start.line;
var loc = {
start: {line: locLine, column: lineIndentStart},
end: {line: locLine, column: lineIndentEnd}
};
report(node, expectedIndent, lineIndent, rangeToReplace, loc);
}
}
currentIndex += line.length;
});
}
};

Expand Down
76 changes: 50 additions & 26 deletions tests/lib/rules/jsx-indent.js
View file
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
// }, {
// 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 @@ -211,17 +213,6 @@ ruleTester.run('jsx-indent', rule, {
'</div>'
].join('\n'),
parserOptions: parserOptions
}, {
// Literals indentation is not touched
code: [
'<div>',
'bar <div>',
' bar',
' bar {foo}',
'bar </div>',
'</div>'
].join('\n'),
parserOptions: parserOptions
}, {
// Multiline ternary
// (colon at the end of the first expression)
Expand Down Expand Up @@ -459,6 +450,39 @@ 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 3 tab characters but found 0.'},
{message: 'Expected indentation of 2 tab characters but found 0.'}
]
}, {
code: [
'function App() {',
Expand Down Expand Up @@ -505,22 +529,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 1e7fbe5

Please sign in to comment.