From 6e5f688065aa218d71742dc4b31929c16ab1078d 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) --- docs/rules/jsx-indent.md | 62 ++++++++++ lib/rules/jsx-indent.js | 209 ++++++++++++++++++++++++---------- tests/lib/rules/jsx-indent.js | 79 ++++++++----- 3 files changed, 264 insertions(+), 86 deletions(-) diff --git a/docs/rules/jsx-indent.md b/docs/rules/jsx-indent.md index 909c5818f1..336e72e382 100644 --- a/docs/rules/jsx-indent.md +++ b/docs/rules/jsx-indent.md @@ -77,6 +77,68 @@ The following patterns are not warnings: ``` +#### indentLogicalExpressions + +```js +... +"react/jsx-indent": [, 'tab'|, {indentLogicalExpressions: true}] +... +``` + +By default this is set to false. When enabled, an additional indentation is required when the JSX is the right of a LogicalExpression + +The following patterns are considered warnings: + +```jsx +// 2 spaces indentation with indentLogicalExpressions as false +// [2, 2, {indentLogicalExpressions: false}] + + { + condition && + + + + } + + +// 2 spaces indentation with indentLogicalExpressions as true +// [2, 2, {indentLogicalExpressions: true}] + + { + condition && + + + + } + +``` + +The following patterns are not warnings: + +```jsx +// 2 spaces indentation with indentLogicalExpressions as true +// [2, 2, {indentLogicalExpressions: true}] + + { + condition && + + + + } + + +// 2 spaces indentation with indentLogicalExpressions as false +// [2, 2, {indentLogicalExpressions: false}] + + { + condition && + + + + } + +``` + ## When not to use If you are not using JSX then you can disable this rule. diff --git a/lib/rules/jsx-indent.js b/lib/rules/jsx-indent.js index 6a75b6f64f..5d1cfda332 100644 --- a/lib/rules/jsx-indent.js +++ b/lib/rules/jsx-indent.js @@ -46,6 +46,14 @@ module.exports = { }, { type: 'integer' }] + }, { + type: 'object', + properties: { + indentLogicalExpressions: { + type: 'boolean' + } + }, + additionalProperties: false }] }, @@ -56,6 +64,7 @@ module.exports = { var extraColumnStart = 0; var indentType = 'space'; var indentSize = 4; + var indentLogicalExpressions = false; var sourceCode = context.getSourceCode(); @@ -67,24 +76,25 @@ module.exports = { indentSize = context.options[0]; indentType = 'space'; } + if (context.options[1]) { + indentLogicalExpressions = context.options[1].indentLogicalExpressions || false; + } } var indentChar = indentType === 'space' ? ' ' : '\t'; /** * 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); }; } @@ -93,46 +103,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]; @@ -154,7 +156,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 +184,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 +238,82 @@ 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; + } + if (prevToken.type === 'JSXText' || prevToken.type === 'Punctuator' && prevToken.value === ',') { + // Use the parent in a list or an array + prevToken = sourceCode.getNodeByRangeIndex(prevToken.start); + prevToken = prevToken.type === 'Literal' ? prevToken.parent : prevToken; + } else if (prevToken.type === 'Punctuator' && prevToken.value === ':') { + // Use the first non-punctuator token in a conditional expression + 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); + } + + if (isRightInLogicalExp(node) && indentLogicalExpressions) { + parentElementIndent += indentSize; + } + + 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) { @@ -261,6 +322,34 @@ module.exports = { } var parentNodeIndent = getNodeIndent(node.parent); checkNodesIndent(node, parentNodeIndent + indentSize); + }, + Literal: function(node) { + if (!node.parent || node.parent.type !== 'JSXElement') { + 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; + }); } }; diff --git a/tests/lib/rules/jsx-indent.js b/tests/lib/rules/jsx-indent.js index a0072709c1..20e07641c7 100644 --- a/tests/lib/rules/jsx-indent.js +++ b/tests/lib/rules/jsx-indent.js @@ -51,14 +51,6 @@ ruleTester.run('jsx-indent', rule, { ].join('\n'), options: [0], parserOptions: parserOptions - }, { - code: [ - ' ', - '', - ' ' - ].join('\n'), - options: [-2], - parserOptions: parserOptions }, { code: [ '', @@ -211,17 +203,6 @@ ruleTester.run('jsx-indent', rule, { '' ].join('\n'), parserOptions: parserOptions - }, { - // Literals indentation is not touched - code: [ - '
', - 'bar
', - ' bar', - ' bar {foo}', - 'bar
', - '
' - ].join('\n'), - parserOptions: parserOptions }, { // Multiline ternary // (colon at the end of the first expression) @@ -416,6 +397,19 @@ ruleTester.run('jsx-indent', rule, { ')' ].join('\n'), parserOptions: parserOptions + }, { + code: [ + '', + ' {', + ' condition &&', + ' ', + ' ', + ' ', + ' }', + '' + ].join('\n'), + options: [4, {indentLogicalExpressions: true}], + parserOptions: parserOptions }], invalid: [{ @@ -459,6 +453,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 (', + ' ', + ' 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 3 tab characters but found 0.'}, + {message: 'Expected indentation of 2 tab characters but found 0.'} + ] }, { code: [ 'function App() {', @@ -505,11 +532,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 +540,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: [ '',