Skip to content

Commit 02c2230

Browse files
Kenneth-KTKenneth-KT
committed
[Fix] jsx-indent: Fix false positive when a jsx element is the last statement within a do expression (with tests)
Fixes jsx-eslint#2199.
1 parent 752de70 commit 02c2230

File tree

2 files changed

+261
-1
lines changed

2 files changed

+261
-1
lines changed

‎lib/rules/jsx-indent.js

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,77 @@ module.exports = {
195195
);
196196
}
197197

198+
/**
199+
* Check if the node is within a DoExpression block but not the first expression (which need to be indented)
200+
* @param {ASTNode} node The node to check
201+
* @return {Boolean} true if its the case, false if not
202+
*/
203+
function isSecondOrSubsequentExpWithinDoExp(node) {
204+
/*
205+
It returns true when node.parent.parent.parent.parent matches:
206+
207+
DoExpression({
208+
...,
209+
body: BlockStatement({
210+
...,
211+
body: [
212+
..., // 1-n times
213+
ExpressionStatement({
214+
...,
215+
expression: JSXElement({
216+
...,
217+
openingElement: JSXOpeningElement() // the node
218+
})
219+
}),
220+
... // 0-n times
221+
]
222+
})
223+
})
224+
225+
except:
226+
227+
DoExpression({
228+
...,
229+
body: BlockStatement({
230+
...,
231+
body: [
232+
ExpressionStatement({
233+
...,
234+
expression: JSXElement({
235+
...,
236+
openingElement: JSXOpeningElement() // the node
237+
})
238+
}),
239+
... // 0-n times
240+
]
241+
})
242+
})
243+
*/
244+
const isInExpStmt = (
245+
node.parent &&
246+
node.parent.parent &&
247+
node.parent.parent.type === 'ExpressionStatement'
248+
);
249+
if (!isInExpStmt) {
250+
return false;
251+
}
252+
253+
const expStmt = node.parent.parent;
254+
const isInBlockStmtWithinDoExp = (
255+
expStmt.parent &&
256+
expStmt.parent.type === 'BlockStatement' &&
257+
expStmt.parent.parent &&
258+
expStmt.parent.parent.type === 'DoExpression'
259+
);
260+
if (!isInBlockStmtWithinDoExp) {
261+
return false;
262+
}
263+
264+
const blockStmt = expStmt.parent;
265+
const blockStmtFirstExp = blockStmt.body[0];
266+
return !(blockStmtFirstExp === expStmt);
267+
}
268+
198269
/**
199270
* Check indent for nodes list
200271
* @param {ASTNode} node The node to check
@@ -239,7 +310,8 @@ module.exports = {
239310
const indent = (
240311
prevToken.loc.start.line === node.loc.start.line ||
241312
isRightInLogicalExp(node) ||
242-
isAlternateInConditionalExp(node)
313+
isAlternateInConditionalExp(node) ||
314+
isSecondOrSubsequentExpWithinDoExp(node)
243315
) ? 0 : indentSize;
244316
checkNodesIndent(node, parentElementIndent + indent);
245317
}

‎tests/lib/rules/jsx-indent.js

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,114 @@ ruleTester.run('jsx-indent', rule, {
687687
].join('\n'),
688688
parser: 'babel-eslint',
689689
options: [2]
690+
}, {
691+
code: [
692+
'<span>',
693+
' {do {',
694+
' const num = rollDice();',
695+
' <Thing num={num} />;',
696+
' }}',
697+
'</span>'
698+
].join('\n'),
699+
parser: 'babel-eslint'
700+
}, {
701+
code: [
702+
'<span>',
703+
' {(do {',
704+
' const num = rollDice();',
705+
' <Thing num={num} />;',
706+
' })}',
707+
'</span>'
708+
].join('\n'),
709+
parser: 'babel-eslint'
710+
}, {
711+
code: [
712+
'<span>',
713+
' {do {',
714+
' const purposeOfLife = getPurposeOfLife();',
715+
' if (purposeOfLife == 42) {',
716+
' <Thing />;',
717+
' } else {',
718+
' <AnotherThing />;',
719+
' }',
720+
' }}',
721+
'</span>'
722+
].join('\n'),
723+
parser: 'babel-eslint'
724+
}, {
725+
code: [
726+
'<span>',
727+
' {(do {',
728+
' const purposeOfLife = getPurposeOfLife();',
729+
' if (purposeOfLife == 42) {',
730+
' <Thing />;',
731+
' } else {',
732+
' <AnotherThing />;',
733+
' }',
734+
' })}',
735+
'</span>'
736+
].join('\n'),
737+
parser: 'babel-eslint'
738+
}, {
739+
code: [
740+
'<span>',
741+
' {do {',
742+
' <Thing num={rollDice()} />;',
743+
' }}',
744+
'</span>'
745+
].join('\n'),
746+
parser: 'babel-eslint'
747+
}, {
748+
code: [
749+
'<span>',
750+
' {(do {',
751+
' <Thing num={rollDice()} />;',
752+
' })}',
753+
'</span>'
754+
].join('\n'),
755+
parser: 'babel-eslint'
756+
}, {
757+
code: [
758+
'<span>',
759+
' {do {',
760+
' <Thing num={rollDice()} />;',
761+
' <Thing num={rollDice()} />;',
762+
' }}',
763+
'</span>'
764+
].join('\n'),
765+
parser: 'babel-eslint'
766+
}, {
767+
code: [
768+
'<span>',
769+
' {(do {',
770+
' <Thing num={rollDice()} />;',
771+
' <Thing num={rollDice()} />;',
772+
' })}',
773+
'</span>'
774+
].join('\n'),
775+
parser: 'babel-eslint'
776+
}, {
777+
code: [
778+
'<span>',
779+
' {do {',
780+
' const purposeOfLife = 42;',
781+
' <Thing num={purposeOfLife} />;',
782+
' <Thing num={purposeOfLife} />;',
783+
' }}',
784+
'</span>'
785+
].join('\n'),
786+
parser: 'babel-eslint'
787+
}, {
788+
code: [
789+
'<span>',
790+
' {(do {',
791+
' const purposeOfLife = 42;',
792+
' <Thing num={purposeOfLife} />;',
793+
' <Thing num={purposeOfLife} />;',
794+
' })}',
795+
'</span>'
796+
].join('\n'),
797+
parser: 'babel-eslint'
690798
}, {
691799
code: `
692800
class Test extends React.Component {
@@ -1652,5 +1760,85 @@ const Component = () => (
16521760
errors: [
16531761
{message: 'Expected indentation of 2 tab characters but found 0.'}
16541762
]
1763+
}, {
1764+
code: [
1765+
'<span>',
1766+
' {do {',
1767+
' const num = rollDice();',
1768+
' <Thing num={num} />;',
1769+
' }}',
1770+
'</span>'
1771+
].join('\n'),
1772+
parser: 'babel-eslint',
1773+
output: [
1774+
'<span>',
1775+
' {do {',
1776+
' const num = rollDice();',
1777+
' <Thing num={num} />;',
1778+
' }}',
1779+
'</span>'
1780+
].join('\n'),
1781+
errors: [
1782+
{message: 'Expected indentation of 8 space characters but found 12.'}
1783+
]
1784+
}, {
1785+
code: [
1786+
'<span>',
1787+
' {(do {',
1788+
' const num = rollDice();',
1789+
' <Thing num={num} />;',
1790+
' })}',
1791+
'</span>'
1792+
].join('\n'),
1793+
parser: 'babel-eslint',
1794+
output: [
1795+
'<span>',
1796+
' {(do {',
1797+
' const num = rollDice();',
1798+
' <Thing num={num} />;',
1799+
' })}',
1800+
'</span>'
1801+
].join('\n'),
1802+
errors: [
1803+
{message: 'Expected indentation of 8 space characters but found 12.'}
1804+
]
1805+
}, {
1806+
code: [
1807+
'<span>',
1808+
' {do {',
1809+
' <Thing num={getPurposeOfLife()} />;',
1810+
' }}',
1811+
'</span>'
1812+
].join('\n'),
1813+
parser: 'babel-eslint',
1814+
output: [
1815+
'<span>',
1816+
' {do {',
1817+
' <Thing num={getPurposeOfLife()} />;',
1818+
' }}',
1819+
'</span>'
1820+
].join('\n'),
1821+
errors: [
1822+
{message: 'Expected indentation of 8 space characters but found 4.'}
1823+
]
1824+
}, {
1825+
code: [
1826+
'<span>',
1827+
' {(do {',
1828+
' <Thing num={getPurposeOfLife()} />;',
1829+
' })}',
1830+
'</span>'
1831+
].join('\n'),
1832+
parser: 'babel-eslint',
1833+
output: [
1834+
'<span>',
1835+
' {(do {',
1836+
' <Thing num={getPurposeOfLife()} />;',
1837+
' })}',
1838+
'</span>'
1839+
].join('\n'),
1840+
errors: [
1841+
{message: 'Expected indentation of 8 space characters but found 4.'}
1842+
]
16551843
}]
16561844
});

0 commit comments

Comments
 (0)