3.0: Move "is unit test" utility functions to dedicated trait

[jrfnl]

This PR will be easiest to review by looking through the individual commits.

Related to #1877

Commit Details

➕ Sniff: remove the is_token_in_test_method() method 🆕

This method is currently unused in WPCS and it is not 100% clear what the method should do, so removing the method for now.

If needs be it can be brought back later if/when there is more clarity on what the method is supposed to solve.

Move "is this test code" related utilities to dedicated IsUnitTestTrait

The "is this test code" related utilities are only used by a small set of sniffs, so are better placed in a dedicated trait.

This moves the (public) $custom_test_class_whitelist and the $test_class_whitelist properties as well as the is_token_in_test_method() and the is_test_class method to a new WordPressCS\WordPress\Helpers\IsUnitTestTrait and starts using that trait in the relevant sniffs.

The trait has been made stand-alone, i.e. it does not presume that a sniff implementing it extends the WordPressCS\WordPress\Sniff class.
This makes it easier to (re-)use the traits, even when a sniff extends an abstract sniff class from PHPCS itself or from PHPCSUtils.

Note: the is_token_in_test_method() method is currently unused in WPCS, but seems useful enough to keep in place.

Related to #1465

IsUnitTestTrait::is_test_class(): use PHPCSUtils

This implements PHPCSUtils alternatives for PHPCS native methods and other PHPCSUtils utilities in the IsUnitTestTrait::is_test_class() code.

The functionality is unchanged (just less buggy).

➖ IsUnitTestTrait::is_token_in_test_method(): implement PHPCSUtils/improve handling nested functions

IsUnitTestTrait: rename protected property

The protected property $test_class_whitelist has been renamed to $known_test_classes as a step towards removing whitelist/blacklist terminology.

IsUnitTestTrait: rename public property

The public property $custom_test_class_whitelist has been renamed to $custom_test_classes as a step towards removing whitelist/blacklist terminology.

Includes adjusting the property name in all tests which included the property.

Related to #1915

To do before release:

• Annotate this change in the wiki custom properties list

IsUnitTestTrait: rename local variable

The function local variable $whitelist has been renamed to $known_test_classes as a step towards removing whitelist/blacklist terminology.

IsUnitTestTrait: add missing WP Core test case classes

Add two more test cases which were added to /tests/phpunit/includes/.

Note: WP Core contains more test cases, but those are in the tests/phpunit/tests directory, while the test cases in /tests/phpunit/includes/ are supposed to be the "public test API", which is why those are the only ones in this list.

IsUnitTestTrait: improve documentation

[dingo-d]

For example: for a token in a function nested within a test method, the method would previously return true and strictly speaking this is correct when the question is "is the token in a test method ?".

However, this answer is incorrect if the question would be "is this a token which is unit test code ?" as the nested function would be in the global scope and not in the scope of the test class.

It seems to me that, in both cases, the current status of what this helper does is not truthful.

If we want to cover both cases, a new helper would need to be created which will account for the scope of the unit test, and a check should be added in the current helper that would exclude the case if it's in a unit case, but just cover the part where we check if the token is in a test method.

If I'm not mistaken, this could be useful if anonymous functions or classes are used in a method within a test class. Don't ask me how I know of this case, let's just say that integration tests in WordPress require some level of imagination 😂

[jrfnl]

It seems to me that, in both cases, the current status of what this helper does is not truthful.

Well, as the method is not used in WPCS at this time, this discussion is theoretical.

Maybe we should just remove the method completely until such time that WPCS needs it again.

More than anything, it would be better for custom rulesets to exclude the test directories completely for these sniffs. This can't be done from within WPCS as we don't know what the name of the test directory is for individual projects.
These methods are only a guestimation to try and bow out before reporting.

[GaryJones]

As this is going to be for a major release, I'm not against the behaviour here being clarified in a particular way, even if there is no extra helper method to help answer the other question.

[jrfnl]

@dingo-d @GaryJones What can I do to move this PR forward ? I'm happy to adjust it, but at this moment there doesn't seem to be consensus on if and how it should be adjusted. Would it be possible to continue the above discussion ?

[dingo-d]

I'm fine with removing it, especially since it's not being used at the moment. We can easily return it back (it's in the commits).

[GaryJones]

I'm in agreement here.

[jrfnl]

Ok, in that case, the method is hereby gone. I've added a new commit at the start of the stack and removed the "improve the method" commit. The changes have been annotated in the above PR description.

[jrfnl]

Rebased now the GH actions PR has been merged. Would appreciate a merge if the build passes,