From 9bf4e15b9ddfc16d162670702f54dc09fb0345a1 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 20 Apr 2024 13:50:05 +0200 Subject: [PATCH] Generic/LowerCaseType: add support for examining DNF types The `Generic.PHP.LowerCaseType` sniff needs to be updated to also handle non-lowercase types which are part of a DNF type declaration. This commit updates the `processUnionType()` method to not only examine union types, but to examine all multi-token types and to do so in a slightly more performant manner and calls that method now for all multi-token type declarations. Note: The method name now doesn't properly cover the functionality anymore, however, renaming the method would be a breaking change as the class is not `final` and the method not `private`. Includes unit tests. Related to 105 Closes 105 --- .../Generic/Sniffs/PHP/LowerCaseTypeSniff.php | 110 +++++++++--------- .../Tests/PHP/LowerCaseTypeUnitTest.inc | 14 +++ .../Tests/PHP/LowerCaseTypeUnitTest.inc.fixed | 14 +++ .../Tests/PHP/LowerCaseTypeUnitTest.php | 8 +- 4 files changed, 88 insertions(+), 58 deletions(-) diff --git a/src/Standards/Generic/Sniffs/PHP/LowerCaseTypeSniff.php b/src/Standards/Generic/Sniffs/PHP/LowerCaseTypeSniff.php index 7fb3b85840..1b085853e1 100644 --- a/src/Standards/Generic/Sniffs/PHP/LowerCaseTypeSniff.php +++ b/src/Standards/Generic/Sniffs/PHP/LowerCaseTypeSniff.php @@ -132,31 +132,11 @@ public function process(File $phpcsFile, $stackPtr) if ($startOfType !== $constName) { $endOfType = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($constName - 1), null, true); - $type = ''; - $isUnionType = false; - $isIntersectionType = false; - for ($j = $startOfType; $j <= $endOfType; $j++) { - if (isset($ignore[$tokens[$j]['code']]) === true) { - continue; - } - - if ($tokens[$j]['code'] === T_TYPE_UNION) { - $isUnionType = true; - } - - if ($tokens[$j]['code'] === T_TYPE_INTERSECTION) { - $isIntersectionType = true; - } - - $type .= $tokens[$j]['content']; - } - $error = 'PHP constant type declarations must be lowercase; expected "%s" but found "%s"'; $errorCode = 'ConstantTypeFound'; - if ($isIntersectionType === true) { - // Intersection types don't support simple types. - } else if ($isUnionType === true) { + if ($startOfType !== $endOfType) { + // Multi-token type. $this->processUnionType( $phpcsFile, $startOfType, @@ -164,8 +144,11 @@ public function process(File $phpcsFile, $stackPtr) $error, $errorCode ); - } else if (isset($this->phpTypes[strtolower($type)]) === true) { - $this->processType($phpcsFile, $startOfType, $type, $error, $errorCode); + } else { + $type = $tokens[$startOfType]['content']; + if (isset($this->phpTypes[strtolower($type)]) === true) { + $this->processType($phpcsFile, $startOfType, $type, $error, $errorCode); + } } }//end if @@ -195,9 +178,8 @@ public function process(File $phpcsFile, $stackPtr) $error = 'PHP property type declarations must be lowercase; expected "%s" but found "%s"'; $errorCode = 'PropertyTypeFound'; - if (strpos($type, '&') !== false) { - // Intersection types don't support simple types. - } else if (strpos($type, '|') !== false) { + if ($props['type_token'] !== $props['type_end_token']) { + // Multi-token type. $this->processUnionType( $phpcsFile, $props['type_token'], @@ -227,9 +209,8 @@ public function process(File $phpcsFile, $stackPtr) $error = 'PHP return type declarations must be lowercase; expected "%s" but found "%s"'; $errorCode = 'ReturnTypeFound'; - if (strpos($returnType, '&') !== false) { - // Intersection types don't support simple types. - } else if (strpos($returnType, '|') !== false) { + if ($props['return_type_token'] !== $props['return_type_end_token']) { + // Multi-token type. $this->processUnionType( $phpcsFile, $props['return_type_token'], @@ -259,9 +240,8 @@ public function process(File $phpcsFile, $stackPtr) $error = 'PHP parameter type declarations must be lowercase; expected "%s" but found "%s"'; $errorCode = 'ParamTypeFound'; - if (strpos($typeHint, '&') !== false) { - // Intersection types don't support simple types. - } else if (strpos($typeHint, '|') !== false) { + if ($param['type_hint_token'] !== $param['type_hint_end_token']) { + // Multi-token type. $this->processUnionType( $phpcsFile, $param['type_hint_token'], @@ -279,7 +259,9 @@ public function process(File $phpcsFile, $stackPtr) /** - * Processes a union type declaration. + * Processes a multi-token type declaration. + * + * {@internal The method name is superseded by the reality, but changing it would be a BC-break.} * * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. * @param int $typeDeclStart The position of the start of the type token. @@ -291,37 +273,51 @@ public function process(File $phpcsFile, $stackPtr) */ protected function processUnionType(File $phpcsFile, $typeDeclStart, $typeDeclEnd, $error, $errorCode) { - $tokens = $phpcsFile->getTokens(); - $current = $typeDeclStart; - - do { - $endOfType = $phpcsFile->findNext(T_TYPE_UNION, $current, $typeDeclEnd); - if ($endOfType === false) { - // This must be the last type in the union. - $endOfType = ($typeDeclEnd + 1); - } + $tokens = $phpcsFile->getTokens(); + $typeTokenCount = 0; + $typeStart = null; + $type = ''; - $hasNsSep = $phpcsFile->findNext(T_NS_SEPARATOR, $current, $endOfType); - if ($hasNsSep !== false) { - // Multi-token class based type. Ignore. - $current = ($endOfType + 1); + for ($i = $typeDeclStart; $i <= $typeDeclEnd; $i++) { + if (isset(Tokens::$emptyTokens[$tokens[$i]['code']]) === true) { continue; } - // Type consisting of a single token. - $startOfType = $phpcsFile->findNext(Tokens::$emptyTokens, $current, $endOfType, true); - if ($startOfType === false) { - // Parse error. - return; + if ($tokens[$i]['code'] === T_TYPE_UNION + || $tokens[$i]['code'] === T_TYPE_INTERSECTION + || $tokens[$i]['code'] === T_TYPE_OPEN_PARENTHESIS + || $tokens[$i]['code'] === T_TYPE_CLOSE_PARENTHESIS + ) { + if ($typeTokenCount === 1 + && $type !== '' + && isset($this->phpTypes[strtolower($type)]) === true + ) { + $this->processType($phpcsFile, $typeStart, $type, $error, $errorCode); + } + + // Reset for the next type in the type string. + $typeTokenCount = 0; + $typeStart = null; + $type = ''; + + continue; } - $type = $tokens[$startOfType]['content']; - if (isset($this->phpTypes[strtolower($type)]) === true) { - $this->processType($phpcsFile, $startOfType, $type, $error, $errorCode); + if (isset($typeStart) === false) { + $typeStart = $i; } - $current = ($endOfType + 1); - } while ($current <= $typeDeclEnd); + ++$typeTokenCount; + $type .= $tokens[$i]['content']; + }//end for + + // Handle type at end of type string. + if ($typeTokenCount === 1 + && $type !== '' + && isset($this->phpTypes[strtolower($type)]) === true + ) { + $this->processType($phpcsFile, $typeStart, $type, $error, $errorCode); + } }//end processUnionType() diff --git a/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.inc b/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.inc index 6674970e10..fb5b1fd5a8 100644 --- a/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.inc +++ b/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.inc @@ -125,6 +125,20 @@ enum TypedEnumConstants { public const sTRing | aRRaY | FaLSe FOURTH = 'fourth'; } +class DNFTypes { + const (Parent&Something)|Float CONST_NAME = 1.5; + + public readonly TRUE|(\A&B) $prop; + + function DNFParamTypes ( + null|(\Package\ClassName&\Package\Other_Class)|INT $DNFinMiddle, + (\Package\ClassName&\Package\Other_Class)|ARRAY $parensAtStart, + False|(\Package\ClassName&\Package\Other_Class) $parentAtEnd, + ) {} + + function DNFReturnTypes ($var): object|(Self&\Package\Other_Class)|sTRINg|false {} +} + // Intentional error, should be ignored by the sniff. interface PropertiesNotAllowed { public $notAllowed; diff --git a/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.inc.fixed b/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.inc.fixed index 59e4af8352..10be06b0bb 100644 --- a/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.inc.fixed +++ b/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.inc.fixed @@ -125,6 +125,20 @@ enum TypedEnumConstants { public const string | array | false FOURTH = 'fourth'; } +class DNFTypes { + const (parent&Something)|float CONST_NAME = 1.5; + + public readonly true|(\A&B) $prop; + + function DNFParamTypes ( + null|(\Package\ClassName&\Package\Other_Class)|int $DNFinMiddle, + (\Package\ClassName&\Package\Other_Class)|array $parensAtStart, + false|(\Package\ClassName&\Package\Other_Class) $parentAtEnd, + ) {} + + function DNFReturnTypes ($var): object|(self&\Package\Other_Class)|string|false {} +} + // Intentional error, should be ignored by the sniff. interface PropertiesNotAllowed { public $notAllowed; diff --git a/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.php b/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.php index c5cfbb18aa..2621932857 100644 --- a/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.php +++ b/src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.php @@ -87,6 +87,12 @@ public function getErrorList() 123 => 2, 124 => 3, 125 => 3, + 129 => 2, + 131 => 1, + 134 => 1, + 135 => 1, + 136 => 1, + 139 => 2, ]; }//end getErrorList() @@ -103,7 +109,7 @@ public function getErrorList() public function getWarningList() { // Warning from getMemberProperties() about parse error. - return [130 => 1]; + return [144 => 1]; }//end getWarningList()