Skip to content

Commit

Permalink
[Fix] jsx-indent: Fix false positive when a jsx element is the last…
Browse files Browse the repository at this point in the history 
… statement within a do expression (with tests)

Fixes #2199.
  • Loading branch information
@Kenneth-KT @ljharb
Kenneth-KT authored and ljharb committed Mar 13, 2019
1 parent 628a4a0 commit 32e27b7
Show file tree
Hide file tree
Showing 2 changed files with 261 additions and 1 deletion.
74 changes: 73 additions & 1 deletion lib/rules/jsx-indent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
188 changes: 188 additions & 0 deletions tests/lib/rules/jsx-indent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,114 @@ ruleTester.run('jsx-indent', rule, {
].join('\n'),
parser: parsers.BABEL_ESLINT,
options: [2]
}, {
code: [
'<span>',
' {do {',
' const num = rollDice();',
' <Thing num={num} />;',
' }}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'<span>',
' {(do {',
' const num = rollDice();',
' <Thing num={num} />;',
' })}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'<span>',
' {do {',
' const purposeOfLife = getPurposeOfLife();',
' if (purposeOfLife == 42) {',
' <Thing />;',
' } else {',
' <AnotherThing />;',
' }',
' }}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'<span>',
' {(do {',
' const purposeOfLife = getPurposeOfLife();',
' if (purposeOfLife == 42) {',
' <Thing />;',
' } else {',
' <AnotherThing />;',
' }',
' })}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'<span>',
' {do {',
' <Thing num={rollDice()} />;',
' }}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'<span>',
' {(do {',
' <Thing num={rollDice()} />;',
' })}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'<span>',
' {do {',
' <Thing num={rollDice()} />;',
' <Thing num={rollDice()} />;',
' }}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'<span>',
' {(do {',
' <Thing num={rollDice()} />;',
' <Thing num={rollDice()} />;',
' })}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'<span>',
' {do {',
' const purposeOfLife = 42;',
' <Thing num={purposeOfLife} />;',
' <Thing num={purposeOfLife} />;',
' }}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'<span>',
' {(do {',
' const purposeOfLife = 42;',
' <Thing num={purposeOfLife} />;',
' <Thing num={purposeOfLife} />;',
' })}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: `
class Test extends React.Component {
Expand Down Expand Up @@ -1695,5 +1803,85 @@ const Component = () => (
errors: [
{message: 'Expected indentation of 12 space characters but found 10.'}
]
}, {
code: [
'<span>',
' {do {',
' const num = rollDice();',
' <Thing num={num} />;',
' }}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT,
output: [
'<span>',
' {do {',
' const num = rollDice();',
' <Thing num={num} />;',
' }}',
'</span>'
].join('\n'),
errors: [
{message: 'Expected indentation of 8 space characters but found 12.'}
]
}, {
code: [
'<span>',
' {(do {',
' const num = rollDice();',
' <Thing num={num} />;',
' })}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT,
output: [
'<span>',
' {(do {',
' const num = rollDice();',
' <Thing num={num} />;',
' })}',
'</span>'
].join('\n'),
errors: [
{message: 'Expected indentation of 8 space characters but found 12.'}
]
}, {
code: [
'<span>',
' {do {',
' <Thing num={getPurposeOfLife()} />;',
' }}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT,
output: [
'<span>',
' {do {',
' <Thing num={getPurposeOfLife()} />;',
' }}',
'</span>'
].join('\n'),
errors: [
{message: 'Expected indentation of 8 space characters but found 4.'}
]
}, {
code: [
'<span>',
' {(do {',
' <Thing num={getPurposeOfLife()} />;',
' })}',
'</span>'
].join('\n'),
parser: parsers.BABEL_ESLINT,
output: [
'<span>',
' {(do {',
' <Thing num={getPurposeOfLife()} />;',
' })}',
'</span>'
].join('\n'),
errors: [
{message: 'Expected indentation of 8 space characters but found 4.'}
]
}]
});

0 comments on commit 32e27b7

Please sign in to comment.