Handle @param when var passed by reference

fredden · jrfnl · commit 22ef98ce42b9 · 2023-07-11T22:24:44.000+02:00
diff --git a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php
@@ -547,24 +547,46 @@ 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';
                     }
 
                     $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
@@ -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
@@ -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
@@ -50,8 +50,7 @@ public function getErrorList()
             138  => 4,
             139  => 4,
             143  => 3,
-            152  => 1,
-            155  => 2,
+            155  => 1,
             159  => 1,
             161  => 2,
             166  => 1,
@@ -126,6 +125,26 @@ public function getErrorList()
             1006 => 1,
             1029 => 1,
             1053 => 1,
+            1058 => 2,
+            1069 => 1,
+            1070 => 1,
+            1071 => 1,
+            1075 => 6,
+            1080 => 2,
+            1083 => 1,
+            1084 => 1,
+            1085 => 1,
+            1089 => 3,
+            1093 => 4,
+            1100 => 1,
+            1101 => 1,
+            1102 => 1,
+            1103 => 1,
+            1107 => 8,
+            1123 => 1,
+            1124 => 1,
+            1125 => 1,
+            1129 => 3,
         ];
 
         // Mixed type hints only work from PHP 8.0 onwards.
