From 27498990e9dc646fc677f93e0aac82372ed9f483 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Wed, 1 Feb 2017 10:37:23 -0700 Subject: [PATCH] [Fix] jsx-indent with tabs (fixes #1057) --- lib/rules/jsx-indent.js | 156 +++++++++++++++++++++------------- tests/lib/rules/jsx-indent.js | 64 ++++++++++---- 2 files changed, 146 insertions(+), 74 deletions(-) diff --git a/lib/rules/jsx-indent.js b/lib/rules/jsx-indent.js index 6a75b6f64f..8fb7826212 100644 --- a/lib/rules/jsx-indent.js +++ b/lib/rules/jsx-indent.js @@ -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); }; } @@ -93,9 +95,9 @@ 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, @@ -103,36 +105,22 @@ module.exports = { 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]; @@ -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 */ @@ -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; } /** @@ -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) { diff --git a/tests/lib/rules/jsx-indent.js b/tests/lib/rules/jsx-indent.js index a0072709c1..b6c9f21458 100644 --- a/tests/lib/rules/jsx-indent.js +++ b/tests/lib/rules/jsx-indent.js @@ -51,14 +51,16 @@ ruleTester.run('jsx-indent', rule, { ].join('\n'), options: [0], parserOptions: parserOptions - }, { - code: [ - ' ', - '', - ' ' - ].join('\n'), - options: [-2], - parserOptions: parserOptions + // }, { + // TODO: should we put effort in making this work? + // who in their right mind would do this? + // code: [ + // ' ', + // '', + // ' ' + // ].join('\n'), + // options: [-2], + // parserOptions: parserOptions }, { code: [ '', @@ -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 (', + ' ', + ' Hello world!', + ' ', + '\t)', + '}' + ].join('\n'), + output: [ + 'function MyComponent(props) {', + '\treturn (', + '\t\t', + '\t\t\tHello world!', + '\t\t', + '\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() {', @@ -505,11 +539,7 @@ ruleTester.run('jsx-indent', rule, { ' );', '}' ].join('\n'), - // The detection logic only thinks is indented wrong, not the other - // two lines following. I *think* because it incorrectly uses '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 (', ' ', @@ -517,10 +547,14 @@ ruleTester.run('jsx-indent', rule, { ' ', ' );', '}' - ].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: [ '',