From 32e27b7b8d31feffaa5cc572916d131d4c40df63 Mon Sep 17 00:00:00 2001 From: Kenneth Law Date: Thu, 14 Mar 2019 06:30:36 +0800 Subject: [PATCH] [Fix] `jsx-indent`: Fix false positive when a jsx element is the last statement within a do expression (with tests) Fixes #2199. --- lib/rules/jsx-indent.js | 74 ++++++++++++- tests/lib/rules/jsx-indent.js | 188 ++++++++++++++++++++++++++++++++++ 2 files changed, 261 insertions(+), 1 deletion(-) diff --git a/lib/rules/jsx-indent.js b/lib/rules/jsx-indent.js index 9373e02616..f6dfc3ab92 100644 --- a/lib/rules/jsx-indent.js +++ b/lib/rules/jsx-indent.js @@ -199,6 +199,77 @@ module.exports = { ); } + /** + * Check if the node is within a DoExpression block but not the first expression (which need to be indented) + * @param {ASTNode} node The node to check + * @return {Boolean} true if its the case, false if not + */ + function isSecondOrSubsequentExpWithinDoExp(node) { + /* + It returns true when node.parent.parent.parent.parent matches: + + DoExpression({ + ..., + body: BlockStatement({ + ..., + body: [ + ..., // 1-n times + ExpressionStatement({ + ..., + expression: JSXElement({ + ..., + openingElement: JSXOpeningElement() // the node + }) + }), + ... // 0-n times + ] + }) + }) + + except: + + DoExpression({ + ..., + body: BlockStatement({ + ..., + body: [ + ExpressionStatement({ + ..., + expression: JSXElement({ + ..., + openingElement: JSXOpeningElement() // the node + }) + }), + ... // 0-n times + ] + }) + }) + */ + const isInExpStmt = ( + node.parent && + node.parent.parent && + node.parent.parent.type === 'ExpressionStatement' + ); + if (!isInExpStmt) { + return false; + } + + const expStmt = node.parent.parent; + const isInBlockStmtWithinDoExp = ( + expStmt.parent && + expStmt.parent.type === 'BlockStatement' && + expStmt.parent.parent && + expStmt.parent.parent.type === 'DoExpression' + ); + if (!isInBlockStmtWithinDoExp) { + return false; + } + + const blockStmt = expStmt.parent; + const blockStmtFirstExp = blockStmt.body[0]; + return !(blockStmtFirstExp === expStmt); + } + /** * Check indent for nodes list * @param {ASTNode} node The node to check @@ -244,7 +315,8 @@ module.exports = { const indent = ( prevToken.loc.start.line === node.loc.start.line || isRightInLogicalExp(node) || - isAlternateInConditionalExp(node) + isAlternateInConditionalExp(node) || + isSecondOrSubsequentExpWithinDoExp(node) ) ? 0 : indentSize; checkNodesIndent(node, parentElementIndent + indent); } diff --git a/tests/lib/rules/jsx-indent.js b/tests/lib/rules/jsx-indent.js index a9794e0427..af01578abc 100644 --- a/tests/lib/rules/jsx-indent.js +++ b/tests/lib/rules/jsx-indent.js @@ -690,6 +690,114 @@ ruleTester.run('jsx-indent', rule, { ].join('\n'), parser: parsers.BABEL_ESLINT, options: [2] + }, { + code: [ + '', + ' {do {', + ' const num = rollDice();', + ' ;', + ' }}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, { + code: [ + '', + ' {(do {', + ' const num = rollDice();', + ' ;', + ' })}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, { + code: [ + '', + ' {do {', + ' const purposeOfLife = getPurposeOfLife();', + ' if (purposeOfLife == 42) {', + ' ;', + ' } else {', + ' ;', + ' }', + ' }}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, { + code: [ + '', + ' {(do {', + ' const purposeOfLife = getPurposeOfLife();', + ' if (purposeOfLife == 42) {', + ' ;', + ' } else {', + ' ;', + ' }', + ' })}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, { + code: [ + '', + ' {do {', + ' ;', + ' }}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, { + code: [ + '', + ' {(do {', + ' ;', + ' })}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, { + code: [ + '', + ' {do {', + ' ;', + ' ;', + ' }}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, { + code: [ + '', + ' {(do {', + ' ;', + ' ;', + ' })}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, { + code: [ + '', + ' {do {', + ' const purposeOfLife = 42;', + ' ;', + ' ;', + ' }}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, { + code: [ + '', + ' {(do {', + ' const purposeOfLife = 42;', + ' ;', + ' ;', + ' })}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT }, { code: ` class Test extends React.Component { @@ -1695,5 +1803,85 @@ const Component = () => ( errors: [ {message: 'Expected indentation of 12 space characters but found 10.'} ] + }, { + code: [ + '', + ' {do {', + ' const num = rollDice();', + ' ;', + ' }}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT, + output: [ + '', + ' {do {', + ' const num = rollDice();', + ' ;', + ' }}', + '' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 8 space characters but found 12.'} + ] + }, { + code: [ + '', + ' {(do {', + ' const num = rollDice();', + ' ;', + ' })}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT, + output: [ + '', + ' {(do {', + ' const num = rollDice();', + ' ;', + ' })}', + '' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 8 space characters but found 12.'} + ] + }, { + code: [ + '', + ' {do {', + ' ;', + ' }}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT, + output: [ + '', + ' {do {', + ' ;', + ' }}', + '' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 8 space characters but found 4.'} + ] + }, { + code: [ + '', + ' {(do {', + ' ;', + ' })}', + '' + ].join('\n'), + parser: parsers.BABEL_ESLINT, + output: [ + '', + ' {(do {', + ' ;', + ' })}', + '' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 8 space characters but found 4.'} + ] }] });