Skip to content

Commit 1347c7f

Browse files
Kenneth-KTstefanwullems
Kenneth-KT
authored and
stefanwullems
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 74342fa commit 1347c7f

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
@@ -199,6 +199,77 @@ module.exports = {
199199
);
200200
}
201201

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

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

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,114 @@ ruleTester.run('jsx-indent', rule, {
690690
].join('\n'),
691691
parser: parsers.BABEL_ESLINT,
692692
options: [2]
693+
}, {
694+
code: [
695+
'<span>',
696+
' {do {',
697+
' const num = rollDice();',
698+
' <Thing num={num} />;',
699+
' }}',
700+
'</span>'
701+
].join('\n'),
702+
parser: parsers.BABEL_ESLINT
703+
}, {
704+
code: [
705+
'<span>',
706+
' {(do {',
707+
' const num = rollDice();',
708+
' <Thing num={num} />;',
709+
' })}',
710+
'</span>'
711+
].join('\n'),
712+
parser: parsers.BABEL_ESLINT
713+
}, {
714+
code: [
715+
'<span>',
716+
' {do {',
717+
' const purposeOfLife = getPurposeOfLife();',
718+
' if (purposeOfLife == 42) {',
719+
' <Thing />;',
720+
' } else {',
721+
' <AnotherThing />;',
722+
' }',
723+
' }}',
724+
'</span>'
725+
].join('\n'),
726+
parser: parsers.BABEL_ESLINT
727+
}, {
728+
code: [
729+
'<span>',
730+
' {(do {',
731+
' const purposeOfLife = getPurposeOfLife();',
732+
' if (purposeOfLife == 42) {',
733+
' <Thing />;',
734+
' } else {',
735+
' <AnotherThing />;',
736+
' }',
737+
' })}',
738+
'</span>'
739+
].join('\n'),
740+
parser: parsers.BABEL_ESLINT
741+
}, {
742+
code: [
743+
'<span>',
744+
' {do {',
745+
' <Thing num={rollDice()} />;',
746+
' }}',
747+
'</span>'
748+
].join('\n'),
749+
parser: parsers.BABEL_ESLINT
750+
}, {
751+
code: [
752+
'<span>',
753+
' {(do {',
754+
' <Thing num={rollDice()} />;',
755+
' })}',
756+
'</span>'
757+
].join('\n'),
758+
parser: parsers.BABEL_ESLINT
759+
}, {
760+
code: [
761+
'<span>',
762+
' {do {',
763+
' <Thing num={rollDice()} />;',
764+
' <Thing num={rollDice()} />;',
765+
' }}',
766+
'</span>'
767+
].join('\n'),
768+
parser: parsers.BABEL_ESLINT
769+
}, {
770+
code: [
771+
'<span>',
772+
' {(do {',
773+
' <Thing num={rollDice()} />;',
774+
' <Thing num={rollDice()} />;',
775+
' })}',
776+
'</span>'
777+
].join('\n'),
778+
parser: parsers.BABEL_ESLINT
779+
}, {
780+
code: [
781+
'<span>',
782+
' {do {',
783+
' const purposeOfLife = 42;',
784+
' <Thing num={purposeOfLife} />;',
785+
' <Thing num={purposeOfLife} />;',
786+
' }}',
787+
'</span>'
788+
].join('\n'),
789+
parser: parsers.BABEL_ESLINT
790+
}, {
791+
code: [
792+
'<span>',
793+
' {(do {',
794+
' const purposeOfLife = 42;',
795+
' <Thing num={purposeOfLife} />;',
796+
' <Thing num={purposeOfLife} />;',
797+
' })}',
798+
'</span>'
799+
].join('\n'),
800+
parser: parsers.BABEL_ESLINT
693801
}, {
694802
code: `
695803
class Test extends React.Component {
@@ -1695,5 +1803,85 @@ const Component = () => (
16951803
errors: [
16961804
{message: 'Expected indentation of 12 space characters but found 10.'}
16971805
]
1806+
}, {
1807+
code: [
1808+
'<span>',
1809+
' {do {',
1810+
' const num = rollDice();',
1811+
' <Thing num={num} />;',
1812+
' }}',
1813+
'</span>'
1814+
].join('\n'),
1815+
parser: parsers.BABEL_ESLINT,
1816+
output: [
1817+
'<span>',
1818+
' {do {',
1819+
' const num = rollDice();',
1820+
' <Thing num={num} />;',
1821+
' }}',
1822+
'</span>'
1823+
].join('\n'),
1824+
errors: [
1825+
{message: 'Expected indentation of 8 space characters but found 12.'}
1826+
]
1827+
}, {
1828+
code: [
1829+
'<span>',
1830+
' {(do {',
1831+
' const num = rollDice();',
1832+
' <Thing num={num} />;',
1833+
' })}',
1834+
'</span>'
1835+
].join('\n'),
1836+
parser: parsers.BABEL_ESLINT,
1837+
output: [
1838+
'<span>',
1839+
' {(do {',
1840+
' const num = rollDice();',
1841+
' <Thing num={num} />;',
1842+
' })}',
1843+
'</span>'
1844+
].join('\n'),
1845+
errors: [
1846+
{message: 'Expected indentation of 8 space characters but found 12.'}
1847+
]
1848+
}, {
1849+
code: [
1850+
'<span>',
1851+
' {do {',
1852+
' <Thing num={getPurposeOfLife()} />;',
1853+
' }}',
1854+
'</span>'
1855+
].join('\n'),
1856+
parser: parsers.BABEL_ESLINT,
1857+
output: [
1858+
'<span>',
1859+
' {do {',
1860+
' <Thing num={getPurposeOfLife()} />;',
1861+
' }}',
1862+
'</span>'
1863+
].join('\n'),
1864+
errors: [
1865+
{message: 'Expected indentation of 8 space characters but found 4.'}
1866+
]
1867+
}, {
1868+
code: [
1869+
'<span>',
1870+
' {(do {',
1871+
' <Thing num={getPurposeOfLife()} />;',
1872+
' })}',
1873+
'</span>'
1874+
].join('\n'),
1875+
parser: parsers.BABEL_ESLINT,
1876+
output: [
1877+
'<span>',
1878+
' {(do {',
1879+
' <Thing num={getPurposeOfLife()} />;',
1880+
' })}',
1881+
'</span>'
1882+
].join('\n'),
1883+
errors: [
1884+
{message: 'Expected indentation of 8 space characters but found 4.'}
1885+
]
16981886
}]
16991887
});

0 commit comments

Comments
 (0)