Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.0: Move "variable" related utilities to dedicated VariableHelper + use PHPCSUtils + handle match #2062

Merged
merged 5 commits into from
Jun 18, 2022

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Jun 17, 2022

Move "array access keys" related utilities to dedicated VariableHelper

The "array access keys" related utilities are only used by a small set of sniffs, so are better placed in a dedicated class.

This commit moves the get_array_access_keys() method and the get_array_access_key() method to a new WordPressCS\WordPress\Helpers\VariableHelper and starts using that class in the relevant sniffs.

In contrast to some of the other "move methods out of the Sniff class" PRs, these methods have been moved to a class and made static - instead of moved to a trait.
The reason for this difference is that the methods in other "moves" are setting properties which the sniff classes would need access to, while these methods are 100% stand-alone.

Note:
It is expected for PHPCSUtils to have dedicated methods for the same at some point in the future. If/when those methods become available, it is recommended for the sniffs to start using the PHPCSUtils methods. With this in mind, this class has been marked as "internal" without BC promise.

Related to #1465

VariableHelper::get_array_access_keys() use PHPCSUtils

Use the PHPCSUtils GetTokensAsString::compact() method to retrieve array access keys without extraneous whitespace or comments.

Move "is comparison" related utility to dedicated VariableHelper

The "is comparison" related utility method is only used by a small set of sniffs, so is better placed in a dedicated class.

As this method also expects a T_VARIABLE token as input, the VariableHelper class seems appropriate.

This commit moves the is_comparison() method to the new WordPressCS\WordPress\Helpers\VariableHelper and starts using that class in the relevant sniffs.

Note:
It is expected for PHPCSUtils to have dedicated methods for the same at some point in the future. If/when those methods become available, it is recommended for the sniffs to start using the PHPCSUtils methods.

Related to #1465

VariableHelper::is_comparison(): use PHPCSUtils

... to get the potential parentheses opener.

PHP 8.0 | VariableHelper::is_comparison(): add support for variables being compared in a match expression

PHP 8.0 introduced match expressions, which do a (strict) comparison on the value in the condition against the options in the match body.
Ref: https://www.php.net/manual/en/control-structures.match.php

This commit updates the is_comparison() method to allow for variables in the "condition" part of a match expression to be considered part of a comparison.

Tested via the ValidatedSanitizedInput sniff.


N.B. While reviewing the "array access keys" methods, I noticed that the methods do not account for array assignments without a key, i.e. $var[]. This may need improved handling at some point, but is outside the scope of this PR.

The "array access keys" related utilities are only used by a small set of sniffs, so are better placed in a dedicated class.

This commit moves the `get_array_access_keys()` method and the `get_array_access_key()` method to a new `WordPressCS\WordPress\Helpers\VariableHelper` and starts using that class in the relevant sniffs.

In contrast to some of the other "move methods out of the Sniff class" PRs, these methods have been moved to a class and made `static` - instead of moved to a `trait`.
The reason for this difference is that the methods in other "moves" are setting properties which the sniff classes would need access to, while these methods are 100% stand-alone.

**Note**:
It is expected for PHPCSUtils to have dedicated methods for the same at some point in the future. If/when those methods become available, it is recommended for the sniffs to start using the PHPCSUtils methods. With this in mind, this class has been marked as "internal" without BC promise.

Related to 1465
Use the PHPCSUtils `GetTokensAsString::compact()` method to retrieve array access keys without extraneous whitespace or comments.
The "is comparison" related utility method is only used by a small set of sniffs, so is better placed in a dedicated class.

As this method also expects a `T_VARIABLE` token as input, the `VariableHelper` class seems appropriate.

This commit moves the `is_comparison()` method to the new `WordPressCS\WordPress\Helpers\VariableHelper` and starts using that class in the relevant sniffs.

**Note**:
It is expected for PHPCSUtils to have dedicated methods for the same at some point in the future. If/when those methods become available, it is recommended for the sniffs to start using the PHPCSUtils methods.

Related to 1465
... to get the potential parentheses opener.
…being compared in a `match` expression

PHP 8.0 introduced `match` expressions, which do a (strict) comparison on the value in the condition against the options in the `match` body.
Ref: https://www.php.net/manual/en/control-structures.match.php

This commit updates the `is_comparison()` method to allow for variables in the "condition" part of a `match` expression to be considered part of a comparison.

Tested via the `ValidatedSanitizedInput` sniff.
Copy link
Member

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GaryJones GaryJones merged commit b0e5332 into develop Jun 18, 2022
@GaryJones GaryJones deleted the feature/new-variablehelper-class branch June 18, 2022 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants