-
-
Notifications
You must be signed in to change notification settings - Fork 8
Add Universal.CodeAnalysis.MixedBooleanOperator #271
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
Closed
Closed
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
6b72184
Add Universal.CodeAnalysis.MixedBooleanOperator
TimWolla 9414411
Fully qualify constants in MixedBooleanOperatorSniff
TimWolla cca88c1
Improve error message in MixedBooleanOperatorSniff
TimWolla b4a8ed0
Use `BCFile::findStartOfStatement` in MixedBooleanOperatorSniff
TimWolla 0cc2f34
Add test with an anonymous function having attributes in MixedBoolean…
TimWolla a051300
Extend attribute example for MixedBooleanOperatorUnitTest
TimWolla b8c6d30
Handle curly braces in MixedBooleanOperatorSniff
TimWolla ce00faf
Remove unreachability assertions in MixedBooleanOperatorSniff
TimWolla 4a34225
Satisfy PHPStan
TimWolla 1e87cae
Add “Not OK” comments to MixedBooleanOperatorUnitTest.inc
TimWolla 8f83191
Make use of a local search in `findPrevious()`
TimWolla b713551
Extend test
TimWolla 54fa62c
Fix handling of ternaries
TimWolla 5c82b55
Handle all binary boolean operators
TimWolla 06b47f1
Leverage Tokens::$booleanOperators
TimWolla ba07bcf
Report only the first operator if a type within a single expression
TimWolla 13e9660
Update README
TimWolla 7a71b8a
Clean up implementation
TimWolla 6f8e5ba
Improve naming of the class property
TimWolla 2985f82
Drop unused empty error data
TimWolla 20541c0
Rename MixedBooleanOperatorSniff::$previousTokens and add docblock
TimWolla 9df7cd2
Add `if()` example to MixedBooleanOperatorStandard.xml
TimWolla 855e932
Do not use `array_merge()`
TimWolla 9df244b
Fix codestyle
TimWolla 27f6552
Consistentify the error message
TimWolla c07507c
Hard wrap the error message
TimWolla ecc5bd4
Fix formatting in MixedBooleanOperatorStandard.xml
TimWolla 7b38efb
Fix match arms
TimWolla 6d84a98
Add additional match tests
TimWolla 14f2d22
Add arrow function test
TimWolla 9f78b15
Run composer fixcs
TimWolla 12ce668
Add additional test
TimWolla 312beb6
Rename Sniff
TimWolla d152ea7
Reference related sniffs
TimWolla 3126cd6
Expand documentation
TimWolla 100115b
Unify `@copyright` line
TimWolla File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
32 changes: 32 additions & 0 deletions
32
Universal/Docs/CodeAnalysis/MixedBooleanOperatorStandard.xml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| <?xml version="1.0"?> | ||
| <documentation xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:noNamespaceSchemaLocation="https://phpcsstandards.github.io/PHPCSDevTools/phpcsdocs.xsd" | ||
| title="Mixed Boolean Operator" | ||
| > | ||
| <standard> | ||
| <![CDATA[ | ||
| Forbids mixing `&&` and `||` within a single expression without making precedence clear using parentheses. | ||
| ]]> | ||
| </standard> | ||
| <code_comparison> | ||
| <code title="Valid: Making precedence clear with parentheses."> | ||
| <![CDATA[ | ||
| $one = false; | ||
| $two = false; | ||
| $three = true; | ||
|
|
||
| $result = <em>($one && $two) || $three</em>; | ||
| $result2 = <em>$one && ($two || $three)</em>; | ||
| ]]> | ||
| </code> | ||
| <code title="Invalid: Not using parentheses."> | ||
| <![CDATA[ | ||
| $one = false; | ||
| $two = false; | ||
| $three = true; | ||
|
|
||
| $result = <em>$one && $two || $three</em>; | ||
| ]]> | ||
| </code> | ||
| </code_comparison> | ||
| </documentation> |
107 changes: 107 additions & 0 deletions
107
Universal/Sniffs/CodeAnalysis/MixedBooleanOperatorSniff.php
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| <?php | ||
| /** | ||
| * PHPCSExtra, a collection of sniffs and standards for use with PHP_CodeSniffer. | ||
| * | ||
| * @package PHPCSExtra | ||
| * @copyright 2021 WoltLab GmbH, 2023 PHPCSExtra Contributors | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| * @license https://opensource.org/licenses/LGPL-3.0 LGPL3 | ||
| * @link https://github.com/PHPCSStandards/PHPCSExtra | ||
| */ | ||
|
|
||
| namespace PHPCSExtra\Universal\Sniffs\CodeAnalysis; | ||
|
|
||
| use PHP_CodeSniffer\Files\File; | ||
| use PHP_CodeSniffer\Sniffs\Sniff; | ||
|
|
||
| /** | ||
| * Forbid mixing `&&` and `||` within a single expression without making precedence | ||
| * clear using parentheses. | ||
| * | ||
TimWolla marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * @link https://github.com/squizlabs/PHP_CodeSniffer/pull/3205 | ||
| * @link https://github.com/php-fig/per-coding-style/issues/22 | ||
| * | ||
| * @since 1.2.0 | ||
| */ | ||
| final class MixedBooleanOperatorSniff implements Sniff | ||
| { | ||
|
|
||
| /** | ||
| * Returns an array of tokens this test wants to listen for. | ||
| * | ||
| * @since 1.2.0 | ||
| * | ||
| * @return array<int|string> | ||
| */ | ||
| public function register() | ||
| { | ||
| return [ | ||
| \T_BOOLEAN_OR, | ||
| \T_BOOLEAN_AND, | ||
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * Processes this test, when one of its tokens is encountered. | ||
| * | ||
| * @since 1.2.0 | ||
| * | ||
| * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. | ||
| * @param int $stackPtr The position of the current token | ||
| * in the stack passed in $tokens. | ||
| * | ||
| * @return void | ||
| */ | ||
| public function process(File $phpcsFile, $stackPtr) | ||
| { | ||
| $tokens = $phpcsFile->getTokens(); | ||
| $token = $tokens[$stackPtr]; | ||
|
|
||
| $start = $phpcsFile->findStartOfStatement($stackPtr); | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| if ($token['code'] === T_BOOLEAN_AND) { | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| $search = T_BOOLEAN_OR; | ||
| } elseif ($token['code'] === T_BOOLEAN_OR) { | ||
| $search = T_BOOLEAN_AND; | ||
| } else { | ||
| throw new \LogicException('Unreachable'); | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| while (true) { | ||
| $previous = $phpcsFile->findPrevious( | ||
| [ | ||
| $search, | ||
| T_OPEN_PARENTHESIS, | ||
| T_OPEN_SQUARE_BRACKET, | ||
| T_CLOSE_PARENTHESIS, | ||
| T_CLOSE_SQUARE_BRACKET, | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ], | ||
| $stackPtr, | ||
| $start | ||
| ); | ||
|
|
||
| if ($previous === false) { | ||
| break; | ||
| } | ||
|
|
||
| if ($tokens[$previous]['code'] === T_OPEN_PARENTHESIS | ||
| || $tokens[$previous]['code'] === T_OPEN_SQUARE_BRACKET | ||
| ) { | ||
| // We halt if we reach the opening parens / bracket of the boolean operator. | ||
| return; | ||
| } elseif ($tokens[$previous]['code'] === T_CLOSE_PARENTHESIS) { | ||
| // We skip the content of nested parens. | ||
| $stackPtr = ($tokens[$previous]['parenthesis_opener'] - 1); | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } elseif ($tokens[$previous]['code'] === T_CLOSE_SQUARE_BRACKET) { | ||
| // We skip the content of nested brackets. | ||
| $stackPtr = ($tokens[$previous]['bracket_opener'] - 1); | ||
| } elseif ($tokens[$previous]['code'] === $search) { | ||
| // We reached a mismatching operator, thus we must report the error. | ||
| $error = "Mixed '&&' and '||' within an expression without using parentheses."; | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| $phpcsFile->addError($error, $stackPtr, 'MissingParentheses', []); | ||
| return; | ||
| } else { | ||
| throw new \LogicException('Unreachable'); | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| } | ||
| } | ||
41 changes: 41 additions & 0 deletions
41
Universal/Tests/CodeAnalysis/MixedBooleanOperatorUnitTest.inc
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| <?php | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| if (true && true || true); | ||
| if ((true && true) || true); | ||
| if (true && (true || true)); | ||
|
|
||
| $var = true && true || true; | ||
| $var = (true && true) || true; | ||
| $var = true && (true || true); | ||
|
|
||
| $complex = true && (true || true) && true; | ||
| $complex = true && (true || true) || true; | ||
|
|
||
| if ( | ||
| true | ||
| && true | ||
| || true | ||
| ); | ||
|
|
||
| if ( | ||
| true | ||
| && ( | ||
| true | ||
| || true | ||
| ) | ||
| ); | ||
|
|
||
| if (true && foo(true || true)); | ||
| if (true && foo(true && true || true)); | ||
| if (true && $foo[true || true]); | ||
| if (true && $foo[true && true || true]); | ||
|
|
||
| if (true && foo(true) || true); | ||
| if (true && $foo[true] || true); | ||
| if (true && foo($foo[true]) || true); | ||
TimWolla marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| $foo[] = true && true || false; | ||
|
|
||
| foo([true && true || false]); | ||
|
|
||
| if (true && true || true && true); | ||
57 changes: 57 additions & 0 deletions
57
Universal/Tests/CodeAnalysis/MixedBooleanOperatorUnitTest.php
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| <?php | ||
| /** | ||
| * PHPCSExtra, a collection of sniffs and standards for use with PHP_CodeSniffer. | ||
| * | ||
| * @package PHPCSExtra | ||
| * @copyright 2021 WoltLab GmbH, 2023 PHPCSExtra Contributors | ||
| * @license https://opensource.org/licenses/LGPL-3.0 LGPL3 | ||
| * @link https://github.com/PHPCSStandards/PHPCSExtra | ||
| */ | ||
|
|
||
| namespace PHPCSExtra\Universal\Tests\CodeAnalysis; | ||
|
|
||
| use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; | ||
|
|
||
| /** | ||
| * Unit test class for the ForeachUniqueAssignment sniff. | ||
| * | ||
| * @covers PHPCSExtra\Universal\Sniffs\CodeAnalysis\MixedBooleanOperatorSniff | ||
| * | ||
| * @since 1.2.0 | ||
| */ | ||
| final class MixedBooleanOperatorUnitTest extends AbstractSniffUnitTest | ||
| { | ||
|
|
||
| /** | ||
| * Returns the lines where errors should occur. | ||
| * | ||
| * @return array<int, int> Key is the line number, value is the number of expected errors. | ||
| */ | ||
| public function getErrorList() | ||
| { | ||
| return [ | ||
| 3 => 1, | ||
| 7 => 1, | ||
| 12 => 1, | ||
| 17 => 1, | ||
| 29 => 1, | ||
| 31 => 1, | ||
| 33 => 1, | ||
| 34 => 1, | ||
| 35 => 1, | ||
| 37 => 1, | ||
| 39 => 1, | ||
| 41 => 2, | ||
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the lines where warnings should occur. | ||
| * | ||
| * @return array<int, int> Key is the line number, value is the number of expected warnings. | ||
| */ | ||
| public function getWarningList() | ||
| { | ||
| return []; | ||
| } | ||
| } |
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.
Uh oh!
There was an error while loading. Please reload this page.