diff --git a/src/Standards/Generic/Sniffs/PHP/LowerCaseConstantSniff.php b/src/Standards/Generic/Sniffs/PHP/LowerCaseConstantSniff.php index 4376daa9dd..7a8605a7c2 100644 --- a/src/Standards/Generic/Sniffs/PHP/LowerCaseConstantSniff.php +++ b/src/Standards/Generic/Sniffs/PHP/LowerCaseConstantSniff.php @@ -37,6 +37,29 @@ class LowerCaseConstantSniff implements Sniff T_NULL => T_NULL, ]; + /** + * Token types which can be encountered in a property type declaration. + * + * @var array + */ + private $propertyTypeTokens = [ + T_CALLABLE => T_CALLABLE, + T_SELF => T_SELF, + T_PARENT => T_PARENT, + T_FALSE => T_FALSE, + T_TRUE => T_TRUE, + T_NULL => T_NULL, + T_STRING => T_STRING, + T_NAME_QUALIFIED => T_NAME_QUALIFIED, + T_NAME_FULLY_QUALIFIED => T_NAME_FULLY_QUALIFIED, + T_NAME_RELATIVE => T_NAME_RELATIVE, + T_NS_SEPARATOR => T_NS_SEPARATOR, + T_NAMESPACE => T_NAMESPACE, + T_TYPE_UNION => T_TYPE_UNION, + T_TYPE_INTERSECTION => T_TYPE_INTERSECTION, + T_NULLABLE => T_NULLABLE, + ]; + /** * Returns an array of tokens this test wants to listen for. @@ -47,7 +70,13 @@ public function register() { $targets = $this->targets; - // Register function keywords to filter out type declarations. + // Register scope modifiers to filter out property type declarations. + $targets += Tokens::$scopeModifiers; + $targets[] = T_VAR; + $targets[] = T_STATIC; + $targets[] = T_READONLY; + + // Register function keywords to filter out param/return type declarations. $targets[] = T_FUNCTION; $targets[] = T_CLOSURE; $targets[] = T_FN; @@ -64,12 +93,43 @@ public function register() * @param int $stackPtr The position of the current token in the * stack passed in $tokens. * - * @return void + * @return void|int Optionally returns a stack pointer. The sniff will not be + * called again on the current file until the returned stack + * pointer is reached. */ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); + /* + * Skip over type declarations for properties. + * + * Note: for other uses of the visibility modifiers (functions, constants, trait use), + * nothing relevant will be skipped as the next non-empty token will be an "non-skippable" + * one. + * Functions are handled separately below (and then skip to their scope opener), so + * this should also not cause any confusion for constructor property promotion. + * + * For other uses of the "static" keyword, it also shouldn't be problematic as the only + * time the next non-empty token will be a "skippable" token will be in return type + * declarations, in which case, it is correct to skip over them. + */ + + if (isset(Tokens::$scopeModifiers[$tokens[$stackPtr]['code']]) === true + || $tokens[$stackPtr]['code'] === T_VAR + || $tokens[$stackPtr]['code'] === T_STATIC + || $tokens[$stackPtr]['code'] === T_READONLY + ) { + $skipOver = (Tokens::$emptyTokens + $this->propertyTypeTokens); + $skipTo = $phpcsFile->findNext($skipOver, ($stackPtr + 1), null, true); + if ($skipTo !== false) { + return $skipTo; + } + + // If we're at the end of the file, just return. + return; + } + // Handle function declarations separately as they may contain the keywords in type declarations. if ($tokens[$stackPtr]['code'] === T_FUNCTION || $tokens[$stackPtr]['code'] === T_CLOSURE @@ -79,9 +139,15 @@ public function process(File $phpcsFile, $stackPtr) return; } + // Make sure to skip over return type declarations. $end = $tokens[$stackPtr]['parenthesis_closer']; if (isset($tokens[$stackPtr]['scope_opener']) === true) { $end = $tokens[$stackPtr]['scope_opener']; + } else { + $skipTo = $phpcsFile->findNext([T_SEMICOLON, T_OPEN_CURLY_BRACKET], ($end + 1), null, false, null, true); + if ($skipTo !== false) { + $end = $skipTo; + } } // Do a quick check if any of the targets exist in the declaration. @@ -114,21 +180,6 @@ public function process(File $phpcsFile, $stackPtr) return $end; }//end if - // Handle property declarations separately as they may contain the keywords in type declarations. - if (isset($tokens[$stackPtr]['conditions']) === true) { - $conditions = $tokens[$stackPtr]['conditions']; - $lastCondition = end($conditions); - if (isset(Tokens::$ooScopeTokens[$lastCondition]) === true) { - // This can only be an OO constant or property declaration as methods are handled above. - $equals = $phpcsFile->findPrevious(T_EQUAL, ($stackPtr - 1), null, false, null, true); - if ($equals !== false) { - $this->processConstant($phpcsFile, $stackPtr); - } - - return; - } - } - // Handle everything else. $this->processConstant($phpcsFile, $stackPtr); diff --git a/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc b/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc index c8c7754edf..5dfb75560a 100644 --- a/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc +++ b/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc @@ -107,3 +107,43 @@ interface InterfaceMethodsWithReturnTypeNoScopeOpener { abstract class ClassMethodsWithReturnTypeNoScopeOpener { abstract public function typed($param = FALSE) : TRUE; } + +// Additional tests to safeguard improved property type skip logic. +readonly class Properties { + use SomeTrait { + sayHello as private myPrivateHello; + } + + public Type|FALSE|NULL $propertyA = array( + 'itemA' => TRUE, + 'itemB' => FALSE, + 'itemC' => NULL, + ), $propertyB = FALSE; + + protected \FullyQualified&Partially\Qualified&namespace\Relative $propertyC; + var ?TRUE $propertyD; + static array|callable|FALSE|self|parent $propertyE = TRUE; + private + // phpcs:ignore Stnd.Cat.Sniff -- for reasons. + TRUE /*comment*/ + $propertyF = TRUE; + + public function __construct( + public FALSE|NULL $promotedPropA, + readonly callable|TRUE $promotedPropB, + ) { + static $var; + echo static::class; + static::foo(); + $var = $var instanceof static; + $obj = new static(); + } + + public static function foo(): static|self|FALSE { + $callable = static function() {}; + } +} + +// Last coding/parse error. +// This has to be the last test in the file. +function UnclosedCurly (): FALSE { diff --git a/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc.fixed b/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc.fixed index cbb864e5fa..6b999cc456 100644 --- a/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc.fixed +++ b/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc.fixed @@ -107,3 +107,43 @@ interface InterfaceMethodsWithReturnTypeNoScopeOpener { abstract class ClassMethodsWithReturnTypeNoScopeOpener { abstract public function typed($param = false) : TRUE; } + +// Additional tests to safeguard improved property type skip logic. +readonly class Properties { + use SomeTrait { + sayHello as private myPrivateHello; + } + + public Type|FALSE|NULL $propertyA = array( + 'itemA' => true, + 'itemB' => false, + 'itemC' => null, + ), $propertyB = false; + + protected \FullyQualified&Partially\Qualified&namespace\Relative $propertyC; + var ?TRUE $propertyD; + static array|callable|FALSE|self|parent $propertyE = true; + private + // phpcs:ignore Stnd.Cat.Sniff -- for reasons. + TRUE /*comment*/ + $propertyF = true; + + public function __construct( + public FALSE|NULL $promotedPropA, + readonly callable|TRUE $promotedPropB, + ) { + static $var; + echo static::class; + static::foo(); + $var = $var instanceof static; + $obj = new static(); + } + + public static function foo(): static|self|FALSE { + $callable = static function() {}; + } +} + +// Last coding/parse error. +// This has to be the last test in the file. +function UnclosedCurly (): FALSE { diff --git a/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.php b/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.php index d3d4d0a192..891bc9f0dd 100644 --- a/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.php +++ b/src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.php @@ -53,6 +53,12 @@ public function getErrorList($testFile='LowerCaseConstantUnitTest.inc') 100 => 2, 104 => 1, 108 => 1, + 118 => 1, + 119 => 1, + 120 => 1, + 121 => 1, + 125 => 1, + 129 => 1, ]; break; case 'LowerCaseConstantUnitTest.js':