From f1f9c90bff65185d3b3f8e8611889893a934e7a0 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 17 Nov 2024 12:57:03 -0500 Subject: [PATCH 1/8] Add test case for constructor property promotion with namespace type --- .../fixtures/ClassWithMembersFixture.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php b/Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php index de49953..7cce4bc 100644 --- a/Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php +++ b/Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php @@ -222,3 +222,13 @@ 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, + ) {} +} From 843b4d69b5c16d321bea5b85f281f60fdaeca30d Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 17 Nov 2024 14:19:00 -0500 Subject: [PATCH 2/8] Simplify isConstructorPromotion and add ignore typehints --- VariableAnalysis/Lib/Helpers.php | 99 +++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 34 deletions(-) diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index e08a296..b9d7f84 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -1535,6 +1535,7 @@ 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; @@ -1542,52 +1543,82 @@ public static function isConstructorPromotion(File $phpcsFile, $stackPtr) $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 (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(); + $token = $tokens[$stackPtr]; - // 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)) { + if (! self::isTokenPossiblyPartOfTypehint($phpcsFile, $stackPtr)) { return false; } - $prev3Token = $tokens[$prev3Index]; - $wasPreviousReadonly = $prevToken['content'] === 'readonly' || $prev2Token['content'] === 'readonly'; - if (in_array($prev3Token['code'], Tokens::$scopeModifiers, true) && $wasPreviousReadonly) { - return true; - } + // 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 false; } From 922564f24f11ab0e65719e93d6d41c70e69f633e Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 17 Nov 2024 14:23:52 -0500 Subject: [PATCH 3/8] Add test for union properties --- Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php b/Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php index 7cce4bc..a1c5c3e 100644 --- a/Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php +++ b/Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php @@ -230,5 +230,6 @@ public function __construct( 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, ) {} } From 90cc33c2e3068a6272164de1da2dac421006fac4 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 17 Nov 2024 14:26:22 -0500 Subject: [PATCH 4/8] Handle unions --- VariableAnalysis/Lib/Helpers.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index b9d7f84..201b37c 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -1584,6 +1584,18 @@ private static function isTokenPossiblyPartOfTypehint(File $phpcsFile, $stackPtr if ($token['code'] === T_STRING) { return true; } + if ($token['code'] === T_TRUE) { + return true; + } + if ($token['code'] === T_FALSE) { + return true; + } + if ($token['code'] === T_NULL) { + return true; + } + if ($token['type'] === 'T_TYPE_UNION') { + return true; + } if (in_array($token['code'], Tokens::$emptyTokens)) { return true; } From 73e498caabc763084ef80ac7bab23fba5388293a Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 17 Nov 2024 14:27:07 -0500 Subject: [PATCH 5/8] Style fixes --- VariableAnalysis/Lib/Helpers.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index 201b37c..7292be2 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -1572,7 +1572,8 @@ public static function isConstructorPromotion(File $phpcsFile, $stackPtr) * * @return bool */ - private static function isTokenPossiblyPartOfTypehint(File $phpcsFile, $stackPtr) { + private static function isTokenPossiblyPartOfTypehint(File $phpcsFile, $stackPtr) + { $tokens = $phpcsFile->getTokens(); $token = $tokens[$stackPtr]; if ($token['code'] === 'PHPCS_T_NULLABLE') { @@ -1610,7 +1611,8 @@ private static function isTokenPossiblyPartOfTypehint(File $phpcsFile, $stackPtr * * @return bool */ - public static function isTokenPartOfTypehint(File $phpcsFile, $stackPtr) { + public static function isTokenPartOfTypehint(File $phpcsFile, $stackPtr) + { $tokens = $phpcsFile->getTokens(); $token = $tokens[$stackPtr]; From b29321c6fa755ad920e1a3c11635a93794c5d99d Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 17 Nov 2024 14:27:38 -0500 Subject: [PATCH 6/8] Remove unused var --- VariableAnalysis/Lib/Helpers.php | 1 - 1 file changed, 1 deletion(-) diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index 7292be2..1e34da9 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -1614,7 +1614,6 @@ private static function isTokenPossiblyPartOfTypehint(File $phpcsFile, $stackPtr public static function isTokenPartOfTypehint(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - $token = $tokens[$stackPtr]; if (! self::isTokenPossiblyPartOfTypehint($phpcsFile, $stackPtr)) { return false; From b776232e504d557a01bc1baaffe5a5574a2b0078 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 17 Nov 2024 14:28:35 -0500 Subject: [PATCH 7/8] Remove unreachable return --- VariableAnalysis/Lib/Helpers.php | 1 - 1 file changed, 1 deletion(-) diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index 1e34da9..a50019b 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -1632,7 +1632,6 @@ public static function isTokenPartOfTypehint(File $phpcsFile, $stackPtr) return ($tokens[$i]['code'] === T_VARIABLE); } } - return false; } /** From 54322acccb090e9076e64c1632f7525b44953627 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 17 Nov 2024 14:35:47 -0500 Subject: [PATCH 8/8] Use union token type content rather than type --- VariableAnalysis/Lib/Helpers.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index a50019b..dfcdc38 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -1594,7 +1594,7 @@ private static function isTokenPossiblyPartOfTypehint(File $phpcsFile, $stackPtr if ($token['code'] === T_NULL) { return true; } - if ($token['type'] === 'T_TYPE_UNION') { + if ($token['content'] === '|') { return true; } if (in_array($token['code'], Tokens::$emptyTokens)) {