search

PHPCSStandards / PHPCSUtils

A suite of utility functions for use with PHP_CodeSniffer
https://phpcsutils.com/
GNU Lesser General Public License v3.0
53 stars 7 forks source link

BCFile/FunctionDeclarations::get[Method]Properties(): skip over closure use statements #573

Closed jrfnl closed 5 months ago

jrfnl commented 5 months ago

Sister-PR to upstream PR PHPCSStandards/PHP_CodeSniffer#421

This PR improves performance of the BCFile::getMethodProperties() and the FunctionDeclarations::getProperties() methods and prevents incorrect return type information for closures use clauses containing invalid variable imports in the use clause (defensive coding).

Closure use statements can only import plain variables, not properties or other more complex variables.

As things were, when such "illegal" variables were imported in a closure use, the information for the return type could get mangled. While this would be a parse error, for the purposes of static analysis, the BCFile::getMethodProperties() method and the FunctionDeclarations::getProperties() method should still handle this correctly.

This commit updates both methods to always skip over the complete use clause, which prevents the issue and improves performance as the same time (less token walking).

Includes unit tests.

jrfnl commented 5 months ago

As per https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/421#issuecomment-2027997416:

Hmm.. turns out the code sample which triggered me to look into this was actually a parse error 🤦🏻‍♀️. I do still believe the change has value, but I've updated the PR description to reflect the reality (and simplified the tests a little).

jrfnl commented 5 months ago

Upstream PR has been merged and will be included in PHPCS 3.9.2.