From e0b958082ef6fb6a4eab58bdaaee2a0cf4172874 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 10 Apr 2024 16:09:22 -0300 Subject: [PATCH 1/3] Generic/DisallowYodaConditions: various minor cleanup * Remove unused variable. * Fix typo in code comment. * Remove incorrect code comment. --- .../Sniffs/ControlStructures/DisallowYodaConditionsSniff.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php b/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php index 4a98566587..44b163218f 100644 --- a/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php +++ b/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php @@ -85,7 +85,6 @@ public function process(File $phpcsFile, $stackPtr) // Is it a parenthesis. if ($tokens[$previousIndex]['code'] === T_CLOSE_PARENTHESIS) { - // Check what exists inside the parenthesis. $closeParenthesisIndex = $phpcsFile->findPrevious( Tokens::$emptyTokens, ($tokens[$previousIndex]['parenthesis_opener'] - 1), @@ -110,7 +109,7 @@ public function process(File $phpcsFile, $stackPtr) return; } - // If there is nothing inside the parenthesis, it it not a Yoda. + // If there is nothing inside the parenthesis, it is not a Yoda condition. $opener = $tokens[$previousIndex]['parenthesis_opener']; $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($previousIndex - 1), ($opener + 1), true); if ($prev === false) { @@ -144,7 +143,6 @@ public function isArrayStatic(File $phpcsFile, $arrayToken) { $tokens = $phpcsFile->getTokens(); - $arrayEnd = null; if ($tokens[$arrayToken]['code'] === T_OPEN_SHORT_ARRAY) { $start = $arrayToken; $end = $tokens[$arrayToken]['bracket_closer']; From fd9d74ed780b4c6d3b46fb1e987e106aedbafe43 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 10 Apr 2024 16:29:40 -0300 Subject: [PATCH 2/3] Generic/DisallowYodaConditions: improve code coverage Includes removing two unreachable conditions The first condition is unreachable because PHPCS will never call DisallowYodaConditionsSniff::process() if there is no non-empty token before a comparison token. Even if the file contains a syntax error, there must be at least a PHP opening tag before the comparison token for the method to be called. The second condition is unreachable because at this point in the code there will always be at least two non-empty tokens before the comparison token. If there is only one, it must be a PHP opening tag and, in this case, the method will bail before reaching the code that sets the `$prevIndex` variable. This commit also documents why a certain line is uncovered by tests - and cannot be covered, nor can it be removed as external sniffs may call `DisallowYodaConditionsSniff::isArrayStatic()` directly. --- .../ControlStructures/DisallowYodaConditionsSniff.php | 8 ++------ .../DisallowYodaConditionsUnitTest.inc | 10 ++++++++++ .../DisallowYodaConditionsUnitTest.php | 5 +++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php b/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php index 44b163218f..6d39207b36 100644 --- a/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php +++ b/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php @@ -57,9 +57,7 @@ public function process(File $phpcsFile, $stackPtr) T_CONSTANT_ENCAPSED_STRING, ]; - if ($previousIndex === false - || in_array($tokens[$previousIndex]['code'], $relevantTokens, true) === false - ) { + if (in_array($tokens[$previousIndex]['code'], $relevantTokens, true) === false) { return; } @@ -71,9 +69,6 @@ public function process(File $phpcsFile, $stackPtr) } $prevIndex = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($previousIndex - 1), null, true); - if ($prevIndex === false) { - return; - } if (in_array($tokens[$prevIndex]['code'], Tokens::$arithmeticTokens, true) === true) { return; @@ -150,6 +145,7 @@ public function isArrayStatic(File $phpcsFile, $arrayToken) $start = $tokens[$arrayToken]['parenthesis_opener']; $end = $tokens[$arrayToken]['parenthesis_closer']; } else { + // Shouldn't be possible but may happen if external sniffs are using this method. return true; } diff --git a/src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.inc b/src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.inc index 9f3c20b970..27053c4d65 100644 --- a/src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.inc +++ b/src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.inc @@ -175,3 +175,13 @@ echo match ($text) { }; 1 ?? $nullCoalescingShouldNotTriggerSniff; + +1 + 2 === $sniffBailsArithmeticToken; + +'string' . 'concat' === $sniffBailsStringConcatToken; + +1 != $value; +1 <> $value; +1 >= $value; +1 <= $value; +1 <=> $value; diff --git a/src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.php b/src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.php index 52fc370a68..64a487d512 100644 --- a/src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.php +++ b/src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.php @@ -67,6 +67,11 @@ public function getErrorList() 167 => 1, 173 => 1, 174 => 1, + 183 => 1, + 184 => 1, + 185 => 1, + 186 => 1, + 187 => 1, ]; }//end getErrorList() From 48a1a43379e44075a91c32d07a386c9f88061f5a Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Tue, 16 Apr 2024 16:11:37 -0300 Subject: [PATCH 3/3] Generic/DisallowYodaConditions: rename variable This commit renames a variable to use a better name that actually matches what the variable contains. The previous name was misleading as the variable did not contain the index of the closing parenthesis. --- .../ControlStructures/DisallowYodaConditionsSniff.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php b/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php index 6d39207b36..3dc7795b37 100644 --- a/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php +++ b/src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php @@ -80,15 +80,15 @@ public function process(File $phpcsFile, $stackPtr) // Is it a parenthesis. if ($tokens[$previousIndex]['code'] === T_CLOSE_PARENTHESIS) { - $closeParenthesisIndex = $phpcsFile->findPrevious( + $beforeOpeningParenthesisIndex = $phpcsFile->findPrevious( Tokens::$emptyTokens, ($tokens[$previousIndex]['parenthesis_opener'] - 1), null, true ); - if ($closeParenthesisIndex === false || $tokens[$closeParenthesisIndex]['code'] !== T_ARRAY) { - if ($tokens[$closeParenthesisIndex]['code'] === T_STRING) { + if ($beforeOpeningParenthesisIndex === false || $tokens[$beforeOpeningParenthesisIndex]['code'] !== T_ARRAY) { + if ($tokens[$beforeOpeningParenthesisIndex]['code'] === T_STRING) { return; } @@ -110,8 +110,8 @@ public function process(File $phpcsFile, $stackPtr) if ($prev === false) { return; } - } else if ($tokens[$closeParenthesisIndex]['code'] === T_ARRAY - && $this->isArrayStatic($phpcsFile, $closeParenthesisIndex) === false + } else if ($tokens[$beforeOpeningParenthesisIndex]['code'] === T_ARRAY + && $this->isArrayStatic($phpcsFile, $beforeOpeningParenthesisIndex) === false ) { return; }//end if