From 84621dd4b5c33601ebb69d2940bdfe33a5ee1ca6 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Tue, 11 Jul 2023 09:27:53 +0100 Subject: [PATCH] Handle @param when var passed by reference --- .../Commenting/FunctionCommentSniff.php | 32 ++++++-- .../Commenting/FunctionCommentUnitTest.inc | 75 +++++++++++++++++++ .../FunctionCommentUnitTest.inc.fixed | 75 +++++++++++++++++++ .../Commenting/FunctionCommentUnitTest.php | 25 ++++++- 4 files changed, 199 insertions(+), 8 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php index f63d09c43b..38170d22e4 100644 --- a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php +++ b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php @@ -555,16 +555,38 @@ protected function processParams(File $phpcsFile, $stackPtr, $commentStart) // Make sure the param name is correct. if (isset($realParams[$pos]) === true) { - $realName = $realParams[$pos]['name']; - if ($realName !== $param['var']) { + $realName = $realParams[$pos]['name']; + $paramVarName = $param['var']; + + if ($param['var'][0] === '&') { + // Even when passed by reference, the variable name in $realParams does not have + // a leading '&'. This sniff will accept both '&$var' and '$var' in these cases. + $paramVarName = substr($param['var'], 1); + + // This makes sure that the 'MissingParamTag' check won't throw a false positive. + $foundParams[(count($foundParams) - 1)] = $paramVarName; + + if ($realParams[$pos]['pass_by_reference'] !== true && $realName === $paramVarName) { + // Don't complain about this unless the param name is otherwise correct. + $error = 'Doc comment for parameter %s is prefixed with "&" but parameter is not passed by reference'; + $code = 'ParamNameUnexpectedAmpersandPrefix'; + $data = [$paramVarName]; + + // We're not offering an auto-fix here because we can't tell if the docblock + // is wrong, or the parameter should be passed by reference. + $phpcsFile->addError($error, $param['tag'], $code, $data); + } + } + + if ($realName !== $paramVarName) { $code = 'ParamNameNoMatch'; $data = [ - $param['var'], + $paramVarName, $realName, ]; $error = 'Doc comment for parameter %s does not match '; - if (strtolower($param['var']) === strtolower($realName)) { + if (strtolower($paramVarName) === strtolower($realName)) { $error .= 'case of '; $code = 'ParamNameNoCaseMatch'; } @@ -572,7 +594,7 @@ protected function processParams(File $phpcsFile, $stackPtr, $commentStart) $error .= 'actual variable name %s'; $phpcsFile->addError($error, $param['tag'], $code, $data); - } + }//end if } else if (substr($param['var'], -4) !== ',...') { // We must have an extra parameter comment. $error = 'Superfluous parameter comment'; diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc index d57017daf5..55e0fe9bb9 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc @@ -1054,3 +1054,78 @@ function throwCommentOneLine() {} * @return void */ function doublePipeFatalError(?stdClass $object) {} + +/** + * Test for passing variables by reference + * + * This sniff treats the '&' as optional for parameters passed by reference, but + * forbidden for parameters which are not passed by reference. + * + * Because mismatches may be in either direction, we cannot auto-fix these. + * + * @param string $foo A string passed in by reference. + * @param string &$bar A string passed in by reference. + * @param string $baz A string NOT passed in by reference. + * @param string &$qux A string NOT passed in by reference. + * @param string &$case1 A string passed in by reference with a case mismatch. + * @param string &$CASE2 A string NOT passed in by reference, also with a case mismatch. + * + * @return void + */ +public function variablesPassedByReference(&$foo, &$bar, $baz, $qux, &$CASE1, $case2) +{ + return; +} + +/** + * Test for param tag containing ref, but param in declaration not being by ref. + * + * @param string &$foo This should be flagged as (only) ParamNameUnexpectedAmpersandPrefix. + * @param string &$bar This should be flagged as (only) ParamNameNoMatch. + * @param string &$baz This should be flagged as (only) ParamNameNoCaseMatch. + * + * @return void + */ +function passedByRefMismatch($foo, $bra, $BAZ) { + return; +} + +/** + * Test variable case + * + * @param string $foo This parameter is lowercase. + * @param string $BAR This parameter is UPPERCASE. + * @param string $BazQux This parameter is TitleCase. + * @param string $corgeGrault This parameter is camelCase. + * @param string $GARPLY This parameter should be in lowercase. + * @param string $waldo This parameter should be in TitleCase. + * @param string $freD This parameter should be in UPPERCASE. + * @param string $PLUGH This parameter should be in TitleCase. + * + * @return void + */ +public function variableCaseTest( + $foo, + $BAR, + $BazQux, + $corgeGrault, + $garply, + $Waldo, + $FRED, + $PluGh +) { + return; +} + +/** + * Test variable order mismatch + * + * @param string $foo This is the third parameter. + * @param string $bar This is the first parameter. + * @param string $baz This is the second parameter. + * + * @return void + */ +public function variableOrderMismatch($bar, $baz, $foo) { + return; +} diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed index 10e939709b..22474caf85 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed @@ -1054,3 +1054,78 @@ function throwCommentOneLine() {} * @return void */ function doublePipeFatalError(?stdClass $object) {} + +/** + * Test for passing variables by reference + * + * This sniff treats the '&' as optional for parameters passed by reference, but + * forbidden for parameters which are not passed by reference. + * + * Because mismatches may be in either direction, we cannot auto-fix these. + * + * @param string $foo A string passed in by reference. + * @param string &$bar A string passed in by reference. + * @param string $baz A string NOT passed in by reference. + * @param string &$qux A string NOT passed in by reference. + * @param string &$case1 A string passed in by reference with a case mismatch. + * @param string &$CASE2 A string NOT passed in by reference, also with a case mismatch. + * + * @return void + */ +public function variablesPassedByReference(&$foo, &$bar, $baz, $qux, &$CASE1, $case2) +{ + return; +} + +/** + * Test for param tag containing ref, but param in declaration not being by ref. + * + * @param string &$foo This should be flagged as (only) ParamNameUnexpectedAmpersandPrefix. + * @param string &$bar This should be flagged as (only) ParamNameNoMatch. + * @param string &$baz This should be flagged as (only) ParamNameNoCaseMatch. + * + * @return void + */ +function passedByRefMismatch($foo, $bra, $BAZ) { + return; +} + +/** + * Test variable case + * + * @param string $foo This parameter is lowercase. + * @param string $BAR This parameter is UPPERCASE. + * @param string $BazQux This parameter is TitleCase. + * @param string $corgeGrault This parameter is camelCase. + * @param string $GARPLY This parameter should be in lowercase. + * @param string $waldo This parameter should be in TitleCase. + * @param string $freD This parameter should be in UPPERCASE. + * @param string $PLUGH This parameter should be in TitleCase. + * + * @return void + */ +public function variableCaseTest( + $foo, + $BAR, + $BazQux, + $corgeGrault, + $garply, + $Waldo, + $FRED, + $PluGh +) { + return; +} + +/** + * Test variable order mismatch + * + * @param string $foo This is the third parameter. + * @param string $bar This is the first parameter. + * @param string $baz This is the second parameter. + * + * @return void + */ +public function variableOrderMismatch($bar, $baz, $foo) { + return; +} diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php index 06ba08f72a..ff1d10c487 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php @@ -48,8 +48,7 @@ public function getErrorList() 138 => 4, 139 => 4, 143 => 2, - 152 => 1, - 155 => 2, + 155 => 1, 159 => 1, 166 => 1, 173 => 1, @@ -117,6 +116,22 @@ public function getErrorList() 1006 => 1, 1029 => 1, 1053 => 1, + 1058 => 2, + 1069 => 1, + 1070 => 1, + 1071 => 1, + 1080 => 2, + 1083 => 1, + 1084 => 1, + 1085 => 1, + 1093 => 4, + 1100 => 1, + 1101 => 1, + 1102 => 1, + 1103 => 1, + 1123 => 1, + 1124 => 1, + 1125 => 1, ]; // Scalar type hints only work from PHP 7 onwards. @@ -132,12 +147,16 @@ public function getErrorList() $errors[575] = 2; $errors[627] = 1; $errors[1002] = 1; + $errors[1075] = 6; + $errors[1089] = 3; + $errors[1107] = 8; + $errors[1129] = 3; } else { $errors[729] = 4; $errors[740] = 2; $errors[752] = 2; $errors[982] = 1; - } + }//end if // Object type hints only work from PHP 7.2 onwards. if (PHP_VERSION_ID >= 70200) {