Skip to content

Commit

Permalink
Support constructor promotion with namespaced and union typehints (#333)
Browse files Browse the repository at this point in the history
* Add test case for constructor property promotion with namespace type

* Simplify isConstructorPromotion and add ignore typehints

* Add test for union properties

* Handle unions

* Style fixes

* Remove unused var

* Remove unreachable return

* Use union token type content rather than type
  • Loading branch information
sirbrillig authored Nov 17, 2024
1 parent 40d4800 commit 779884c
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 35 deletions.
11 changes: 11 additions & 0 deletions Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,14 @@ public function getMessage(): string {
return $this->message;
}
}

class ClassWithNamespacedConstructorPropertyPromotion
{
public function __construct(
public \App\Models\User $user,
public readonly \App\Models\Blog $blog,
private \App\Models\Game $game,
protected ?\App\Models\Flag $flag,
protected true|false|int|string|null|\App\Models\Favorite $favorite,
) {}
}
113 changes: 78 additions & 35 deletions VariableAnalysis/Lib/Helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -1535,62 +1535,105 @@ public static function getForLoopForIncrementVariable($stackPtr, $forLoops)
*/
public static function isConstructorPromotion(File $phpcsFile, $stackPtr)
{
// If we are not in a function's parameters, this is not promotion.
$functionIndex = self::getFunctionIndexForFunctionParameter($phpcsFile, $stackPtr);
if (! $functionIndex) {
return false;
}

$tokens = $phpcsFile->getTokens();

// If the previous token is a visibility keyword, this is constructor
// promotion. eg: `public $foobar`.
$prevIndex = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), $functionIndex, true);
if (! is_int($prevIndex)) {
// Move backwards from the token, ignoring whitespace, typehints, and the
// 'readonly' keyword, and return true if the previous token is a
// visibility keyword (eg: `public`).
for ($i = $stackPtr - 1; $i > $functionIndex; $i--) {
if (in_array($tokens[$i]['code'], Tokens::$scopeModifiers, true)) {
return true;
}
if (in_array($tokens[$i]['code'], Tokens::$emptyTokens, true)) {
continue;
}
if ($tokens[$i]['content'] === 'readonly') {
continue;
}
if (self::isTokenPartOfTypehint($phpcsFile, $i)) {
continue;
}
return false;
}
$prevToken = $tokens[$prevIndex];
if (in_array($prevToken['code'], Tokens::$scopeModifiers, true)) {
return false;
}

/**
* Return false if the token is definitely not part of a typehint
*
* @param File $phpcsFile
* @param int $stackPtr
*
* @return bool
*/
private static function isTokenPossiblyPartOfTypehint(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();
$token = $tokens[$stackPtr];
if ($token['code'] === 'PHPCS_T_NULLABLE') {
return true;
}

// If the previous token is not a visibility keyword, but the one before it
// is, the previous token was probably a typehint and this is constructor
// promotion. eg: `public boolean $foobar`.
$prev2Index = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prevIndex - 1), $functionIndex, true);
if (! is_int($prev2Index)) {
return false;
if ($token['code'] === T_NS_SEPARATOR) {
return true;
}
$prev2Token = $tokens[$prev2Index];
// If the token that might be a visibility keyword is a nullable typehint,
// ignore it and move back one token further eg: `public ?boolean $foobar`.
if ($prev2Token['code'] === 'PHPCS_T_NULLABLE') {
$prev2Index = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev2Index - 1), $functionIndex, true);
if (! is_int($prev2Index)) {
return false;
}
if ($token['code'] === T_STRING) {
return true;
}
$prev2Token = $tokens[$prev2Index];
if (in_array($prev2Token['code'], Tokens::$scopeModifiers, true)) {
if ($token['code'] === T_TRUE) {
return true;
}

// If the previous token is not a visibility keyword, but the one two
// before it is, and one of the tokens is `readonly`, the previous token
// was probably a typehint and this is constructor promotion. eg: `public
// readonly boolean $foobar`.
$prev3Index = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev2Index - 1), $functionIndex, true);
if (! is_int($prev3Index)) {
return false;
if ($token['code'] === T_FALSE) {
return true;
}
$prev3Token = $tokens[$prev3Index];
$wasPreviousReadonly = $prevToken['content'] === 'readonly' || $prev2Token['content'] === 'readonly';
if (in_array($prev3Token['code'], Tokens::$scopeModifiers, true) && $wasPreviousReadonly) {
if ($token['code'] === T_NULL) {
return true;
}
if ($token['content'] === '|') {
return true;
}
if (in_array($token['code'], Tokens::$emptyTokens)) {
return true;
}

return false;
}

/**
* Return true if the token is inside a typehint
*
* @param File $phpcsFile
* @param int $stackPtr
*
* @return bool
*/
public static function isTokenPartOfTypehint(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

if (! self::isTokenPossiblyPartOfTypehint($phpcsFile, $stackPtr)) {
return false;
}

// Examine every following token, ignoring everything that might be part of
// a typehint. If we find a variable at the end, this is part of a
// typehint.
$i = $stackPtr;
while (true) {
$i += 1;
if (! isset($tokens[$i])) {
return false;
}
if (! self::isTokenPossiblyPartOfTypehint($phpcsFile, $i)) {
return ($tokens[$i]['code'] === T_VARIABLE);
}
}
}

/**
* Return true if the token is inside an abstract class.
*
Expand Down

0 comments on commit 779884c

Please sign in to comment.