Generic/UnusedFunctionParameter: bug fix x 2 (magic methods and error codes)

CHANGELOG.md

@@ -73,6 +73,8 @@ The file documents changes to the PHP_CodeSniffer project.
     - 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
     - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
+- Generic.CodeAnalysis.UnusedFunctionParameter will now ignore magic methods for which the signature is defined by PHP
+    - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
 - Generic.Functions.OpeningFunctionBraceBsdAllman will now check the brace indent before the opening brace for empty functions
     - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
 - Generic.Functions.OpeningFunctionBraceKernighanRitchie will now check the spacing before the opening brace for empty functions
@@ -114,6 +116,8 @@ The file documents changes to the PHP_CodeSniffer project.
     - Thanks to Juliette Reinders Folmer (@jrfnl) 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 #3715 : Generic/UnusedFunctionParameter: fixed incorrect errorcode for closures/arrow functions nested within extended classes/classes which implement.
+    - 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 #3720 : Generic/RequireStrictTypes : will now bow out silently in case of parse errors/live coding instead of throwing false positives/false negatives

src/Standards/Generic/Sniffs/CodeAnalysis/UnusedFunctionParameterSniff.php

@@ -30,6 +30,32 @@ class UnusedFunctionParameterSniff implements Sniff
      */
     public $ignoreTypeHints = [];
 
+    /**
+     * A list of all PHP magic methods with fixed method signatures.
+     *
+     * Note: `__construct()` and `__invoke()` are excluded on purpose
+     * as their method signature is not fixed.
+     *
+     * @var array
+     */
+    private $magicMethods = [
+        '__destruct'    => true,
+        '__call'        => true,
+        '__callstatic'  => true,
+        '__get'         => true,
+        '__set'         => true,
+        '__isset'       => true,
+        '__unset'       => true,
+        '__sleep'       => true,
+        '__wakeup'      => true,
+        '__serialize'   => true,
+        '__unserialize' => true,
+        '__tostring'    => true,
+        '__set_state'   => true,
+        '__clone'       => true,
+        '__debuginfo'   => true,
+    ];
+
 
     /**
      * Returns an array of tokens this test wants to listen for.
@@ -69,16 +95,29 @@ public function process(File $phpcsFile, $stackPtr)
         $errorCode  = 'Found';
         $implements = false;
         $extends    = false;
-        $classPtr   = $phpcsFile->getCondition($stackPtr, T_CLASS);
-        if ($classPtr !== false) {
-            $implements = $phpcsFile->findImplementedInterfaceNames($classPtr);
-            $extends    = $phpcsFile->findExtendedClassName($classPtr);
-            if ($extends !== false) {
-                $errorCode .= 'InExtendedClass';
-            } else if ($implements !== false) {
-                $errorCode .= 'InImplementedInterface';
+
+        if ($token['code'] === T_FUNCTION) {
+            $classPtr = $phpcsFile->getCondition($stackPtr, T_CLASS);
+            if ($classPtr !== false) {
+                // Check for magic methods and ignore these as the method signature cannot be changed.
+                $methodName = $phpcsFile->getDeclarationName($stackPtr);
+                if (empty($methodName) === false) {
+                    $methodNameLc = strtolower($methodName);
+                    if (isset($this->magicMethods[$methodNameLc]) === true) {
+                        return;
+                    }
+                }
+
+                // Check for extends/implements and adjust the error code when found.
+                $implements = $phpcsFile->findImplementedInterfaceNames($classPtr);
+                $extends    = $phpcsFile->findExtendedClassName($classPtr);
+                if ($extends !== false) {
+                    $errorCode .= 'InExtendedClass';
+                } else if ($implements !== false) {
+                    $errorCode .= 'InImplementedInterface';
+                }
             }
-        }
+        }//end if
 
         $params       = [];
         $methodParams = $phpcsFile->getMethodParameters($stackPtr);

src/Standards/Generic/Tests/CodeAnalysis/UnusedFunctionParameterUnitTest.inc

@@ -44,13 +44,13 @@ HERE;
     $x = $parameter; // This line must be immediately after the HERE; with no intervening blank lines.
     $tango = <<<HERE
 1 Must be a HEREdoc
-2 
-3 
-4 
-5   
+2
+3
+4
+5
 6
 7
-8 
+8
 9 at least 9 lines long
 HERE;
 }
@@ -152,3 +152,100 @@ class ConstructorPropertyPromotionWithContentInMethod {
 }
 
 $found = in_array_cb($needle, $haystack, fn($array, $needle) => $array[2] === $needle);
+
+
+/*
+ * Don't adjust the error code for closures and arrow functions in extended classes/classes implementing interfaces.
+ */
+class MyExtendedClass extends SomeClass {
+    public function something($a, $b) {
+        $c = $a + $b;
+        $closure = function ($c, $d) {
+            return $c * 2;
+        };
+    }
+}
+
+class MyExtendedClass implements SomeInterface {
+    public function something($a, $b) {
+        $c = $a + $b;
+        $fn = fn($c, $d) => $c[2];
+    }
+}
+
+
+/**
+ * Magic methods must match the function signature dictated by PHP.
+ * Flagging unused parameters leads to notices which cannot be solved.
+ */
+class MagicMethodsWithParams {
+    public function __set(string $name, mixed $value) {
+        // Forbid dynamic properties & overloading inaccessible properties.
+        throw new RuntimeException('Forbidden');
+    }
+
+    public function __get(string $name) {
+        throw new RuntimeException('Forbidden');
+    }
+
+    public function __isset(string $name) {
+        throw new RuntimeException('Forbidden');
+    }
+
+    public function __unset(string $name) {
+        throw new RuntimeException('Forbidden');
+    }
+
+    public function __unserialize( array $data ) {
+        // Prevent unserializing from a stored representation of the object for security reasons.
+        $this->instance = new self();
+    }
+
+    public static function __set_state(array $properties) {
+        return new self();
+    }
+
+    public function __call(string $name, array $arguments) {
+        if (method_exists($this, $name)) {
+            // None of the methods which can be called in this class take arguments, so not passing them.
+            return $this->$name();
+        }
+    }
+
+    public static function __callStatic(string $name, array $arguments) {
+        if (method_exists($this, $name)) {
+            // None of the methods which can be called in this class take arguments, so not passing them.
+            return self::$name();
+        }
+    }
+}
+
+/**
+ * Unused parameters in magic methods which have flexible function signatures should still be flagged.
+ */
+class MagicMethodsWithParamsNotDictatedByPHP {
+    public $foo;
+    public function __construct($foo, $bar, $baz) {
+        $this->foo = $foo;
+    }
+
+    public function __invoke($foo, $bar, $baz) {
+        $this->foo = $foo;
+    }
+}
+
+/**
+ * Unused parameters in magic methods which have flexible function signatures
+ * where the method potentially overloads a parent method should still be flagged,
+ * but should use the `FoundInExtendedClassAfterLastUsed` error code.
+ */
+class MagicMethodsWithParamsNotDictatedByPHPInChildClass extends SomeParent{
+    public $foo;
+    public function __construct($foo, $bar, $baz) {
+        $this->foo = $foo;
+    }
+
+    public function __invoke($foo, $bar, $baz) {
+        $this->foo = $foo;
+    }
+}

src/Standards/Generic/Tests/CodeAnalysis/UnusedFunctionParameterUnitTest.php

@@ -50,6 +50,12 @@ public function getWarningList()
             117 => 1,
             121 => 2,
             125 => 2,
+            163 => 1,
+            172 => 1,
+            228 => 2,
+            232 => 2,
+            244 => 2,
+            248 => 2,
         ];
 
     }//end getWarningList()