squizlabs
Generic/UnusedFunctionParameter: bug fix x 2 (magic methods and error codes)
Generic/UnusedFunctionParameter: ignore class context for closures/arrow functions
As things were, if a closure or arrow function declared within a class which extends or implements would have an unused function parameter, it would get the InExtendedClass or InImplementedInterface addition in the error code.
Those additions were only intended for function declarations where the declaration would potentially be overloading a method from a parent and would have to comply with the method signature of the method in the parent class/interface.
This could lead to underreporting if a standard explicitly excludes the error codes contain InExtendedClass and/or InImplementedInterface.
Fixed now.
Includes additional unit test, though the tests don't safeguard this much as they don't check the error codes of the messages thrown.
The change can be tested manually by running the new tests against master, which will show:
163 | WARNING | The method parameter $d is never used (Generic.CodeAnalysis.UnusedFunctionParameter.FoundInExtendedClassAfterLastUsed)
172 | WARNING | The method parameter $d is never used
| | (Generic.CodeAnalysis.UnusedFunctionParameter.FoundInImplementedInterfaceAfterLastUsed)
... while with the change in this commit, this will be fixed to:
163 | WARNING | The method parameter $d is never used (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
172 | WARNING | The method parameter $d is never used (Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed)
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).
The function signature of magic methods is dictated by PHP and unused parameters cannot be removed
Aren't __construct and __invoke exceptions to this?
The function signature of magic methods is dictated by PHP and unused parameters cannot be removed
Aren't
__constructand__invokeexceptions to this?
@gsherwood Ow! Good catch. Thanks for that!
I've updated the commit to exclude __construct() and __invoke() now and I've added two extra sets of tests at the bottom of the test file to safeguard that those will continue to trigger errors.
Closing as replaced by https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/50
FYI: this fix is included in today's PHP_CodeSniffer 3.8.0 release.
As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).