Skip to content

Commit a96d0ee

Browse files
jrfnljrfnl
authored
Merge pull request #50 from PHPCSStandards/feature/generic-unusedfunctionparam-various-improvements
Generic/UnusedFunctionParameter: bug fix x 2 (magic methods and error codes)
2 parents 4c2ba65 + 78ea17b commit a96d0ee

File tree

4 files changed

+160
-14
lines changed

4 files changed

+160
-14
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ The file documents changes to the PHP_CodeSniffer project.
7373
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
7474
- Sniff error messages are now more informative to help bugs get reported to the correct project
7575
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
76+
- Generic.CodeAnalysis.UnusedFunctionParameter will now ignore magic methods for which the signature is defined by PHP
77+
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
7678
- Generic.Functions.OpeningFunctionBraceBsdAllman will now check the brace indent before the opening brace for empty functions
7779
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
7880
- 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.
114116
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
115117
- Fixed bug #3557 : Squiz.Arrays.ArrayDeclaration will now ignore PHP 7.4 array unpacking when determining whether an array is associative
116118
- Thanks to Volker Dusch (@edorian) for the patch
119+
- Fixed bug #3715 : Generic/UnusedFunctionParameter: fixed incorrect errorcode for closures/arrow functions nested within extended classes/classes which implement.
120+
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
117121
- Fixed bug #3717 : Squiz.Commenting.FunctionComment: fixed false positive for InvalidNoReturn when type is never
118122
- Thanks to Choraimy Kroonstuiver (@axlon) for the patch
119123
- 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

Lines changed: 48 additions & 9 deletions
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.
@@ -69,16 +95,29 @@ public function process(File $phpcsFile, $stackPtr)
6995
$errorCode = 'Found';
7096
$implements = false;
7197
$extends = false;
72-
$classPtr = $phpcsFile->getCondition($stackPtr, T_CLASS);
73-
if ($classPtr !== false) {
74-
$implements = $phpcsFile->findImplementedInterfaceNames($classPtr);
75-
$extends = $phpcsFile->findExtendedClassName($classPtr);
76-
if ($extends !== false) {
77-
$errorCode .= 'InExtendedClass';
78-
} else if ($implements !== false) {
79-
$errorCode .= 'InImplementedInterface';
98+
99+
if ($token['code'] === T_FUNCTION) {
100+
$classPtr = $phpcsFile->getCondition($stackPtr, T_CLASS);
101+
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.
112+
$implements = $phpcsFile->findImplementedInterfaceNames($classPtr);
113+
$extends = $phpcsFile->findExtendedClassName($classPtr);
114+
if ($extends !== false) {
115+
$errorCode .= 'InExtendedClass';
116+
} else if ($implements !== false) {
117+
$errorCode .= 'InImplementedInterface';
118+
}
80119
}
81-
}
120+
}//end if
82121

83122
$params = [];
84123
$methodParams = $phpcsFile->getMethodParameters($stackPtr);

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

Lines changed: 102 additions & 5 deletions
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
}
@@ -152,3 +152,100 @@ class ConstructorPropertyPromotionWithContentInMethod {
152152
}
153153

154154
$found = in_array_cb($needle, $haystack, fn($array, $needle) => $array[2] === $needle);
155+
156+
157+
/*
158+
* Don't adjust the error code for closures and arrow functions in extended classes/classes implementing interfaces.
159+
*/
160+
class MyExtendedClass extends SomeClass {
161+
public function something($a, $b) {
162+
$c = $a + $b;
163+
$closure = function ($c, $d) {
164+
return $c * 2;
165+
};
166+
}
167+
}
168+
169+
class MyExtendedClass implements SomeInterface {
170+
public function something($a, $b) {
171+
$c = $a + $b;
172+
$fn = fn($c, $d) => $c[2];
173+
}
174+
}
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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ public function getWarningList()
5050
117 => 1,
5151
121 => 2,
5252
125 => 2,
53+
163 => 1,
54+
172 => 1,
55+
228 => 2,
56+
232 => 2,
57+
244 => 2,
58+
248 => 2,
5359
];
5460

5561
}//end getWarningList()

0 commit comments

Comments
 (0)