diff --git a/CHANGELOG.md b/CHANGELOG.md index 986dfa413f..c83cef690f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/Standards/Generic/Sniffs/CodeAnalysis/UnusedFunctionParameterSniff.php b/src/Standards/Generic/Sniffs/CodeAnalysis/UnusedFunctionParameterSniff.php index ac6e599a03..70247c263d 100644 --- a/src/Standards/Generic/Sniffs/CodeAnalysis/UnusedFunctionParameterSniff.php +++ b/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); diff --git a/src/Standards/Generic/Tests/CodeAnalysis/UnusedFunctionParameterUnitTest.inc b/src/Standards/Generic/Tests/CodeAnalysis/UnusedFunctionParameterUnitTest.inc index d800d690fd..154f03157b 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/UnusedFunctionParameterUnitTest.inc +++ b/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 = << $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; + } +} diff --git a/src/Standards/Generic/Tests/CodeAnalysis/UnusedFunctionParameterUnitTest.php b/src/Standards/Generic/Tests/CodeAnalysis/UnusedFunctionParameterUnitTest.php index b6384888e5..2f3aab80be 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/UnusedFunctionParameterUnitTest.php +++ b/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()