Skip to content

Commit

Permalink
Fixed bug #3287 : Wrongly assumed indentation with match and arrays
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@gsherwood
gsherwood committed Apr 5, 2021
1 parent 03bd360 commit bffe7a5
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 29 deletions.
40 changes: 21 additions & 19 deletions src/Files/File.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2299,8 +2299,8 @@ public function findStartOfStatement($start, $ignore=null)
return $next;
}

$nextComma = $this->findNext(T_COMMA, ($prevMatchArrow + 1));
$next = $this->findNext(Tokens::$emptyTokens, ($nextComma + 1), null, true);
$end = $this->findEndOfStatement($prevMatchArrow);
$next = $this->findNext(Tokens::$emptyTokens, ($end + 1), null, true);
return $next;
}
}//end if
Expand Down Expand Up @@ -2404,26 +2404,28 @@ public function findEndOfStatement($start, $ignore=null)
// If the start token is inside the case part of a match expression,
// advance to the match arrow and continue looking for the
// end of the statement from there so that we skip over commas.
$matchExpression = $this->getCondition($start, T_MATCH);
if ($matchExpression !== false) {
$beforeArrow = true;
$prevMatchArrow = $this->findPrevious(T_MATCH_ARROW, ($start - 1), $this->tokens[$matchExpression]['scope_opener']);
if ($prevMatchArrow !== false) {
$prevComma = $this->findNext(T_COMMA, ($prevMatchArrow + 1), $start);
if ($prevComma === false) {
// No comma between this token and the last match arrow,
// so this token exists after the arrow and we can continue
// checking as normal.
$beforeArrow = false;
if ($this->tokens[$start]['code'] !== T_MATCH_ARROW) {
$matchExpression = $this->getCondition($start, T_MATCH);
if ($matchExpression !== false) {
$beforeArrow = true;
$prevMatchArrow = $this->findPrevious(T_MATCH_ARROW, ($start - 1), $this->tokens[$matchExpression]['scope_opener']);
if ($prevMatchArrow !== false) {
$prevComma = $this->findNext(T_COMMA, ($prevMatchArrow + 1), $start);
if ($prevComma === false) {
// No comma between this token and the last match arrow,
// so this token exists after the arrow and we can continue
// checking as normal.
$beforeArrow = false;
}
}
}

if ($beforeArrow === true) {
$nextMatchArrow = $this->findNext(T_MATCH_ARROW, ($start + 1), $this->tokens[$matchExpression]['scope_closer']);
if ($nextMatchArrow !== false) {
$start = $nextMatchArrow;
if ($beforeArrow === true) {
$nextMatchArrow = $this->findNext(T_MATCH_ARROW, ($start + 1), $this->tokens[$matchExpression]['scope_closer']);
if ($nextMatchArrow !== false) {
$start = $nextMatchArrow;
}
}
}
}//end if
}//end if

$lastNotEmpty = $start;
Expand Down
14 changes: 14 additions & 0 deletions src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,20 @@ $list = [
}
];

$foo = match ($type) {
'a' => [
'aa' => 'DESC',
'ab' => 'DESC',
],
'b' => [
'ba' => 'DESC',
'bb' => 'DESC',
],
default => [
'da' => 'DESC',
],
};

/* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
?>

Expand Down
14 changes: 14 additions & 0 deletions src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,20 @@ $list = [
}
];

$foo = match ($type) {
'a' => [
'aa' => 'DESC',
'ab' => 'DESC',
],
'b' => [
'ba' => 'DESC',
'bb' => 'DESC',
],
default => [
'da' => 'DESC',
],
};

/* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
?>

Expand Down
14 changes: 14 additions & 0 deletions src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,20 @@ $list = [
}
];

$foo = match ($type) {
'a' => [
'aa' => 'DESC',
'ab' => 'DESC',
],
'b' => [
'ba' => 'DESC',
'bb' => 'DESC',
],
default => [
'da' => 'DESC',
],
};

/* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
?>

Expand Down
14 changes: 14 additions & 0 deletions src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,20 @@ $list = [
}
];

$foo = match ($type) {
'a' => [
'aa' => 'DESC',
'ab' => 'DESC',
],
'b' => [
'ba' => 'DESC',
'bb' => 'DESC',
],
default => [
'da' => 'DESC',
],
};

/* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
?>

Expand Down
8 changes: 4 additions & 4 deletions src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,10 @@ public function getErrorList($testFile='ScopeIndentUnitTest.inc')
1527 => 1,
1529 => 1,
1530 => 1,
1544 => 1,
1545 => 1,
1546 => 1,
1547 => 1,
1558 => 1,
1559 => 1,
1560 => 1,
1561 => 1,
];

}//end getErrorList()
Expand Down
5 changes: 4 additions & 1 deletion tests/Core/File/FindEndOfStatementTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,12 @@ public function testMatchMultipleCase()
{
$start = $this->getTargetToken('/* testMatchMultipleCase */', T_LNUMBER);
$found = self::$phpcsFile->findEndOfStatement($start);

$this->assertSame(($start + 13), $found);

$start += 6;
$found = self::$phpcsFile->findEndOfStatement($start);
$this->assertSame(($start + 7), $found);

}//end testMatchMultipleCase()


Expand Down
3 changes: 2 additions & 1 deletion tests/Core/File/FindStartOfStatementTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ $result = match ($key) {
/* testMatchArray */
$result = match ($key) {
1 => [1,2,3],
2 => [1 => one(), 2 => two()],
2 => [1 => one($a, $b), 2 => two($b, $c)],
3 => [],
};

/* testNestedMatch */
Expand Down
17 changes: 13 additions & 4 deletions tests/Core/File/FindStartOfStatementTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,16 +419,25 @@ public function testMatchClosure()
*/
public function testMatchArray()
{
$start = $this->getTargetToken('/* testMatchArray */', T_LNUMBER);
// Start of first case statement.
$start = $this->getTargetToken('/* testMatchArray */', T_LNUMBER);
$found = self::$phpcsFile->findStartOfStatement($start);
$this->assertSame($start, $found);

// Comma after first statement.
$start += 11;
$found = self::$phpcsFile->findStartOfStatement($start);

$this->assertSame(($start - 7), $found);

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

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

}//end testMatchArray()

Expand Down

0 comments on commit bffe7a5

Please sign in to comment.