WordPress-Coding-Standards icon indicating copy to clipboard operation
WordPress-Coding-Standards copied to clipboard

PHPCS 3.4.0: Activate select codes from Generic.CodeAnalysis.UnusedFunctionParameter sniff

Open jrfnl opened this issue 7 years ago • 3 comments

Rationale

The upstream Generic.CodeAnalysis.UnusedFunctionParameter checks if all parameters declared in a function signature are used within the function.

This sniff was, so far, not added to WPCS as it contained the following two flaws making it unusable within a WP context:

  • The sniff did not check properly if a class implemented an interface or extended another class. In those cases, the sniff would throw warnings for unused function parameters which couldn't be removed as the function signature of the method in the child class has to be the same as the one in the parent class/interface.
  • The sniff would throw warnings for function parameters being unused before the last used parameter. The whole WordPress hook mechanism is based on call-backs. A function hooked into a filter/action, may not use all parameters the action/filter passes. Example: an action hook may pass four parameters, a hooked in function may only use parameter 1 and 3. In that case, a warning for the second parameter being unused would be useless (as it cannot be removed), while a warning for the fourth parameter not being used would be useful.

Upstream PR https://github.com/squizlabs/PHP_CodeSniffer/pull/1318 improves the sniff to take all those situations into account and introduces a large range of modular error codes covering them. This PR has now been merged upstream and will be released as part of PHPCS 3.4.0.

Of the newly introduced error codes, I'd like to suggest adding the following codes to WPCS:

  • Found
  • FoundAfterLastUsed

For the following errorcodes, some more tests would need to be run to see if these should be added to the standard or not:

  • FoundInExtentedClass
  • FoundInExtentedClassAfterLastUsed
  • FoundInImplementedInterface
  • FoundInImplementedInterfaceAfterLastUsed

I'd like to suggest adding the error codes from this sniff to the Extra ruleset once the minimum PHPCS requirement has been upped to PHPCS 3.4.0.

References

  • Upstream (merged) PR: https://github.com/squizlabs/PHP_CodeSniffer/pull/1318 - Please see the whole thread for the different error codes now available.
  • https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/pull/382#discussion-diff-29981655 - earlier discussion about this sniff where it was decided to not add the sniff to WPCS at that time.

Action Checklist

Once PHPCS 3.4.0 has been released and WPCS is ready/willing to up the minimum required PHPCS version:

  • [ ] Update the PHPCS version requirements in the composer.json file.
  • [ ] Update references to the WPCS minimum PHPCS version requirements in the Readme and wherever else needed.
  • [ ] Add select errorcodes from the Generic.CodeAnalysis.UnusedFunctionParameter sniff to the WordPress-Extra ruleset.xml file and remove the commented out reference to the sniff from the ruleset.

jrfnl avatar Oct 23 '18 03:10 jrfnl

Just a quick question, will this be incorporated in the next version of WPCS (2.2.0)?

dingo-d avatar Apr 14 '19 07:04 dingo-d

What about using something like $__myunusedvariable (with two underscores) to indicate the variable is not used purposefully?

chvillanuevap avatar Jun 27 '20 21:06 chvillanuevap

@chvillanuevap The upstream sniff does not allow for that at this time. See: https://github.com/squizlabs/PHP_CodeSniffer/issues/1962

jrfnl avatar Jun 27 '20 21:06 jrfnl