Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] jsx-indent: Fix false positive when a jsx element is the last statement within a do expression #2200

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 73 additions & 1 deletion lib/rules/jsx-indent.js
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
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.'}
]
}]
});