From e9fb52d332e15686865952371c333b19af13eede Mon Sep 17 00:00:00 2001 From: Yannick Croissant Date: Mon, 20 Mar 2017 22:04:02 +0100 Subject: [PATCH] Revert "[Fix] jsx-indent with tabs (fixes #1057)" This reverts commit 6e5f688065aa218d71742dc4b31929c16ab1078d. Conflicts: lib/rules/jsx-indent.js tests/lib/rules/jsx-indent.js --- docs/rules/jsx-indent.md | 62 ---------- lib/rules/jsx-indent.js | 210 ++++++++++------------------------ tests/lib/rules/jsx-indent.js | 103 +++++------------ 3 files changed, 86 insertions(+), 289 deletions(-) diff --git a/docs/rules/jsx-indent.md b/docs/rules/jsx-indent.md index 336e72e382..909c5818f1 100644 --- a/docs/rules/jsx-indent.md +++ b/docs/rules/jsx-indent.md @@ -77,68 +77,6 @@ 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 791d8cb9d0..8d5af5421c 100644 --- a/lib/rules/jsx-indent.js +++ b/lib/rules/jsx-indent.js @@ -46,14 +46,6 @@ module.exports = { }, { type: 'integer' }] - }, { - type: 'object', - properties: { - indentLogicalExpressions: { - type: 'boolean' - } - }, - additionalProperties: false }] }, @@ -64,7 +56,6 @@ module.exports = { var extraColumnStart = 0; var indentType = 'space'; var indentSize = 4; - var indentLogicalExpressions = false; var sourceCode = context.getSourceCode(); @@ -76,25 +67,24 @@ 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 {Boolean} rangeToReplace is used to specify the range - * to replace with the correct indentation. + * @param {ASTNode} node Node violating the indent rule * @param {Number} needed Expected indentation character count * @returns {Function} function to be executed by the fixer * @private */ - function getFixerFunction(rangeToReplace, needed) { + function getFixerFunction(node, needed) { return function(fixer) { var indent = Array(needed + 1).join(indentChar); - return fixer.replaceTextRange(rangeToReplace, indent); + return fixer.replaceTextRange( + [node.start - node.loc.start.column, node.start], + indent + ); }; } @@ -103,38 +93,46 @@ 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 {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 + * @param {Object} loc Error line and column location */ - function report(node, needed, gotten, rangeToReplace, loc) { + function report(node, needed, gotten, loc) { var msgContext = { needed: needed, type: indentType, characters: needed === 1 ? 'character' : 'characters', gotten: gotten }; - rangeToReplace = rangeToReplace || [node.start - node.loc.start.column, node.start]; - context.report({ - node: node, - loc: loc || node.loc, - message: MESSAGE, - data: msgContext, - fix: getFixerFunction(rangeToReplace, needed) - }); + 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) + }); + } } /** - * 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 + * 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 getIndentFromString(src, byLastLine, excludeCommas) { + function getNodeIndent(node, byLastLine, excludeCommas) { + byLastLine = byLastLine || false; + excludeCommas = excludeCommas || false; + + var src = sourceCode.getText(node, node.loc.start.column + extraColumnStart); var lines = src.split('\n'); if (byLastLine) { src = lines[lines.length - 1]; @@ -156,24 +154,7 @@ module.exports = { } /** - * 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 + * Checks node is the first in its own start line. By default it looks by start line. * @param {ASTNode} node The node to check * @return {Boolean} true if its the first in the its start line */ @@ -184,9 +165,8 @@ 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 || whitespaceOnly; + return startLine !== endLine; } /** @@ -238,83 +218,41 @@ 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.range[0]); - - 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' && (!prevToken.parent || prevToken.parent.type !== 'LogicalExpression')) { - 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; } - var startIndent = getOpeningElementIndent(node); - checkNodesIndent(node, startIndent); - checkTagEndIndent(node, startIndent); + // 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.range[0]); + 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); }, JSXClosingElement: function(node) { if (!node.parent) { return; } - var peerElementIndent = getOpeningElementIndent(node.parent.openingElement); + var peerElementIndent = getNodeIndent(node.parent.openingElement); checkNodesIndent(node, peerElementIndent); }, JSXExpressionContainer: function(node) { @@ -323,34 +261,6 @@ module.exports = { } var parentNodeIndent = getNodeIndent(node.parent); checkNodesIndent(node, parentNodeIndent + indentSize); - }, - Literal: function(node) { - if (!node.parent || node.parent.type !== 'JSXElement' || node.loc.start.line === node.parent.loc.start.line) { - 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 a06b58f8e6..4d1789eccf 100644 --- a/tests/lib/rules/jsx-indent.js +++ b/tests/lib/rules/jsx-indent.js @@ -51,6 +51,14 @@ ruleTester.run('jsx-indent', rule, { ].join('\n'), options: [0], parserOptions: parserOptions + }, { + code: [ + ' ', + '', + ' ' + ].join('\n'), + options: [-2], + parserOptions: parserOptions }, { code: [ '', @@ -203,6 +211,17 @@ 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) @@ -397,19 +416,6 @@ ruleTester.run('jsx-indent', rule, { ')' ].join('\n'), parserOptions: parserOptions - }, { - code: [ - '', - ' {', - ' condition &&', - ' ', - ' ', - ' ', - ' }', - '' - ].join('\n'), - options: [4, {indentLogicalExpressions: true}], - parserOptions: parserOptions }, { code: [ '', @@ -451,30 +457,6 @@ ruleTester.run('jsx-indent', rule, { ].join('\n'), options: [2], parserOptions: parserOptions - }, { - code: [ - 'function foo() {', - ' Text', - '}' - ].join('\n'), - options: [2], - parserOptions: parserOptions - }, { - code: [ - 'function foo() {', - ' return (', - '
', - ' {bar &&', - '
', - ' ', - '
', - ' }', - '
', - ' );', - '}' - ].join('\n'), - options: [2], - parserOptions: parserOptions }], invalid: [{ @@ -518,39 +500,6 @@ 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() {', @@ -597,7 +546,11 @@ ruleTester.run('jsx-indent', rule, { ' );', '}' ].join('\n'), - output: [ + // 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: [ 'function App() {', ' return (', ' ', @@ -605,14 +558,10 @@ ruleTester.run('jsx-indent', rule, { ' ', ' );', '}' - ].join('\n'), + ].join('\n'), */ options: [2], parserOptions: parserOptions, - 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.'} - ] + errors: [{message: 'Expected indentation of 4 space characters but found 0.'}] }, { code: [ '',