From b3ff1d7e34e3953ae6e1296c9c0b0ce3f0160172 Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 15 May 2025 11:47:47 +0700 Subject: [PATCH 1/6] [ticket/17512] Add PHP Sniffer coding standard for union types PHPBB-17512 --- .../CodeLayout/UnionTypesCheckSniff.php | 96 +++++++++++++++++++ build/code_sniffer/ruleset-php-strict.xml | 4 + 2 files changed, 100 insertions(+) create mode 100644 build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php diff --git a/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php new file mode 100644 index 0000000000..c74ec56128 --- /dev/null +++ b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php @@ -0,0 +1,96 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\Sniffs\CodeLayout; + +use PHP_CodeSniffer\Files\File; +use PHP_CodeSniffer\Sniffs\Sniff; + +/** + * Checks that union type declarations follows the coding guidelines. + */ +class UnionTypesCheckSniff implements Sniff +{ + /** + * {@inheritdoc} + */ + public function register() + { + return [ + T_TYPE_UNION, + T_NULLABLE, + ]; + } + + /** + * {@inheritdoc} + */ + public function process(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + + // Check if nullable type shortcut syntax ('?typehint') is used + if ($tokens[$stackPtr]['type'] === 'T_NULLABLE') + { + $type = $tokens[$stackPtr + 1]['content']; + $error = 'Nullable shortcut syntax must not be used. Use union type instead: %2$s|null; found %1$s%2$s'; + $data = [trim($tokens[$stackPtr]['content']), $type]; + $phpcsFile->addError($error, $stackPtr, 'ShortNullableSyntax', $data); + } + + // Get the entry before the 1st '|' and all entries after it untill the end of union type declaration + if ($tokens[$stackPtr]['type'] === 'T_TYPE_UNION' && $tokens[$stackPtr - 2]['type'] !== 'T_TYPE_UNION') + { + // Get all the types within the union type declaration + $types_array = []; + for ($i = $stackPtr - 2; $i++;) + { + if (in_array($tokens[$i]['type'], ['T_TYPE_UNION', 'T_STRING', 'T_NULL'])) + { + if ($tokens[$i]['type'] != 'T_TYPE_UNION') + { + $types_array[] = $tokens[$i]['content']; + } + } + else + { + break; + } + } + + $types_array_sorted = $types_array_null_less = $types_array; + + // Check 'null' to be the last element + $null_position = array_search('null', $types_array); + if ($null_position !== false && $null_position != array_key_last($types_array)) + { + $error = 'The "null" type hint must be the last of the union type elements; found %s'; + $data = [implode('|', $types_array)]; + $phpcsFile->addError($error, $stackPtr, 'NullAlwaysLast', $data); + } + + // Check types excepting 'null' to follow alphabetical order + if ($null_position !== false) + { + array_splice($types_array_null_less, $null_position, 1); + } + sort($types_array_sorted); + if (!empty(array_diff_assoc($types_array_null_less, $types_array_sorted))) + { + $error = 'Union type elements must be sorted alphabetically excepting the "null" type hint must be the last if any; found %s'; + $data = [implode('|', $types_array)]; + $phpcsFile->addError($error, $stackPtr, 'AlphabeticalSort', $data); + } + } + } +} diff --git a/build/code_sniffer/ruleset-php-strict.xml b/build/code_sniffer/ruleset-php-strict.xml index 9e2f0664d8..58fe270021 100644 --- a/build/code_sniffer/ruleset-php-strict.xml +++ b/build/code_sniffer/ruleset-php-strict.xml @@ -45,4 +45,8 @@ + + + From 7a4b3f52aef2e492b3e72a3dfd7f26e31ebe82a4 Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 15 May 2025 16:50:51 +0700 Subject: [PATCH 2/6] [ticket/17512] Refactor PHP Sniffer coding standard logic PHPBB-17512 --- .../CodeLayout/UnionTypesCheckSniff.php | 76 ++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php index c74ec56128..7092c07b31 100644 --- a/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php +++ b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php @@ -22,52 +22,60 @@ use PHP_CodeSniffer\Sniffs\Sniff; class UnionTypesCheckSniff implements Sniff { /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function register() { return [ - T_TYPE_UNION, - T_NULLABLE, + T_FUNCTION, + T_CLASS, ]; } /** - * {@inheritdoc} - */ + * {@inheritdoc} + */ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - // Check if nullable type shortcut syntax ('?typehint') is used - if ($tokens[$stackPtr]['type'] === 'T_NULLABLE') + if ($tokens[$stackPtr]['type'] === 'T_FUNCTION') { - $type = $tokens[$stackPtr + 1]['content']; - $error = 'Nullable shortcut syntax must not be used. Use union type instead: %2$s|null; found %1$s%2$s'; - $data = [trim($tokens[$stackPtr]['content']), $type]; - $phpcsFile->addError($error, $stackPtr, 'ShortNullableSyntax', $data); - } - - // Get the entry before the 1st '|' and all entries after it untill the end of union type declaration - if ($tokens[$stackPtr]['type'] === 'T_TYPE_UNION' && $tokens[$stackPtr - 2]['type'] !== 'T_TYPE_UNION') - { - // Get all the types within the union type declaration - $types_array = []; - for ($i = $stackPtr - 2; $i++;) + $method_params = $phpcsFile->getMethodParameters($stackPtr); + $method_params_array = array_column($method_params, 'type_hint', 'token'); + foreach ($method_params_array as $stack_pointer => $type_hint) { - if (in_array($tokens[$i]['type'], ['T_TYPE_UNION', 'T_STRING', 'T_NULL'])) - { - if ($tokens[$i]['type'] != 'T_TYPE_UNION') - { - $types_array[] = $tokens[$i]['content']; - } - } - else - { - break; - } + $this->check_union_type($phpcsFile, $stack_pointer, $type_hint); } + $method_properties = $phpcsFile->getMethodProperties($stackPtr); + $this->check_union_type($phpcsFile, $stackPtr, $method_properties['return_type']); + } + else if ($tokens[$stackPtr]['type'] === 'T_CLASS') + { + $class_member_pointer = $phpcsFile->findNext(T_VARIABLE, $stackPtr); + $first_method_pointer = $phpcsFile->findNext(T_FUNCTION, $stackPtr); + do + { + $properties = $phpcsFile->getMemberProperties($class_member_pointer); + $this->check_union_type($phpcsFile, $class_member_pointer, $properties['type']); + $class_member_pointer = $phpcsFile->findNext(T_VARIABLE, $class_member_pointer + 1); + } + while($class_member_pointer !== false && ($class_member_pointer < $first_method_pointer)); + } + } + + public function check_union_type(File $phpcsFile, $stack_pointer, $type_hint) + { + if (!strpos($type_hint, '|') && $type_hint[0] == '?') // Check nullable shortcut syntax + { + $type = substr($type_hint, 1); + $error = 'Nullable shortcut syntax must not be used. Use union type instead: %1$s|null; found %2$s'; + $data = [$type, $type_hint]; + $phpcsFile->addError($error, $stack_pointer, 'ShortNullableSyntax', $data); + } + else if (($types_array = explode('|', $type_hint)) > 1) // Check union type layout + { $types_array_sorted = $types_array_null_less = $types_array; // Check 'null' to be the last element @@ -76,7 +84,7 @@ class UnionTypesCheckSniff implements Sniff { $error = 'The "null" type hint must be the last of the union type elements; found %s'; $data = [implode('|', $types_array)]; - $phpcsFile->addError($error, $stackPtr, 'NullAlwaysLast', $data); + $phpcsFile->addError($error, $stack_pointer, 'NullAlwaysLast', $data); } // Check types excepting 'null' to follow alphabetical order @@ -89,8 +97,8 @@ class UnionTypesCheckSniff implements Sniff { $error = 'Union type elements must be sorted alphabetically excepting the "null" type hint must be the last if any; found %s'; $data = [implode('|', $types_array)]; - $phpcsFile->addError($error, $stackPtr, 'AlphabeticalSort', $data); + $phpcsFile->addError($error, $stack_pointer, 'AlphabeticalSort', $data); } - } + } } } From c01d1967ddc9ecf08c2e6e25a5a8c4048388118c Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 15 May 2025 17:02:03 +0700 Subject: [PATCH 3/6] [ticket/17512] Fix handling tokens with no/empty type hint PHPBB-17512 --- .../phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php index 7092c07b31..89ea31efcc 100644 --- a/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php +++ b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php @@ -67,6 +67,11 @@ class UnionTypesCheckSniff implements Sniff public function check_union_type(File $phpcsFile, $stack_pointer, $type_hint) { + if (empty($type_hint)) + { + return; + } + if (!strpos($type_hint, '|') && $type_hint[0] == '?') // Check nullable shortcut syntax { $type = substr($type_hint, 1); From 146f917d19d52187fa6ad8dc12962d7bc3906f6e Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 15 May 2025 17:41:50 +0700 Subject: [PATCH 4/6] [ticket/17512] Fix sniffer error The error message was: $stackPtr is not a class member var PHPBB-17512 --- .../phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php index 89ea31efcc..1c6b76b0fe 100644 --- a/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php +++ b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php @@ -38,7 +38,6 @@ class UnionTypesCheckSniff implements Sniff public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - if ($tokens[$stackPtr]['type'] === 'T_FUNCTION') { $method_params = $phpcsFile->getMethodParameters($stackPtr); @@ -53,15 +52,18 @@ class UnionTypesCheckSniff implements Sniff } else if ($tokens[$stackPtr]['type'] === 'T_CLASS') { - $class_member_pointer = $phpcsFile->findNext(T_VARIABLE, $stackPtr); + $class_token = $tokens[$stackPtr]; + $class_closer_pointer = $class_token['scope_closer']; $first_method_pointer = $phpcsFile->findNext(T_FUNCTION, $stackPtr); - do + $class_members_declarations_end_pointer = $first_method_pointer ?: $class_closer_pointer; + + $stack_pointer = $stackPtr; + while(($class_member_pointer = $phpcsFile->findNext(T_VARIABLE, $stack_pointer)) !== false && ($class_member_pointer < $class_members_declarations_end_pointer)) { $properties = $phpcsFile->getMemberProperties($class_member_pointer); $this->check_union_type($phpcsFile, $class_member_pointer, $properties['type']); - $class_member_pointer = $phpcsFile->findNext(T_VARIABLE, $class_member_pointer + 1); + $stack_pointer = $class_member_pointer + 1; } - while($class_member_pointer !== false && ($class_member_pointer < $first_method_pointer)); } } From 6a0c949ed1e3ecddc8d878142b4233868d18db79 Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 15 May 2025 20:34:16 +0700 Subject: [PATCH 5/6] [ticket/17512] Fix sorting types check logic PHPBB-17512 --- .../CodeLayout/UnionTypesCheckSniff.php | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php index 1c6b76b0fe..64b30a859e 100644 --- a/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php +++ b/build/code_sniffer/phpbb/Sniffs/CodeLayout/UnionTypesCheckSniff.php @@ -81,9 +81,9 @@ class UnionTypesCheckSniff implements Sniff $data = [$type, $type_hint]; $phpcsFile->addError($error, $stack_pointer, 'ShortNullableSyntax', $data); } - else if (($types_array = explode('|', $type_hint)) > 1) // Check union type layout + else if ((count($types_array = explode('|', $type_hint))) > 1) // Check union type layout { - $types_array_sorted = $types_array_null_less = $types_array; + $types_array_null_less = $types_array; // Check 'null' to be the last element $null_position = array_search('null', $types_array); @@ -99,12 +99,17 @@ class UnionTypesCheckSniff implements Sniff { array_splice($types_array_null_less, $null_position, 1); } - sort($types_array_sorted); - if (!empty(array_diff_assoc($types_array_null_less, $types_array_sorted))) + + if (count($types_array_null_less) > 1) { - $error = 'Union type elements must be sorted alphabetically excepting the "null" type hint must be the last if any; found %s'; - $data = [implode('|', $types_array)]; - $phpcsFile->addError($error, $stack_pointer, 'AlphabeticalSort', $data); + $types_array_null_less_sorted = $types_array_null_less; + sort($types_array_null_less_sorted); + if (!empty(array_diff_assoc($types_array_null_less, $types_array_null_less_sorted))) + { + $error = 'Union type elements must be sorted alphabetically excepting the "null" type hint must be the last if any; found %s'; + $data = [implode('|', $types_array)]; + $phpcsFile->addError($error, $stack_pointer, 'AlphabeticalSort', $data); + } } } } From 1cd17caf87f22699cf9a3cf86c134edbf53fe0de Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 15 May 2025 20:42:24 +0700 Subject: [PATCH 6/6] [ticket/17512] Fix the code layout error reported PHPBB-17512 --- phpBB/phpbb/routing/loader_resolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/routing/loader_resolver.php b/phpBB/phpbb/routing/loader_resolver.php index de3adcaa34..2e21d8bb3d 100644 --- a/phpBB/phpbb/routing/loader_resolver.php +++ b/phpBB/phpbb/routing/loader_resolver.php @@ -33,7 +33,7 @@ class loader_resolver implements LoaderResolverInterface /** * {@inheritdoc} */ - public function resolve($resource, $type = null): false|\Symfony\Component\Config\Loader\LoaderInterface + public function resolve($resource, $type = null): \Symfony\Component\Config\Loader\LoaderInterface|false { /** @var \Symfony\Component\Config\Loader\LoaderInterface $loader */ foreach ($this->loaders as $loader)