PHP_CodeSniffer
PHP_CodeSniffer copied to clipboard
PHPCSStandards
test(phpcs): Check for unused variables in PHPCS itself
I found an used variable in src/Standards/Squiz/Sniffs/Functions/MultiLineFunctionDeclarationSniff.php . We should check for unused variables with automated testing.
Description
We can check for unused variables with https://github.com/slevomat/coding-standard/blob/master/doc/variables.md#slevomatcodingstandardvariablesunusedvariable .
Not sure if you want to have a dependency on Slevomat for development? This pull request demonstrates a possibility for the approach, but I did not finish fixing all unused variables yet.
Let me know if this direction makes sense and then I can fix them!
Suggested changelog entry
not needed, as this is an internal fix and will not be visible for users.
Related issues/external references
Types of changes
Bug fix
PR checklist
- [x] I have checked there is no other PR open for the same change.
- [x] I have read the Contribution Guidelines.
- [x] I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
- [x] I have added tests to cover my changes.
- [x] I have verified that the code complies with the projects coding standards.
- [x] [Required for new sniffs] I have added XML documentation for the sniff.
- [x] [Required for new files] I have added any new files to the
package.xmlfile.
@klausi Thanks for this PR, I can accept the code changes, but not the change to CI/dependency addition.
OK, I assume you also don't want to fork the Slevomat sniff into PHPCS to avoid the dependency?
Anyway, I could setup an external CI to check the PHPCS master branch once per day with cron and then send you manual pull requests whenever new unused variables are introduced by accident. It should not happen often anyway.
I will fix the reported instances first to see if there are any false positives.
OK, I assume you also don't want to fork the Slevomat sniff into PHPCS to avoid the dependency?
Correct, see the CONTRIBUTING guide on copyright.
Aynway, I could setup an external CI to check the PHPCS master branch once per day with cron and then send you manual pull requests whenever new unused variables are introduced by accident. It should not happen often anyway.
I appreciate the offer, but no need, I can add it to my local phpcs.xml overload file which contains various other extra sniffs as well and based on which I once in a while send in a clean up PR, like #339. I think that should be enough.
I will fix the reported instances first to see if there are any false positives.
Appreciated, I'll have a look. One word of warning: please be careful, not all code is covered by tests.
Here is the Slevomat config I used to verify that all unused variables are fixed: https://github.com/klausi/PHP_CodeSniffer/commit/3386955e3b2c3d2052f8b73a4c305adec1a375ae
Sorry, got busy with other things, but your feedback looks good! Will work it in when I have time again.