Merge remote-tracking branch 'upstream/master' into whitespace-incompatibility

Author: fredden

package.xml

@@ -17,8 +17,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
  <date>2022-06-18</date>
  <time>17:20:00</time>
  <version>
-  <release>3.7.2</release>
-  <api>3.7.2</api>
+  <release>3.8.0</release>
+  <api>3.8.0</api>
  </version>
  <stability>
   <release>stable</release>
@@ -41,11 +41,6 @@ http://pear.php.net/dtd/package-2.0.xsd">
     -- Existing code will continue to work but will throw a deprecation error
     -- The backwards compatiblity layer will be removed in PHPCS 4.0
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Newer versions of Composer will now suggest installing PHPCS using require-dev instead of require
-    -- Thanks to Gary Jones (@GaryJones) for the patch
-  - A custom Out Of Memory error will now be shown if PHPCS or PHPCBF run out of memory during a run
-    -- Error message provides actionable information about how to fix the problem and ensures the error is not silent
-    -- Thanks to Juliette Reinders Folmer (@jrfnl) and Alain Schlesser (@schlessera) for the patch
   - When using auto report width (the default) a value of 80 columns will be used if an auto width cannot be determined
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
   - Sniff error messages are now more informative to help bugs get reported to the correct project
@@ -64,41 +59,23 @@ http://pear.php.net/dtd/package-2.0.xsd">
     -- Squiz.Commenting.FileComment
     -- Squiz.Commenting.InlineComment
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Generic.PHP.LowerCaseType sniff now correctly examines types inside arrow functions
-    -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
   - PSR2.Classes.PropertyDeclaration now enforces that the readonly modifier comes after the visibility modifier
     - PSR2 and PSR12 do not have documented rules for this as they pre-date the readonly modifier
     - PSR-PER has been used to confirm the order of this keyword so it can be applied to PSR2 and PSR12 correctly
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Squiz.Formatting.OperatorBracket no longer reports false positives in match() structures
+  - Squiz.Commenting.FunctionComment: new ParamNameUnexpectedAmpersandPrefix error for parameters annotated as passed by reference while the parameter is not passed by reference
+    -- Thanks to Dan Wallis (@fredden) for the patch
+  - Squiz.PHP.InnerFunctions sniff no longer reports on OO methods for OO structures declared within a function or closure
+    -- Thanks to @Daimona for the patch
   - Documentation has been added for the following sniffs:
     -- PSR2.Files.ClosingTag
     -- PSR2.Methods.FunctionCallSignature
     -- PSR2.Methods.FunctionClosingBrace
     -- Thanks to Atsushi Okui (@blue32a) for the patch
   - Fixed bug #3557 : Squiz.Arrays.ArrayDeclaration will now ignore PHP 7.4 array unpacking when determining whether an array is associative
     -- Thanks to Volker Dusch (@edorian) for the patch
-  - Fixed bug #3616 : Squiz.PHP.DisallowComparisonAssignment false positive for PHP 8 match expression
-    -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Fixed bug #3618 : Generic.WhiteSpace.ArbitraryParenthesesSpacing false positive for return new parent()
-    -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Fixed bug #3632 : Short list not tokenized correctly in control structures without braces
-    -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Fixed bug #3639 : Tokenizer not applying tab replacement to heredoc/nowdoc closers
-    -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Fixed bug #3640 : Generic.WhiteSpace.DisallowTabIndent not reporting errors for PHP 7.3 flexible heredoc/nowdoc syntax
-    -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Fixed bug #3645 : PHPCS can show 0 exit code when running in parallel even if child process has fatal error
-    -- Thanks to Alex Panshin (@enl) for the patch
-  - Fixed bug #3653 : False positives for match() in OperatorSpacingSniff
-    -- Thanks to Jaroslav Hanslík (@kukulich) for the patch
-  - Fixed bug #3666 : PEAR.Functions.FunctionCallSignature incorrect indent fix when checking mixed HTML/PHP files
-  - Fixed bug #3668 : PSR12.Classes.ClassInstantiation.MissingParentheses false positive when instantiating parent classes
-    -- Similar issues also fixed in Generic.Functions.FunctionCallArgumentSpacing and Squiz.Formatting.OperatorBracket
-    -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Fixed bug #3672 : Incorrect ScopeIndent.IncorrectExact report for match inside array literal
-  - Fixed bug #3694 : Generic.WhiteSpace.SpreadOperatorSpacingAfter does not ignore spread operator in PHP 8.1 first class callables
-    -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
+  - Fixed bug #3717 : Squiz.Commenting.FunctionComment: fixed false positive for InvalidNoReturn when type is never
+    -- Thanks to Choraimy Kroonstuiver (@axlon) for the patch
   - Fixed bug #3722 : Potential "Uninitialized string offset 1" in octal notation backfill
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
   - Fixed bug #3728 : PHP 8.2 | PSR1/SideEffects: allow for readonly classes
@@ -109,7 +86,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
   - Fixed bug #3779 : Squiz/LowercasePHPFunctions + Generic/ForbiddenFunctions: bug fix for class names in attributes
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
-  - Fixed bug #3785 : Squiz/FunctionComment: potential "Uninitialized string offset 0" when a type contains a duplicate pipe symbol
+  - Fixed bug #3785 : Squiz.Commenting.FunctionComment: potential "Uninitialized string offset 0" when a type contains a duplicate pipe symbol
     -- Thanks to Dan Wallis (@fredden) for the patch
   - Fixed bug #3787 : PEAR/Squiz/[MultiLine]FunctionDeclaration: allow for PHP 8.1 new in initializers
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
@@ -121,6 +98,10 @@ http://pear.php.net/dtd/package-2.0.xsd">
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
   - Fixed bug #3805 : Generic/FunctionCallArgumentSpacing: prevent fixer conflict over PHP 7.3+ trailing comma's in function calls
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
+  - Fixed bug #3806 : Squiz.PHP.InnerFunctions sniff now correctly reports inner functions declared within a closure
+    -- Thanks to @Daimona for the patch
+  - Fixed bug #3813 : Squiz.Commenting.FunctionComment: false positive for parameter name mismatch on parameters annotated as passed by reference
+    -- Thanks to Dan Wallis (@fredden) for the patch
   - Fixed bug #3816 : PSR12/FileHeader: bug fix - false positives on PHP 8.2+ readonly classes
     -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
  </notes>

src/Config.php

@@ -80,7 +80,7 @@ class Config
      *
      * @var string
      */
-    const VERSION = '3.7.2';
+    const VERSION = '3.8.0';
 
     /**
      * Package stability; either stable, beta or alpha.

src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php

@@ -143,9 +143,12 @@ protected function processReturn(File $phpcsFile, $stackPtr, $commentStart)
                             }
                         }
                     }//end if
-                } else if ($returnType !== 'mixed' && in_array('void', $typeNames, true) === false) {
-                    // If return type is not void, there needs to be a return statement
-                    // somewhere in the function that returns something.
+                } else if ($returnType !== 'mixed'
+                    && $returnType !== 'never'
+                    && in_array('void', $typeNames, true) === false
+                ) {
+                    // If return type is not void, never, or mixed, there needs to be a
+                    // return statement somewhere in the function that returns something.
                     if (isset($tokens[$stackPtr]['scope_closer']) === true) {
                         $endToken = $tokens[$stackPtr]['scope_closer'];
                         for ($returnToken = $stackPtr; $returnToken < $endToken; $returnToken++) {
@@ -555,24 +558,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';

src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php

@@ -11,6 +11,7 @@
 
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Tokens;
 
 class InnerFunctionsSniff implements Sniff
 {
@@ -41,21 +42,28 @@ public function process(File $phpcsFile, $stackPtr)
     {
         $tokens = $phpcsFile->getTokens();
 
-        $function = $phpcsFile->getCondition($stackPtr, T_FUNCTION);
-        if ($function === false) {
-            // Not a nested function.
+        if (isset($tokens[$stackPtr]['conditions']) === false) {
             return;
         }
 
-        $class = $phpcsFile->getCondition($stackPtr, T_ANON_CLASS, false);
-        if ($class !== false && $class > $function) {
-            // Ignore methods in anon classes.
-            return;
+        $conditions         = $tokens[$stackPtr]['conditions'];
+        $reversedConditions = array_reverse($conditions, true);
+
+        $outerFuncToken = null;
+        foreach ($reversedConditions as $condToken => $condition) {
+            if ($condition === T_FUNCTION || $condition === T_CLOSURE) {
+                $outerFuncToken = $condToken;
+                break;
+            }
+
+            if (\array_key_exists($condition, Tokens::$ooScopeTokens) === true) {
+                // Ignore methods in OOP structures defined within functions.
+                return;
+            }
         }
 
-        $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true);
-        if ($tokens[$prev]['code'] === T_EQUAL) {
-            // Ignore closures.
+        if ($outerFuncToken === null) {
+            // Not a nested function.
             return;
         }
 

src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc

@@ -1054,3 +1054,83 @@ 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;
+}
+
+/**
+ * @return never
+ */
+function foo() {}

src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed

@@ -1054,3 +1054,83 @@ 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;
+}
+
+/**
+ * @return never
+ */
+function foo() {}