Skip to content

Commit 767d15d

Browse files
committed
Generic/UnusedFunctionParameter: ignore magic methods
The function signature of magic methods - with the exception of `__construct()` and `__invoke()` - is dictated by PHP and unused parameters cannot be removed, which means that the warnings for these can never be resolved, only ignored via annotations. This commit fixes this by checking whether a function is a magic method and if so, bowing out. Includes unit tests. Note: while not all magic methods take arguments, I'm still including the (nearly) full list of magic methods in the property as the other magic methods can be ignored anyway (no arguments).
1 parent 7993c81 commit 767d15d

File tree

3 files changed

+124
-7
lines changed

3 files changed

+124
-7
lines changed

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

+38-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,32 @@ class UnusedFunctionParameterSniff implements Sniff
3030
*/
3131
public $ignoreTypeHints = [];
3232

33+
/**
34+
* A list of all PHP magic methods with fixed method signatures.
35+
*
36+
* Note: `__construct()` and `__invoke()` are excluded on purpose
37+
* as their method signature is not fixed.
38+
*
39+
* @var array
40+
*/
41+
private $magicMethods = [
42+
'__destruct' => true,
43+
'__call' => true,
44+
'__callstatic' => true,
45+
'__get' => true,
46+
'__set' => true,
47+
'__isset' => true,
48+
'__unset' => true,
49+
'__sleep' => true,
50+
'__wakeup' => true,
51+
'__serialize' => true,
52+
'__unserialize' => true,
53+
'__tostring' => true,
54+
'__set_state' => true,
55+
'__clone' => true,
56+
'__debuginfo' => true,
57+
];
58+
3359

3460
/**
3561
* Returns an array of tokens this test wants to listen for.
@@ -71,8 +97,18 @@ public function process(File $phpcsFile, $stackPtr)
7197
$extends = false;
7298

7399
if ($token['code'] === T_FUNCTION) {
74-
$classPtr = $phpcsFile->getCondition($stackPtr, T_CLASS);
100+
$classPtr = $phpcsFile->getCondition($stackPtr, T_CLASS);
75101
if ($classPtr !== false) {
102+
// Check for magic methods and ignore these as the method signature cannot be changed.
103+
$methodName = $phpcsFile->getDeclarationName($stackPtr);
104+
if (empty($methodName) === false) {
105+
$methodNameLc = strtolower($methodName);
106+
if (isset($this->magicMethods[$methodNameLc]) === true) {
107+
return;
108+
}
109+
}
110+
111+
// Check for extends/implements and adjust the error code when found.
76112
$implements = $phpcsFile->findImplementedInterfaceNames($classPtr);
77113
$extends = $phpcsFile->findExtendedClassName($classPtr);
78114
if ($extends !== false) {
@@ -81,7 +117,7 @@ public function process(File $phpcsFile, $stackPtr)
81117
$errorCode .= 'InImplementedInterface';
82118
}
83119
}
84-
}
120+
}//end if
85121

86122
$params = [];
87123
$methodParams = $phpcsFile->getMethodParameters($stackPtr);

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

+82-5
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,13 @@ HERE;
4444
$x = $parameter; // This line must be immediately after the HERE; with no intervening blank lines.
4545
$tango = <<<HERE
4646
1 Must be a HEREdoc
47-
2
48-
3
49-
4
50-
5
47+
2
48+
3
49+
4
50+
5
5151
6
5252
7
53-
8
53+
8
5454
9 at least 9 lines long
5555
HERE;
5656
}
@@ -172,3 +172,80 @@ class MyExtendedClass implements SomeInterface {
172172
$fn = fn($c, $d) => $c[2];
173173
}
174174
}
175+
176+
177+
/**
178+
* Magic methods must match the function signature dictated by PHP.
179+
* Flagging unused parameters leads to notices which cannot be solved.
180+
*/
181+
class MagicMethodsWithParams {
182+
public function __set(string $name, mixed $value) {
183+
// Forbid dynamic properties & overloading inaccessible properties.
184+
throw new RuntimeException('Forbidden');
185+
}
186+
187+
public function __get(string $name) {
188+
throw new RuntimeException('Forbidden');
189+
}
190+
191+
public function __isset(string $name) {
192+
throw new RuntimeException('Forbidden');
193+
}
194+
195+
public function __unset(string $name) {
196+
throw new RuntimeException('Forbidden');
197+
}
198+
199+
public function __unserialize( array $data ) {
200+
// Prevent unserializing from a stored representation of the object for security reasons.
201+
$this->instance = new self();
202+
}
203+
204+
public static function __set_state(array $properties) {
205+
return new self();
206+
}
207+
208+
public function __call(string $name, array $arguments) {
209+
if (method_exists($this, $name)) {
210+
// None of the methods which can be called in this class take arguments, so not passing them.
211+
return $this->$name();
212+
}
213+
}
214+
215+
public static function __callStatic(string $name, array $arguments) {
216+
if (method_exists($this, $name)) {
217+
// None of the methods which can be called in this class take arguments, so not passing them.
218+
return self::$name();
219+
}
220+
}
221+
}
222+
223+
/**
224+
* Unused parameters in magic methods which have flexible function signatures should still be flagged.
225+
*/
226+
class MagicMethodsWithParamsNotDictatedByPHP {
227+
public $foo;
228+
public function __construct($foo, $bar, $baz) {
229+
$this->foo = $foo;
230+
}
231+
232+
public function __invoke($foo, $bar, $baz) {
233+
$this->foo = $foo;
234+
}
235+
}
236+
237+
/**
238+
* Unused parameters in magic methods which have flexible function signatures
239+
* where the method potentially overloads a parent method should still be flagged,
240+
* but should use the `FoundInExtendedClassAfterLastUsed` error code.
241+
*/
242+
class MagicMethodsWithParamsNotDictatedByPHPInChildClass extends SomeParent{
243+
public $foo;
244+
public function __construct($foo, $bar, $baz) {
245+
$this->foo = $foo;
246+
}
247+
248+
public function __invoke($foo, $bar, $baz) {
249+
$this->foo = $foo;
250+
}
251+
}

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

+4
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ public function getWarningList()
5252
125 => 2,
5353
163 => 1,
5454
172 => 1,
55+
228 => 2,
56+
232 => 2,
57+
244 => 2,
58+
248 => 2,
5559
];
5660

5761
}//end getWarningList()

0 commit comments

Comments
 (0)