Skip to content

Commit 97f7898

Browse files
gsherwoodgsherwood
committed
Fixed bug #3287 : Wrongly assumed indentation with match and arrays
Match checking in findStartOfStatement needed to skip over short arrays and function calls etc, so now using findEndOfStatement for that. That exposed a bug in findEndOfStatement when checking a match arrow directly, which is now also fixed.
1 parent f5d8a62 commit 97f7898

File tree

9 files changed

+100
-29
lines changed

9 files changed

+100
-29
lines changed

‎src/Files/File.php‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2354,8 +2354,8 @@ public function findStartOfStatement($start, $ignore=null)
23542354
return $next;
23552355
}
23562356

2357-
$nextComma = $this->findNext(T_COMMA, ($prevMatchArrow + 1));
2358-
$next = $this->findNext(Util\Tokens::$emptyTokens, ($nextComma + 1), null, true);
2357+
$end = $this->findEndOfStatement($prevMatchArrow);
2358+
$next = $this->findNext(Util\Tokens::$emptyTokens, ($end + 1), null, true);
23592359
return $next;
23602360
}
23612361
}//end if
@@ -2459,26 +2459,28 @@ public function findEndOfStatement($start, $ignore=null)
24592459
// If the start token is inside the case part of a match expression,
24602460
// advance to the match arrow and continue looking for the
24612461
// end of the statement from there so that we skip over commas.
2462-
$matchExpression = $this->getCondition($start, T_MATCH);
2463-
if ($matchExpression !== false) {
2464-
$beforeArrow = true;
2465-
$prevMatchArrow = $this->findPrevious(T_MATCH_ARROW, ($start - 1), $this->tokens[$matchExpression]['scope_opener']);
2466-
if ($prevMatchArrow !== false) {
2467-
$prevComma = $this->findNext(T_COMMA, ($prevMatchArrow + 1), $start);
2468-
if ($prevComma === false) {
2469-
// No comma between this token and the last match arrow,
2470-
// so this token exists after the arrow and we can continue
2471-
// checking as normal.
2472-
$beforeArrow = false;
2462+
if ($this->tokens[$start]['code'] !== T_MATCH_ARROW) {
2463+
$matchExpression = $this->getCondition($start, T_MATCH);
2464+
if ($matchExpression !== false) {
2465+
$beforeArrow = true;
2466+
$prevMatchArrow = $this->findPrevious(T_MATCH_ARROW, ($start - 1), $this->tokens[$matchExpression]['scope_opener']);
2467+
if ($prevMatchArrow !== false) {
2468+
$prevComma = $this->findNext(T_COMMA, ($prevMatchArrow + 1), $start);
2469+
if ($prevComma === false) {
2470+
// No comma between this token and the last match arrow,
2471+
// so this token exists after the arrow and we can continue
2472+
// checking as normal.
2473+
$beforeArrow = false;
2474+
}
24732475
}
2474-
}
24752476

2476-
if ($beforeArrow === true) {
2477-
$nextMatchArrow = $this->findNext(T_MATCH_ARROW, ($start + 1), $this->tokens[$matchExpression]['scope_closer']);
2478-
if ($nextMatchArrow !== false) {
2479-
$start = $nextMatchArrow;
2477+
if ($beforeArrow === true) {
2478+
$nextMatchArrow = $this->findNext(T_MATCH_ARROW, ($start + 1), $this->tokens[$matchExpression]['scope_closer']);
2479+
if ($nextMatchArrow !== false) {
2480+
$start = $nextMatchArrow;
2481+
}
24802482
}
2481-
}
2483+
}//end if
24822484
}//end if
24832485

24842486
$lastNotEmpty = $start;

‎src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,6 +1533,20 @@ $list = [
15331533
}
15341534
];
15351535

1536+
$foo = match ($type) {
1537+
'a' => [
1538+
'aa' => 'DESC',
1539+
'ab' => 'DESC',
1540+
],
1541+
'b' => [
1542+
'ba' => 'DESC',
1543+
'bb' => 'DESC',
1544+
],
1545+
default => [
1546+
'da' => 'DESC',
1547+
],
1548+
};
1549+
15361550
/* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
15371551
?>
15381552

‎src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,6 +1533,20 @@ $list = [
15331533
}
15341534
];
15351535

1536+
$foo = match ($type) {
1537+
'a' => [
1538+
'aa' => 'DESC',
1539+
'ab' => 'DESC',
1540+
],
1541+
'b' => [
1542+
'ba' => 'DESC',
1543+
'bb' => 'DESC',
1544+
],
1545+
default => [
1546+
'da' => 'DESC',
1547+
],
1548+
};
1549+
15361550
/* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
15371551
?>
15381552

‎src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,6 +1533,20 @@ $list = [
15331533
}
15341534
];
15351535

1536+
$foo = match ($type) {
1537+
'a' => [
1538+
'aa' => 'DESC',
1539+
'ab' => 'DESC',
1540+
],
1541+
'b' => [
1542+
'ba' => 'DESC',
1543+
'bb' => 'DESC',
1544+
],
1545+
default => [
1546+
'da' => 'DESC',
1547+
],
1548+
};
1549+
15361550
/* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
15371551
?>
15381552

‎src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,6 +1533,20 @@ $list = [
15331533
}
15341534
];
15351535

1536+
$foo = match ($type) {
1537+
'a' => [
1538+
'aa' => 'DESC',
1539+
'ab' => 'DESC',
1540+
],
1541+
'b' => [
1542+
'ba' => 'DESC',
1543+
'bb' => 'DESC',
1544+
],
1545+
default => [
1546+
'da' => 'DESC',
1547+
],
1548+
};
1549+
15361550
/* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
15371551
?>
15381552

‎src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,10 @@ public function getErrorList($testFile='ScopeIndentUnitTest.inc')
187187
1527 => 1,
188188
1529 => 1,
189189
1530 => 1,
190-
1544 => 1,
191-
1545 => 1,
192-
1546 => 1,
193-
1547 => 1,
190+
1558 => 1,
191+
1559 => 1,
192+
1560 => 1,
193+
1561 => 1,
194194
];
195195

196196
}//end getErrorList()

‎tests/Core/File/FindEndOfStatementTest.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,12 @@ public function testMatchMultipleCase()
286286
{
287287
$start = $this->getTargetToken('/* testMatchMultipleCase */', T_LNUMBER);
288288
$found = self::$phpcsFile->findEndOfStatement($start);
289-
290289
$this->assertSame(($start + 13), $found);
291290

291+
$start += 6;
292+
$found = self::$phpcsFile->findEndOfStatement($start);
293+
$this->assertSame(($start + 7), $found);
294+
292295
}//end testMatchMultipleCase()
293296

294297

‎tests/Core/File/FindStartOfStatementTest.inc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ $result = match ($key) {
9494
/* testMatchArray */
9595
$result = match ($key) {
9696
1 => [1,2,3],
97-
2 => [1 => one(), 2 => two()],
97+
2 => [1 => one($a, $b), 2 => two($b, $c)],
98+
3 => [],
9899
};
99100

100101
/* testNestedMatch */

‎tests/Core/File/FindStartOfStatementTest.php‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -419,16 +419,25 @@ public function testMatchClosure()
419419
*/
420420
public function testMatchArray()
421421
{
422-
$start = $this->getTargetToken('/* testMatchArray */', T_LNUMBER);
422+
// Start of first case statement.
423+
$start = $this->getTargetToken('/* testMatchArray */', T_LNUMBER);
424+
$found = self::$phpcsFile->findStartOfStatement($start);
425+
$this->assertSame($start, $found);
426+
427+
// Comma after first statement.
423428
$start += 11;
424429
$found = self::$phpcsFile->findStartOfStatement($start);
425-
426430
$this->assertSame(($start - 7), $found);
427431

428-
$start += 25;
432+
// Start of second case statement.
433+
$start += 3;
429434
$found = self::$phpcsFile->findStartOfStatement($start);
435+
$this->assertSame($start, $found);
430436

431-
$this->assertSame(($start - 18), $found);
437+
// Comma after first statement.
438+
$start += 30;
439+
$found = self::$phpcsFile->findStartOfStatement($start);
440+
$this->assertSame(($start - 26), $found);
432441

433442
}//end testMatchArray()
434443

0 commit comments

Comments
 (0)