WIP: PHPCS 4.x changes #3
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rodrigoprimo
force-pushed
the
phpcs-4
branch
4 times, most recently
from
f04e071 to
c78b598
Compare
As of PHPCS 4.0, the base sniff test class has been renamed from `AbstractSniffUnitTest` to `AbstractSniffTestCase`. Additionally, the PHPCS test setup no longer uses the outdated custom test suite creation. This means that to allow for the tests to run on both PHPCS 3.x and 4.x, some changes are needed. This commit handles this by: * Changing all test files to `extend` the new test case class and adding a class alias to the test bootstrap for compatibility with PHPCS 3.x. * Adding separate scripts to the `composer.json` file for invoking the tests on PHPCS 3.x vs 4.x. * Updating the minimum PHPUnit 9 version to 9.3.4 as required by the PHPCS 4.x native test framework. * Add jobs to test against PHPCS 4.x to all `test` matrices. * Updating the `quicktest` and `unit-tests` jobs to use the correct Composer script based on the installed PHPCS version. This commit also adds a step `unit-tests` to remove the `PHPCompatibility` dependency. This dependency is not needed in the tests and is currently not (yet) compatible with PHPCS 4.x, so it would block the `composer require`. _Note: even though PHPCS 4.x supports PHPUnit 10 and 11, we cannot widen the version restrictions for PHPUnit (yet) while PHPCS 3.x is also supported as it would lead to PHPUnit 10/11 being installed for PHPCS 3.x on PHP >= 8.1, which would break the test runs._ --------- Co-authored-by: Juliette <[email protected]>
…ache functions Before this sniff was not properly differentiating between fully qualified calls to global WP cache functions, and namespaced calls to non-WP functions with the same name as the WP functions. In other words, the sniff was correctly identifying `\wp_cache_get()` as a WP cache function, but it was incorrectly identifying `MyNamespace\wp_cache_get()` as a WP cache function. This commit fixes this issue.
rodrigoprimo
force-pushed
the
phpcs-4
branch
4 times, most recently
from
d3f8d96 to
5d90556
Compare
…ive in constant alias This method was incorrectly identifying constant alias as the use of a global constant which is incorrect. This change aligns this method with how the sibling method in PHPCompatibility behaves.
There were already a few tests with namespaced names, so this commit adds only what was missing in the existing tests.
Add tests safeguarding that namespaced calls to functions named `define` are handled correctly.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when checking if a given fully qualified function is a target function. In those cases, the leading backslash is removed from the token content to allow for the function name comparisons to continue working as they worked in PHPCS 3.x.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when checking for calls to `define()` and hook functions to handle fully qualified calls correctly.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when checking if a given token is a target token.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when checking function calls after the `T_ASPERAND` token.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when determining the name of the function being called.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when checking if a WP time constant was used.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when checking if a PHP native array key exists function was used.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when determining if a constant is global or not.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when checking the name of a given constants to see if it is one of the discouraged constants and when checking if `define()` was called.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization to this sniff.
Add a new error when validating prefixes if a prefix is not a string or is an empty string. This is necessary in preparation for PHPCS 4.0. Starting with this version, array properties can have types other than string, and the sniff needs to be updated to handle non-string prefixes. Using the same error when the string is empty made sense as before the sniff would generate an error complaining about the length of the prefix. TODO: the value of the prefix is type casted to a string when displaying the error message. For example, `true` becomes `1`. I need to think if this is a problem or not. Also, if `true` is passed as the first and only element of an array property there seems to be an error in PHPCS. Check how PHPCSStandards/PHP_CodeSniffer#1172 and PHPCSStandards/PHP_CodeSniffer#1193 impact this commit.
Check the way the tests are structured. There is for sure room to improve their organization. Also check the way that I'm running the tests in the WordPress/Util/Tests in composer.json and the need to add a custom autoloader to bootstrap.php. There is probably a better way to achieve both.
Check the way the tests are structured. There is for sure room to improve their organization.
Check the way the tests are structured. There is for sure room to improve their organization. This commit is just the basic structured generated by Claude. Everything needs to be reviewed.
Check the way the tests are structured. There is for sure room to improve their organization. This commit is just the basic structured generated by Claude. Everything needs to be reviewed.
Check the way the tests are structured. There is for sure room to improve their organization. This commit is just the basic structured generated by Claude. Everything needs to be reviewed.
Check the way the tests are structured. There is for sure room to improve their organization.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization when determining if a token is namespaced or not. Since the tokenization of namespaced names changed between PHPCS 3 and 4, it was necessary to use different logic depending on the PHPCS version. TODO check if the approach to support both PHPCS 3 and 4 is good.
Check the way the tests are structured. There is for sure room to improve their organization. This commit is just the basic structured generated by Claude. Everything needs to be reviewed.
…s in code comments
… `null` This commit fixes how the sniff handles non-lowercased `null` when passed as the value of the `$ver` parameter. Now the sniff consistently handles `null` regardless of the case and always returns a warning. Before, it would return a warning only for lower-cased `null` and an error for all other variations of `null`.
…null` correctly Before this change passing `\false` or `\null` as the `$ver` parameter would result in a false negative.
I'm adding two different tests for fully qualified global function calls to cover all the global functions that are referenced directly in the `EnqueuedResourceParametersSniff::process_parameters` method.
Need to complete the list of tests
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization of fully qualified static calls to `wpdb::prepare()`, `sprintf()` and `implode()`.
… cases both in PHPCS 3 and 4 In 3 there is only one error because it treats MyNamespace as T_STRING and generates an error and then thinks `esc_sql()` as a valid call to the global function which is incorrect.
The tokenization of (namespaced) "names" has changed in PHP 8.0, and this new tokenization will come into effect for PHP_CodeSniffer as of version 4.0.0. This commit adds handling for the new tokenization of fully qualified function calls and static method calls to this sniff.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Creating this PR to test the GH action changes.