From cae56aff37bf346d468ca46430f8313752b850ef Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Wed, 29 Mar 2023 17:17:02 +0100 Subject: [PATCH 1/4] Add test to demonstrate bug --- .../Squiz/Tests/Commenting/FunctionCommentUnitTest.inc | 8 ++++++++ .../Tests/Commenting/FunctionCommentUnitTest.inc.fixed | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc index 4f59f60b71..d57017daf5 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc @@ -1046,3 +1046,11 @@ public function ignored() { * @return void * @throws Exception If any other error occurs. */ function throwCommentOneLine() {} + +/** + * When two adjacent pipe symbols are used (by mistake), the sniff should not throw a PHP Fatal error + * + * @param stdClass||null $object While invalid, this should not throw a PHP Fatal error. + * @return void + */ +function doublePipeFatalError(?stdClass $object) {} diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed index 21a4103eb5..330563b0d5 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed @@ -1046,3 +1046,11 @@ public function ignored() { * @return void * @throws Exception If any other error occurs. */ function throwCommentOneLine() {} + +/** + * When two adjacent pipe symbols are used (by mistake), the sniff should not throw a PHP Fatal error + * + * @param stdClass||null $object While invalid, this should not throw a PHP Fatal error. + * @return void + */ +function doublePipeFatalError(?stdClass $object) {} From 8acfadab7f5a8419e287cfe73150707389b8b1a4 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Wed, 29 Mar 2023 17:17:17 +0100 Subject: [PATCH 2/4] Fix bug --- src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php index ba3e1710f0..91f5693ef4 100644 --- a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php +++ b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php @@ -406,7 +406,7 @@ protected function processParams(File $phpcsFile, $stackPtr, $commentStart) foreach ($typeNames as $typeName) { // Strip nullable operator. - if ($typeName[0] === '?') { + if (strlen($typeName) > 1 && $typeName[0] === '?') { $typeName = substr($typeName, 1); } From c38bb12175691e6e48a02c00ccb69dc5da1ae158 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Wed, 29 Mar 2023 18:48:38 +0100 Subject: [PATCH 3/4] Use isset() over strlen() for better performance --- src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php index 91f5693ef4..1101165cbd 100644 --- a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php +++ b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php @@ -406,7 +406,7 @@ protected function processParams(File $phpcsFile, $stackPtr, $commentStart) foreach ($typeNames as $typeName) { // Strip nullable operator. - if (strlen($typeName) > 1 && $typeName[0] === '?') { + if (isset($typeName[0]) === true && $typeName[0] === '?') { $typeName = substr($typeName, 1); } From 8c84123220a4c870f898e8a0c33024e490b479cf Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Thu, 8 Jun 2023 16:24:30 +0100 Subject: [PATCH 4/4] Skip empty types --- .../Squiz/Sniffs/Commenting/FunctionCommentSniff.php | 6 +++++- .../Tests/Commenting/FunctionCommentUnitTest.inc.fixed | 2 +- .../Squiz/Tests/Commenting/FunctionCommentUnitTest.php | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php index 1101165cbd..f63d09c43b 100644 --- a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php +++ b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php @@ -405,8 +405,12 @@ protected function processParams(File $phpcsFile, $stackPtr, $commentStart) $suggestedTypeNames = []; foreach ($typeNames as $typeName) { + if ($typeName === '') { + continue; + } + // Strip nullable operator. - if (isset($typeName[0]) === true && $typeName[0] === '?') { + if ($typeName[0] === '?') { $typeName = substr($typeName, 1); } diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed index 330563b0d5..10e939709b 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed @@ -1050,7 +1050,7 @@ function throwCommentOneLine() {} /** * When two adjacent pipe symbols are used (by mistake), the sniff should not throw a PHP Fatal error * - * @param stdClass||null $object While invalid, this should not throw a PHP Fatal error. + * @param stdClass|null $object While invalid, this should not throw a PHP Fatal error. * @return void */ function doublePipeFatalError(?stdClass $object) {} diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php index 632b7c51b2..06ba08f72a 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php @@ -116,6 +116,7 @@ public function getErrorList() 1004 => 2, 1006 => 1, 1029 => 1, + 1053 => 1, ]; // Scalar type hints only work from PHP 7 onwards.