From 19644913ca2deb323454058ff3279b331b4766c6 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Mon, 19 Jan 2026 21:37:54 -0400 Subject: [PATCH] Updae flip sniffs to handle object properties better --- .../FlipForeachIfToContinueSniff.php | 66 ++++++++++++++--- .../CodeAnalysis/FlipIfToEarlyReturnSniff.php | 66 ++++++++++++++--- .../FlipIfToEarlyReturnVariableSniff.php | 72 +++++++++++++++---- .../FlipLargeIfSmallElseSniff.php | 67 ++++++++++++++--- .../FlipLoopIfElseToContinueSniff.php | 66 ++++++++++++++--- 5 files changed, 287 insertions(+), 50 deletions(-) diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipForeachIfToContinueSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipForeachIfToContinueSniff.php index c3cb4b7acd..27599c5b4f 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipForeachIfToContinueSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipForeachIfToContinueSniff.php @@ -269,7 +269,7 @@ private function negateCondition( $condition ) { * @return false|string The flipped condition, or false if not a simple comparison. */ private function flipComparisonOperator( $condition ) { - // Map of operators to their opposites. + // Map of operators to their opposites (check longer ones first). $operatorMap = array( '!==' => '===', '===' => '!==', @@ -277,22 +277,72 @@ private function flipComparisonOperator( $condition ) { '==' => '!=', '>=' => '<', '<=' => '>', - '>' => '<=', - '<' => '>=', ); + // Make sure this is a simple comparison (no && or ||). + if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { + return false; + } + // Check for each operator (check longer ones first). foreach ( $operatorMap as $op => $opposite ) { $pos = strpos( $condition, $op ); - if ( $pos !== false ) { - // Make sure this is a simple comparison (no && or ||). - if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { - return false; + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + } + } + + // Handle single < and > separately to avoid matching -> or =>. + $pos = $this->findComparisonOperator( $condition, '>' ); + + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '<=' . substr( $condition, $pos + 1 ); + } + + $pos = $this->findComparisonOperator( $condition, '<' ); + + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '>=' . substr( $condition, $pos + 1 ); + } + + return false; + } + + /** + * Find a single comparison operator (< or >) that is not part of -> or =>. + * + * @param string $condition The condition string. + * @param string $operator The operator to find (< or >). + * + * @return false|int The position of the operator, or false if not found. + */ + private function findComparisonOperator( $condition, $operator ) { + $pos = 0; + + while ( ( $pos = strpos( $condition, $operator, $pos ) ) !== false ) { + // Check character before to avoid matching -> or =>. + if ( $pos > 0 ) { + $charBefore = $condition[ $pos - 1 ]; + + // Skip if this is part of ->, =>, <=, >=, or a multi-char comparison. + if ( $charBefore === '-' || $charBefore === '=' || $charBefore === '<' || $charBefore === '>' || $charBefore === '!' ) { + ++$pos; + continue; } + } - return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + // Check character after to avoid matching <=, >=, =>. + if ( $pos < strlen( $condition ) - 1 ) { + $charAfter = $condition[ $pos + 1 ]; + + if ( $charAfter === '=' || $charAfter === '>' ) { + ++$pos; + continue; + } } + + return $pos; } return false; diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php index e43021a55f..37ec8f41e3 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnSniff.php @@ -322,7 +322,7 @@ private function hasTopLevelOperator( $condition ) { * @return false|string The flipped condition, or false if not a simple comparison. */ private function flipComparisonOperator( $condition ) { - // Map of operators to their opposites. + // Map of operators to their opposites (check longer ones first). $operatorMap = array( '!==' => '===', '===' => '!==', @@ -330,22 +330,72 @@ private function flipComparisonOperator( $condition ) { '==' => '!=', '>=' => '<', '<=' => '>', - '>' => '<=', - '<' => '>=', ); + // Make sure this is a simple comparison (no && or ||). + if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { + return false; + } + // Check for each operator (check longer ones first). foreach ( $operatorMap as $op => $opposite ) { $pos = strpos( $condition, $op ); - if ( $pos !== false ) { - // Make sure this is a simple comparison (no && or ||). - if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { - return false; + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + } + } + + // Handle single < and > separately to avoid matching -> or =>. + $pos = $this->findComparisonOperator( $condition, '>' ); + + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '<=' . substr( $condition, $pos + 1 ); + } + + $pos = $this->findComparisonOperator( $condition, '<' ); + + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '>=' . substr( $condition, $pos + 1 ); + } + + return false; + } + + /** + * Find a single comparison operator (< or >) that is not part of -> or =>. + * + * @param string $condition The condition string. + * @param string $operator The operator to find (< or >). + * + * @return false|int The position of the operator, or false if not found. + */ + private function findComparisonOperator( $condition, $operator ) { + $pos = 0; + + while ( ( $pos = strpos( $condition, $operator, $pos ) ) !== false ) { + // Check character before to avoid matching -> or =>. + if ( $pos > 0 ) { + $charBefore = $condition[ $pos - 1 ]; + + // Skip if this is part of ->, =>, <=, >=, or a multi-char comparison. + if ( $charBefore === '-' || $charBefore === '=' || $charBefore === '<' || $charBefore === '>' || $charBefore === '!' ) { + ++$pos; + continue; } + } - return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + // Check character after to avoid matching <=, >=, =>. + if ( $pos < strlen( $condition ) - 1 ) { + $charAfter = $condition[ $pos + 1 ]; + + if ( $charAfter === '=' || $charAfter === '>' ) { + ++$pos; + continue; + } } + + return $pos; } return false; diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnVariableSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnVariableSniff.php index 20bf1e1e55..d673b62f8d 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnVariableSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipIfToEarlyReturnVariableSniff.php @@ -557,13 +557,7 @@ private function hasTopLevelOperator( $condition ) { * @return false|string The flipped condition, or false if not a simple comparison. */ private function flipComparisonOperator( $condition ) { - // Skip if condition contains object operators (->), array access, or method calls. - // These can be confused with comparison operators. - if ( strpos( $condition, '->' ) !== false || strpos( $condition, '::' ) !== false ) { - return false; - } - - // Map of operators to their opposites. + // Map of operators to their opposites (check longer ones first). $operatorMap = array( '!==' => '===', '===' => '!==', @@ -571,22 +565,72 @@ private function flipComparisonOperator( $condition ) { '==' => '!=', '>=' => '<', '<=' => '>', - '>' => '<=', - '<' => '>=', ); + // Make sure this is a simple comparison (no && or ||). + if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { + return false; + } + // Check for each operator (check longer ones first). foreach ( $operatorMap as $op => $opposite ) { $pos = strpos( $condition, $op ); - if ( $pos !== false ) { - // Make sure this is a simple comparison (no && or ||). - if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { - return false; + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + } + } + + // Handle single < and > separately to avoid matching -> or =>. + $pos = $this->findComparisonOperator( $condition, '>' ); + + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '<=' . substr( $condition, $pos + 1 ); + } + + $pos = $this->findComparisonOperator( $condition, '<' ); + + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '>=' . substr( $condition, $pos + 1 ); + } + + return false; + } + + /** + * Find a single comparison operator (< or >) that is not part of -> or =>. + * + * @param string $condition The condition string. + * @param string $operator The operator to find (< or >). + * + * @return false|int The position of the operator, or false if not found. + */ + private function findComparisonOperator( $condition, $operator ) { + $pos = 0; + + while ( ( $pos = strpos( $condition, $operator, $pos ) ) !== false ) { + // Check character before to avoid matching -> or =>. + if ( $pos > 0 ) { + $charBefore = $condition[ $pos - 1 ]; + + // Skip if this is part of ->, =>, <=, >=, or a multi-char comparison. + if ( $charBefore === '-' || $charBefore === '=' || $charBefore === '<' || $charBefore === '>' || $charBefore === '!' ) { + ++$pos; + continue; } + } - return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + // Check character after to avoid matching <=, >=, =>. + if ( $pos < strlen( $condition ) - 1 ) { + $charAfter = $condition[ $pos + 1 ]; + + if ( $charAfter === '=' || $charAfter === '>' ) { + ++$pos; + continue; + } } + + return $pos; } return false; diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLargeIfSmallElseSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLargeIfSmallElseSniff.php index 4f2880845b..8ec9bbb86a 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLargeIfSmallElseSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLargeIfSmallElseSniff.php @@ -366,7 +366,7 @@ private function hasTopLevelOperator( $condition ) { * @return false|string The flipped condition, or false if not a simple comparison. */ private function flipComparisonOperator( $condition ) { - // Map of operators to their opposites. + // Map of operators to their opposites (check longer ones first). $operatorMap = array( '!==' => '===', '===' => '!==', @@ -374,27 +374,72 @@ private function flipComparisonOperator( $condition ) { '==' => '!=', '>=' => '<', '<=' => '>', - '>' => '<=', - '<' => '>=', ); + // Make sure this is a simple comparison (no && or ||). + if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { + return false; + } + // Check for each operator (check longer ones first). foreach ( $operatorMap as $op => $opposite ) { $pos = strpos( $condition, $op ); - if ( $pos !== false ) { - // Make sure this is a simple comparison (no && or ||). - if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { - return false; - } + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + } + } + + // Handle single < and > separately to avoid matching -> or =>. + $pos = $this->findComparisonOperator( $condition, '>' ); - // Skip if this is part of -> (object operator). - if ( $op === '>' && $pos > 0 && $condition[ $pos - 1 ] === '-' ) { + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '<=' . substr( $condition, $pos + 1 ); + } + + $pos = $this->findComparisonOperator( $condition, '<' ); + + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '>=' . substr( $condition, $pos + 1 ); + } + + return false; + } + + /** + * Find a single comparison operator (< or >) that is not part of -> or =>. + * + * @param string $condition The condition string. + * @param string $operator The operator to find (< or >). + * + * @return false|int The position of the operator, or false if not found. + */ + private function findComparisonOperator( $condition, $operator ) { + $pos = 0; + + while ( ( $pos = strpos( $condition, $operator, $pos ) ) !== false ) { + // Check character before to avoid matching -> or =>. + if ( $pos > 0 ) { + $charBefore = $condition[ $pos - 1 ]; + + // Skip if this is part of ->, =>, <=, >=, or a multi-char comparison. + if ( $charBefore === '-' || $charBefore === '=' || $charBefore === '<' || $charBefore === '>' || $charBefore === '!' ) { + ++$pos; continue; } + } - return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + // Check character after to avoid matching <=, >=, =>. + if ( $pos < strlen( $condition ) - 1 ) { + $charAfter = $condition[ $pos + 1 ]; + + if ( $charAfter === '=' || $charAfter === '>' ) { + ++$pos; + continue; + } } + + return $pos; } return false; diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLoopIfElseToContinueSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLoopIfElseToContinueSniff.php index 2dc70782df..a2f5928519 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLoopIfElseToContinueSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLoopIfElseToContinueSniff.php @@ -381,6 +381,7 @@ private function hasTopLevelOperator( $condition ) { * @return false|string */ private function flipComparisonOperator( $condition ) { + // Map of operators to their opposites (check longer ones first). $operatorMap = array( '!==' => '===', '===' => '!==', @@ -388,25 +389,72 @@ private function flipComparisonOperator( $condition ) { '==' => '!=', '>=' => '<', '<=' => '>', - '>' => '<=', - '<' => '>=', ); + // Make sure this is a simple comparison (no && or ||). + if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { + return false; + } + + // Check for each operator (check longer ones first). foreach ( $operatorMap as $op => $opposite ) { $pos = strpos( $condition, $op ); - if ( $pos !== false ) { - if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { - return false; - } + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + } + } + + // Handle single < and > separately to avoid matching -> or =>. + $pos = $this->findComparisonOperator( $condition, '>' ); - // Skip if this is part of -> (object operator). - if ( $op === '>' && $pos > 0 && $condition[ $pos - 1 ] === '-' ) { + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '<=' . substr( $condition, $pos + 1 ); + } + + $pos = $this->findComparisonOperator( $condition, '<' ); + + if ( false !== $pos ) { + return substr( $condition, 0, $pos ) . '>=' . substr( $condition, $pos + 1 ); + } + + return false; + } + + /** + * Find a single comparison operator (< or >) that is not part of -> or =>. + * + * @param string $condition The condition string. + * @param string $operator The operator to find (< or >). + * + * @return false|int The position of the operator, or false if not found. + */ + private function findComparisonOperator( $condition, $operator ) { + $pos = 0; + + while ( ( $pos = strpos( $condition, $operator, $pos ) ) !== false ) { + // Check character before to avoid matching -> or =>. + if ( $pos > 0 ) { + $charBefore = $condition[ $pos - 1 ]; + + // Skip if this is part of ->, =>, <=, >=, or a multi-char comparison. + if ( $charBefore === '-' || $charBefore === '=' || $charBefore === '<' || $charBefore === '>' || $charBefore === '!' ) { + ++$pos; continue; } + } - return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + // Check character after to avoid matching <=, >=, =>. + if ( $pos < strlen( $condition ) - 1 ) { + $charAfter = $condition[ $pos + 1 ]; + + if ( $charAfter === '=' || $charAfter === '>' ) { + ++$pos; + continue; + } } + + return $pos; } return false;