NamingConventions/ValidFunctionName: implement PHPCSUtils and support modern PHP #2191
Conversation
The handling of function names only consisting of underscore was already special cased in the sniff, but not covered by tests. Similarly, live coding was not covered.
PHP is largely case-insensitive for class and method name, but not completely: the case-insensitivety does not apply to the non-ascii range. The PHPCSUtils methods recognize this correctly. Includes minor efficiency tweak by only doing the PHP4 constructor/destructor comparison when it's a real class as a name match can never be made for anonymous classes anyway. Includes unit tests. Note: the test would previously pass, but only due to the `strtolower()` mangling both the class name as well as the method name equally.
The various PHPCSUtils `Scopes` functions support enums since PHPCSUtils 1.0.0-alpha4, so enums are automatically supported now, but let's safeguard this with a test anyway.
| // PHP4 destructors are allowed to break our rules. | ||
| if ( NamingConventions::isEqual( $methodName, '_' . $className ) === true ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
I thought destructors didn't exist until PHP 5?
I've not seen them suggested with a leading underscore in PHP 4 as:
class Foo {
function Foo() {} // Constructor.
function _Foo() {} // Destructor.
}Was that the de facto approach back then? How did they get triggered?
There was a problem hiding this comment.
All good questions. To be honest, it's so far back, that this should be redundant and I hope to never come across that type of code again.
For the purposes of this PR, I've just kept the existing functionality, which already handled things in this way (whether correct or not).
Also note that the PHPCS native PEAR.NamingConventions.ValidFunctionName sniff, which this sniff was originally based on, treats _ClassName() methods the same, i.e. as PHP 4-destructors.
I realize this is not an answer to your question, but there's only so much time (research) I'm willing to spend on work arounds for code which shouldn't exist anymore in the first place.
GaryJones
deleted the
feature/`validfunctionname-phpcsutils-and-modern-php
branch
What about we open an issue to remove those exceptions in WPCS 4.0 ? |
|
I've opened issue #2279 as a reminder to remove the PHP 4 related exceptions from sniffs for WPCS 4.0. |
NamingConventions/ValidFunctionName: add extra tests
The handling of function names only consisting of underscore was already special cased in the sniff, but not covered by tests.
Similarly, live coding was not covered.
NamingConventions/ValidFunctionName: implement PHPCSUtils methods
PHP is largely case-insensitive for class and method name, but not completely: the case-insensitivety does not apply to the non-ascii range.
The PHPCSUtils methods recognize this correctly.
Includes minor efficiency tweak by only doing the PHP4 constructor/destructor comparison when it's a real class as a name match can never be made for anonymous classes anyway.
Includes unit tests.
Note: the test would previously pass, but only due to the
strtolower()mangling both the class name as well as the method name equally.NamingConventions/ValidFunctionName: add tests covering PHP 8.1+ enums
The various PHPCSUtils
Scopesfunctions support enums since PHPCSUtils 1.0.0-alpha4, so enums are automatically supported now, but let's safeguard this with a test anyway.NamingConventions/ValidFunctionName: minor comment fix