CodeAnalysis/AssignmentInCondition: defer to upstream, implement PHPCSUtils & support modern PHP #2198
Conversation
This file contains 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
Upstream PR squizlabs/PHP_CodeSniffer 1594 pulled largely the same sniff to PHPCS itself and was merged in PHPCS 3.1.0. The only part of the sniff I held back at the time, was the "is this an assignment in a ternary condition" part of the sniff as that was (and still is) more prone to false positives/negatives due to the difficulty in determining the start and end of the condition. As the minimum supported PHPCS version for WPCS is now well beyond PHPCS 3.1.0, I'd like to propose to defer to the upstream sniff for the control structure condition checking and to only keep the code related to checking for assignments in ternary conditions in WPCS. To prevent confusion between the two sniffs (the upstream one and this one) and to make it clear which is checking what, I also propose to rename the sniff to `WordPress.CodeAnalysis.AssignmentInTernaryCondition`. Includes bringing the WPCS native PHPCS ruleset inline with this change.
There will always be non-empty tokens before a ternary, if nothing else, a PHP open tag, so this condition is redundant.
... to cover code (which is bowing out of the sniff) previously uncovered.
…l coalesce equals ... to ensure it is recognized as an assignment.
dingo-d
approved these changes
Jan 12, 2023
dingo-d
deleted the
feature/assignment-in-condition-phpcsutils-modern-php
branch
19 tasks
GaryJones
added a commit
to Automattic/VIP-Coding-Standards
that referenced
this pull request
Jun 20, 2023
Update references to account for the WPCS changes: WordPress/WordPress-Coding-Standards#2198
4 tasks
GaryJones
added a commit
to Automattic/VIP-Coding-Standards
that referenced
this pull request
Jun 22, 2023
Update references to account for the WPCS changes: WordPress/WordPress-Coding-Standards#2198
GaryJones
added a commit
to Automattic/VIP-Coding-Standards
that referenced
this pull request
Jun 22, 2023
Update references to account for the WPCS changes: WordPress/WordPress-Coding-Standards#2198
GaryJones
added a commit
to Automattic/VIP-Coding-Standards
that referenced
this pull request
Jun 22, 2023
Update references to account for the WPCS changes: WordPress/WordPress-Coding-Standards#2198
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
CodeAnalysis/AssignmentInCondition: defer to upstream sniff
Upstream PR squizlabs/PHP_CodeSniffer#1594 pulled largely the same sniff to PHPCS itself and was merged in PHPCS 3.1.0.
The only part of the sniff I held back at the time, was the "is this an assignment in a ternary condition" part of the sniff as that was (and still is) more prone to false positives/negatives due to the difficulty in determining the start and end of the condition.
As the minimum supported PHPCS version for WPCS is now well beyond PHPCS 3.1.0, I'd like to propose to defer to the upstream sniff for the control structure condition checking and to only keep the code related to checking for assignments in ternary conditions in WPCS.
To prevent confusion between the two sniffs (the upstream one and this one) and to make it clear which is checking what, I also propose to rename the sniff to
WordPress.CodeAnalysis.AssignmentInTernaryCondition.Includes bringing the WPCS native PHPCS ruleset inline with this change.
CodeAnalysis/AssignmentInTernaryCondition: remove some redundant code
There will always be non-empty tokens before a ternary, if nothing else, a PHP open tag, so this condition is redundant.
CodeAnalysis/AssignmentInTernaryCondition: add some extra tests
... to cover code (which is bowing out of the sniff) previously uncovered.
CodeAnalysis/AssignmentInTernaryCondition: implement PHPCSUtils
CodeAnalysis/AssignmentInTernaryCondition: add test with PHP 7.4+ null coalesce equals
... to ensure it is recognized as an assignment.