Skip to content

File::findStartOfStatement(): 3 bug fixes related to match expressions #502

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 21, 2024
Merged

File::findStartOfStatement(): 3 bug fixes related to match expressions #502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
467d284
File::findStartOfStatement(): bug fix - don't give nested scopes the …
jrfnl May 19, 2024
b82438f
File::findStartOfStatement(): bug fix - don't give tokens within nest…
jrfnl May 19, 2024
d3abcd6
File::findStartOfStatement(): bug fix - don't confuse comma's in shor…
jrfnl May 19, 2024
28c376e
Generic/ScopeIndent: add tests for issues 110 and 437
jrfnl May 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 77 additions & 40 deletions src/Files/File.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2435,51 +2435,88 @@ public function findStartOfStatement($start, $ignore=null)
// If the start token is inside the case part of a match expression,
// find the start of the condition. If it's in the statement part, find
// the token that comes after the match arrow.
$matchExpression = $this->getCondition($start, T_MATCH);
if ($matchExpression !== false) {
for ($prevMatch = $start; $prevMatch > $this->tokens[$matchExpression]['scope_opener']; $prevMatch--) {
if ($prevMatch !== $start
&& ($this->tokens[$prevMatch]['code'] === T_MATCH_ARROW
|| $this->tokens[$prevMatch]['code'] === T_COMMA)
) {
break;
}
if (empty($this->tokens[$start]['conditions']) === false) {
$conditions = $this->tokens[$start]['conditions'];
$lastConditionOwner = end($conditions);
$matchExpression = key($conditions);

if ($lastConditionOwner === T_MATCH
// Check if the $start token is at the same parentheses nesting level as the match token.
&& ((empty($this->tokens[$matchExpression]['nested_parenthesis']) === true
&& empty($this->tokens[$start]['nested_parenthesis']) === true)
|| ((empty($this->tokens[$matchExpression]['nested_parenthesis']) === false
&& empty($this->tokens[$start]['nested_parenthesis']) === false)
&& $this->tokens[$matchExpression]['nested_parenthesis'] === $this->tokens[$start]['nested_parenthesis']))
) {
// Walk back to the previous match arrow (if it exists).
$lastComma = null;
$inNestedExpression = false;
for ($prevMatch = $start; $prevMatch > $this->tokens[$matchExpression]['scope_opener']; $prevMatch--) {
if ($prevMatch !== $start && $this->tokens[$prevMatch]['code'] === T_MATCH_ARROW) {
break;
}

// Skip nested statements.
if (isset($this->tokens[$prevMatch]['bracket_opener']) === true
&& $prevMatch === $this->tokens[$prevMatch]['bracket_closer']
) {
$prevMatch = $this->tokens[$prevMatch]['bracket_opener'];
} else if (isset($this->tokens[$prevMatch]['parenthesis_opener']) === true
&& $prevMatch === $this->tokens[$prevMatch]['parenthesis_closer']
) {
$prevMatch = $this->tokens[$prevMatch]['parenthesis_opener'];
}
}
if ($prevMatch !== $start && $this->tokens[$prevMatch]['code'] === T_COMMA) {
$lastComma = $prevMatch;
continue;
}

if ($prevMatch <= $this->tokens[$matchExpression]['scope_opener']) {
// We're before the arrow in the first case.
$next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true);
if ($next === false) {
return $start;
}
// Skip nested statements.
if (isset($this->tokens[$prevMatch]['bracket_opener']) === true
&& $prevMatch === $this->tokens[$prevMatch]['bracket_closer']
) {
$prevMatch = $this->tokens[$prevMatch]['bracket_opener'];
continue;
}

return $next;
}
if (isset($this->tokens[$prevMatch]['parenthesis_opener']) === true
&& $prevMatch === $this->tokens[$prevMatch]['parenthesis_closer']
) {
$prevMatch = $this->tokens[$prevMatch]['parenthesis_opener'];
continue;
}

if ($this->tokens[$prevMatch]['code'] === T_COMMA) {
// We're before the arrow, but not in the first case.
$prevMatchArrow = $this->findPrevious(T_MATCH_ARROW, ($prevMatch - 1), $this->tokens[$matchExpression]['scope_opener']);
if ($prevMatchArrow === false) {
// We're before the arrow in the first case.
$next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true);
return $next;
}
// Stop if we're _within_ a nested short array statement, which may contain comma's too.
// No need to deal with parentheses, those are handled above via the `nested_parenthesis` checks.
if (isset($this->tokens[$prevMatch]['bracket_opener']) === true
&& $this->tokens[$prevMatch]['bracket_closer'] > $start
) {
$inNestedExpression = true;
break;
}
}//end for

if ($inNestedExpression === false) {
// $prevMatch will now either be the scope opener or a match arrow.
// If it is the scope opener, go the first non-empty token after. $start will have been part of the first condition.
if ($prevMatch <= $this->tokens[$matchExpression]['scope_opener']) {
// We're before the arrow in the first case.
$next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true);
if ($next === false) {
// Shouldn't be possible.
return $start;
}

$end = $this->findEndOfStatement($prevMatchArrow);
$next = $this->findNext(Tokens::$emptyTokens, ($end + 1), null, true);
return $next;
}
return $next;
}

// Okay, so we found a match arrow.
// If $start was part of the "next" condition, the last comma will be set.
// Otherwise, $start must have been part of a return expression.
if (isset($lastComma) === true && $lastComma > $prevMatch) {
$prevMatch = $lastComma;
}

// In both cases, go to the first non-empty token after.
$next = $this->findNext(Tokens::$emptyTokens, ($prevMatch + 1), null, true);
if ($next === false) {
// Shouldn't be possible.
return $start;
}

return $next;
}//end if
}//end if
}//end if

$lastNotEmpty = $start;
Expand Down
35 changes: 35 additions & 0 deletions src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,41 @@ foo(function ($foo) {
];
});

// Issue #110.
echo match (1) {
0 => match (2) {
2 => match (3) {
3 => 3,
default => -1,
},
},
1 => match (2) {
1 => match (3) {
3 => 3,
default => -1,
},
2 => match (3) {
3 => 3,
default => -1,
},
},
};

// Issue #437.
match (true) {
default => [
'unrelated' => '',
'example' => array_filter(
array_map(
function () {
return null;
},
[]
)
)
]
};

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

Expand Down
35 changes: 35 additions & 0 deletions src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,41 @@ foo(function ($foo) {
];
});

// Issue #110.
echo match (1) {
0 => match (2) {
2 => match (3) {
3 => 3,
default => -1,
},
},
1 => match (2) {
1 => match (3) {
3 => 3,
default => -1,
},
2 => match (3) {
3 => 3,
default => -1,
},
},
};

// Issue #437.
match (true) {
default => [
'unrelated' => '',
'example' => array_filter(
array_map(
function () {
return null;
},
[]
)
)
]
};

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

Expand Down
35 changes: 35 additions & 0 deletions src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,41 @@ foo(function ($foo) {
];
});

// Issue #110.
echo match (1) {
0 => match (2) {
2 => match (3) {
3 => 3,
default => -1,
},
},
1 => match (2) {
1 => match (3) {
3 => 3,
default => -1,
},
2 => match (3) {
3 => 3,
default => -1,
},
},
};

// Issue #437.
match (true) {
default => [
'unrelated' => '',
'example' => array_filter(
array_map(
function () {
return null;
},
[]
)
)
]
};

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

Expand Down
35 changes: 35 additions & 0 deletions src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,41 @@ foo(function ($foo) {
];
});

// Issue #110.
echo match (1) {
0 => match (2) {
2 => match (3) {
3 => 3,
default => -1,
},
},
1 => match (2) {
1 => match (3) {
3 => 3,
default => -1,
},
2 => match (3) {
3 => 3,
default => -1,
},
},
};

// Issue #437.
match (true) {
default => [
'unrelated' => '',
'example' => array_filter(
array_map(
function () {
return null;
},
[]
)
)
]
};

/* 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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,10 @@ public function getErrorList($testFile='')
1527 => 1,
1529 => 1,
1530 => 1,
1590 => 1,
1591 => 1,
1592 => 1,
1593 => 1,
1625 => 1,
1626 => 1,
1627 => 1,
1628 => 1,
];

}//end getErrorList()
Expand Down
36 changes: 36 additions & 0 deletions tests/Core/File/FindStartOfStatementTest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,39 @@ switch ($foo) {
/* testInsideDefaultContinueStatement */
continue $var;
}

match ($var) {
true =>
/* test437ClosureDeclaration */
function ($var) {
/* test437EchoNestedWithinClosureWithinMatch */
echo $var, 'text', PHP_EOL;
},
default => false
};

match ($var) {
/* test437NestedLongArrayWithinMatch */
'a' => array( 1, 2.5, $var),
/* test437NestedFunctionCallWithinMatch */
'b' => functionCall( 11, $var, 50.50),
/* test437NestedArrowFunctionWithinMatch */
'c' => fn($p1, /* test437FnSecondParamWithinMatch */ $p2) => $p1 + $p2,
default => false
};

callMe($paramA, match ($var) {
/* test437NestedLongArrayWithinNestedMatch */
'a' => array( 1, 2.5, $var),
/* test437NestedFunctionCallWithinNestedMatch */
'b' => functionCall( 11, $var, 50.50),
/* test437NestedArrowFunctionWithinNestedMatch */
'c' => fn($p1, /* test437FnSecondParamWithinNestedMatch */ $p2) => $p1 + $p2,
default => false
});

match ($var) {
/* test437NestedShortArrayWithinMatch */
'a' => [ 1, 2.5, $var],
default => false
};
Loading