From fb73194940f22a961a503c066ec162937120cc43 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Mon, 19 Jan 2026 16:04:35 -0400 Subject: [PATCH 1/2] New sniff to change if else in loop to continue when if condition is larger than else --- classes/helpers/FrmAppHelper.php | 15 +- classes/helpers/FrmCSVExportHelper.php | 25 +- classes/models/FrmEntryMeta.php | 20 +- .../FlipLoopIfElseToContinueSniff.php | 461 ++++++++++++++++++ phpcs-sniffs/Formidable/ruleset.xml | 1 + .../FrmSquareLiteAppController.php | 17 +- .../FrmSquareLiteEventsController.php | 11 +- .../FrmStrpLiteEventsController.php | 11 +- stripe/models/FrmStrpLiteAuth.php | 28 +- 9 files changed, 529 insertions(+), 60 deletions(-) create mode 100644 phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLoopIfElseToContinueSniff.php diff --git a/classes/helpers/FrmAppHelper.php b/classes/helpers/FrmAppHelper.php index 35a7d0f227..ecd0b6f186 100644 --- a/classes/helpers/FrmAppHelper.php +++ b/classes/helpers/FrmAppHelper.php @@ -3747,14 +3747,15 @@ public static function format_form_data( &$form ) { $key = $input['name']; - if ( isset( $formatted[ $key ] ) ) { - if ( is_array( $formatted[ $key ] ) ) { - $formatted[ $key ][] = $input['value']; - } else { - $formatted[ $key ] = array( $formatted[ $key ], $input['value'] ); - } - } else { + if ( ! isset( $formatted[ $key ] ) ) { $formatted[ $key ] = $input['value']; + continue; + } + + if ( is_array( $formatted[ $key ] ) ) { + $formatted[ $key ][] = $input['value']; + } else { + $formatted[ $key ] = array( $formatted[ $key ], $input['value'] ); } } diff --git a/classes/helpers/FrmCSVExportHelper.php b/classes/helpers/FrmCSVExportHelper.php index 5ae615821c..90956ddf85 100644 --- a/classes/helpers/FrmCSVExportHelper.php +++ b/classes/helpers/FrmCSVExportHelper.php @@ -418,21 +418,22 @@ private static function csv_headings( &$headings ) { // phpcs:ignore SlevomatCod $flat = array(); foreach ( $headings as $key => $heading ) { - if ( is_array( $heading ) ) { - $repeater_id = str_replace( 'repeater', '', $key ); - $repeater_headings = array(); + if ( ! is_array( $heading ) ) { + $flat[ $key ] = $heading; + continue; + } - foreach ( $fields_by_repeater_id[ $repeater_id ] as $col ) { - $repeater_headings += self::field_headings( $col ); - } + $repeater_id = str_replace( 'repeater', '', $key ); + $repeater_headings = array(); - for ( $i = 0; $i < $max[ $repeater_id ]; $i++ ) { - foreach ( $repeater_headings as $repeater_key => $repeater_name ) { - $flat[ $repeater_key . '[' . $i . ']' ] = $repeater_name; - } + foreach ( $fields_by_repeater_id[ $repeater_id ] as $col ) { + $repeater_headings += self::field_headings( $col ); + } + + for ( $i = 0; $i < $max[ $repeater_id ]; $i++ ) { + foreach ( $repeater_headings as $repeater_key => $repeater_name ) { + $flat[ $repeater_key . '[' . $i . ']' ] = $repeater_name; } - } else { - $flat[ $key ] = $heading; } } diff --git a/classes/models/FrmEntryMeta.php b/classes/models/FrmEntryMeta.php index 4aced90fe6..fb91fcfa62 100644 --- a/classes/models/FrmEntryMeta.php +++ b/classes/models/FrmEntryMeta.php @@ -152,18 +152,18 @@ public static function update_entry_metas( $entry_id, $values ) { self::get_value_to_save( compact( 'field', 'field_id', 'entry_id' ), $meta_value ); - // phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict - if ( $previous_field_ids && in_array( $field_id, $previous_field_ids ) ) { - if ( $meta_value === array() || ( ! is_array( $meta_value ) && trim( $meta_value ) === '' ) ) { - // Remove blank fields. - unset( $values_indexed_by_field_id[ $field_id ] ); - } else { - // if value exists, then update it - self::update_entry_meta( $entry_id, $field_id, '', $meta_value ); - } - } else { + if ( ! $previous_field_ids || ! in_array( $field_id, $previous_field_ids, true ) ) { // if value does not exist, then create it self::add_entry_meta( $entry_id, $field_id, '', $meta_value ); + continue; + } + + if ( $meta_value === array() || ( ! is_array( $meta_value ) && trim( $meta_value ) === '' ) ) { + // Remove blank fields. + unset( $values_indexed_by_field_id[ $field_id ] ); + } else { + // if value exists, then update it + self::update_entry_meta( $entry_id, $field_id, '', $meta_value ); } }//end foreach diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLoopIfElseToContinueSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLoopIfElseToContinueSniff.php new file mode 100644 index 0000000000..2dc70782df --- /dev/null +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipLoopIfElseToContinueSniff.php @@ -0,0 +1,461 @@ +getTokens(); + + // Must have scope opener/closer. + if ( ! isset( $tokens[ $stackPtr ]['scope_opener'] ) || ! isset( $tokens[ $stackPtr ]['scope_closer'] ) ) { + return; + } + + $loopOpener = $tokens[ $stackPtr ]['scope_opener']; + $loopCloser = $tokens[ $stackPtr ]['scope_closer']; + + // Find the last statement in the loop (should be an if/else). + $lastToken = $phpcsFile->findPrevious( T_WHITESPACE, $loopCloser - 1, $loopOpener, true ); + + if ( false === $lastToken ) { + return; + } + + // Should be a closing brace. + if ( $tokens[ $lastToken ]['code'] !== T_CLOSE_CURLY_BRACKET ) { + return; + } + + // Find what this brace belongs to. + if ( ! isset( $tokens[ $lastToken ]['scope_condition'] ) ) { + return; + } + + $scopeCondition = $tokens[ $lastToken ]['scope_condition']; + + // Must be an else. + if ( $tokens[ $scopeCondition ]['code'] !== T_ELSE ) { + return; + } + + $elseToken = $scopeCondition; + $elseOpener = $tokens[ $elseToken ]['scope_opener']; + $elseCloser = $tokens[ $elseToken ]['scope_closer']; + + // Find the if that this else belongs to. + $ifToken = $this->findIfForElse( $phpcsFile, $elseToken ); + + if ( false === $ifToken ) { + return; + } + + // Make sure there's no elseif between if and else. + $ifCloser = $tokens[ $ifToken ]['scope_closer']; + $afterIfClose = $phpcsFile->findNext( T_WHITESPACE, $ifCloser + 1, null, true ); + + if ( false !== $afterIfClose && $tokens[ $afterIfClose ]['code'] === T_ELSEIF ) { + return; + } + + $ifOpener = $tokens[ $ifToken ]['scope_opener']; + + // Count lines in if and else blocks. + $ifLineCount = $this->countLinesInScope( $phpcsFile, $ifOpener, $ifCloser ); + $elseLineCount = $this->countLinesInScope( $phpcsFile, $elseOpener, $elseCloser ); + + // Check if if is large and else is small. + if ( $ifLineCount < $this->minIfLines || $elseLineCount > $this->maxElseLines ) { + return; + } + + // Don't trigger if the else is larger than or equal to the if. + if ( $elseLineCount >= $ifLineCount ) { + return; + } + + // Check that the if is the last thing in the loop (nothing after the else closer except whitespace). + $afterElse = $phpcsFile->findNext( T_WHITESPACE, $elseCloser + 1, $loopCloser, true ); + + if ( false !== $afterElse ) { + return; + } + + $fix = $phpcsFile->addFixableError( + 'Large if (%d lines) with small else (%d lines) at end of loop. Consider flipping the condition and using continue to reduce indentation.', + $ifToken, + 'Found', + array( $ifLineCount, $elseLineCount ) + ); + + if ( true === $fix ) { + $this->applyFix( $phpcsFile, $ifToken, $elseToken ); + } + } + + /** + * Find the if token that an else belongs to. + * + * @param File $phpcsFile The file being scanned. + * @param int $elseToken The else token position. + * + * @return false|int + */ + private function findIfForElse( File $phpcsFile, $elseToken ) { + $tokens = $phpcsFile->getTokens(); + + // Look backwards for the if. + for ( $i = $elseToken - 1; $i >= 0; $i-- ) { + if ( $tokens[ $i ]['code'] === T_CLOSE_CURLY_BRACKET ) { + if ( isset( $tokens[ $i ]['scope_condition'] ) ) { + $condition = $tokens[ $i ]['scope_condition']; + + if ( $tokens[ $condition ]['code'] === T_IF ) { + return $condition; + } + } + + break; + } + } + + return false; + } + + /** + * Count the number of lines inside a scope. + * + * @param File $phpcsFile The file being scanned. + * @param int $scopeOpener The scope opener position. + * @param int $scopeCloser The scope closer position. + * + * @return int + */ + private function countLinesInScope( File $phpcsFile, $scopeOpener, $scopeCloser ) { + $tokens = $phpcsFile->getTokens(); + + $startLine = $tokens[ $scopeOpener ]['line']; + $endLine = $tokens[ $scopeCloser ]['line']; + + return max( 0, $endLine - $startLine - 1 ); + } + + /** + * Apply the fix. + * + * @param File $phpcsFile The file being scanned. + * @param int $ifToken The if token position. + * @param int $elseToken The else token position. + * + * @return void + */ + private function applyFix( File $phpcsFile, $ifToken, $elseToken ) { + $tokens = $phpcsFile->getTokens(); + $fixer = $phpcsFile->fixer; + + $ifOpener = $tokens[ $ifToken ]['scope_opener']; + $ifCloser = $tokens[ $ifToken ]['scope_closer']; + $elseOpener = $tokens[ $elseToken ]['scope_opener']; + $elseCloser = $tokens[ $elseToken ]['scope_closer']; + $conditionOpener = $tokens[ $ifToken ]['parenthesis_opener']; + $conditionCloser = $tokens[ $ifToken ]['parenthesis_closer']; + + // Get the original condition. + $originalCondition = ''; + + for ( $i = $conditionOpener + 1; $i < $conditionCloser; $i++ ) { + $originalCondition .= $tokens[ $i ]['content']; + } + $originalCondition = trim( $originalCondition ); + + // Negate the condition. + $negatedCondition = $this->negateCondition( $originalCondition ); + + // Get the if body content. + $ifBodyContent = ''; + + for ( $i = $ifOpener + 1; $i < $ifCloser; $i++ ) { + $ifBodyContent .= $tokens[ $i ]['content']; + } + + // Get the else body content. + $elseBodyContent = ''; + + for ( $i = $elseOpener + 1; $i < $elseCloser; $i++ ) { + $elseBodyContent .= $tokens[ $i ]['content']; + } + + // Dedent the if body. + $ifBodyContent = $this->dedentCode( $ifBodyContent ); + + // Get the indentation. + $ifIndent = $this->getIndentation( $phpcsFile, $ifToken ); + + $fixer->beginChangeset(); + + // Replace the condition with negated version. + for ( $i = $conditionOpener + 1; $i < $conditionCloser; $i++ ) { + $fixer->replaceToken( $i, '' ); + } + $fixer->addContent( $conditionOpener, ' ' . $negatedCondition . ' ' ); + + // Replace the if body with the else body + continue. + $newIfBody = rtrim( $elseBodyContent ) . $phpcsFile->eolChar . $ifIndent . "\t" . 'continue;'; + + for ( $i = $ifOpener + 1; $i < $ifCloser; $i++ ) { + $fixer->replaceToken( $i, '' ); + } + $fixer->addContent( $ifOpener, $phpcsFile->eolChar . $newIfBody . $phpcsFile->eolChar . $ifIndent ); + + // Remove the else keyword, braces, and content. + for ( $i = $ifCloser + 1; $i <= $elseCloser; $i++ ) { + $fixer->replaceToken( $i, '' ); + } + + // Add the dedented if body after the if's closing brace. + $ifBodyContent = rtrim( $ifBodyContent ); + $fixer->addContent( + $ifCloser, + $phpcsFile->eolChar . $phpcsFile->eolChar . + $ifIndent . ltrim( $ifBodyContent ) . $phpcsFile->eolChar + ); + + $fixer->endChangeset(); + } + + /** + * Negate a condition string. + * + * @param string $condition The original condition. + * + * @return string The negated condition. + */ + private function negateCondition( $condition ) { + $condition = trim( $condition ); + + // Check if this is a compound condition. + $isCompound = $this->hasTopLevelOperator( $condition ); + + if ( ! $isCompound ) { + // If condition starts with !, remove it. + if ( strpos( $condition, '! ' ) === 0 ) { + return substr( $condition, 2 ); + } + + if ( strpos( $condition, '!' ) === 0 && strpos( $condition, '!=' ) !== 0 ) { + return substr( $condition, 1 ); + } + + // Try to flip comparison operators. + $flipped = $this->flipComparisonOperator( $condition ); + + if ( $flipped !== false ) { + return $flipped; + } + + return '! ' . $condition; + } + + return '! ( ' . $condition . ' )'; + } + + /** + * Check if a condition has && or || at the top level. + * + * @param string $condition The condition to check. + * + * @return bool + */ + private function hasTopLevelOperator( $condition ) { + $parenDepth = 0; + $len = strlen( $condition ); + + for ( $i = 0; $i < $len; $i++ ) { + $char = $condition[ $i ]; + + if ( $char === '(' ) { + ++$parenDepth; + continue; + } + + if ( $char === ')' ) { + --$parenDepth; + continue; + } + + if ( $parenDepth !== 0 ) { + continue; + } + + if ( $i < $len - 1 ) { + $twoChars = $condition[ $i ] . $condition[ $i + 1 ]; + + if ( $twoChars === '&&' || $twoChars === '||' ) { + return true; + } + } + } + + return false; + } + + /** + * Try to flip a comparison operator. + * + * @param string $condition The condition to flip. + * + * @return false|string + */ + private function flipComparisonOperator( $condition ) { + $operatorMap = array( + '!==' => '===', + '===' => '!==', + '!=' => '==', + '==' => '!=', + '>=' => '<', + '<=' => '>', + '>' => '<=', + '<' => '>=', + ); + + foreach ( $operatorMap as $op => $opposite ) { + $pos = strpos( $condition, $op ); + + if ( $pos !== false ) { + if ( strpos( $condition, '&&' ) !== false || strpos( $condition, '||' ) !== false ) { + return false; + } + + // Skip if this is part of -> (object operator). + if ( $op === '>' && $pos > 0 && $condition[ $pos - 1 ] === '-' ) { + continue; + } + + return substr( $condition, 0, $pos ) . $opposite . substr( $condition, $pos + strlen( $op ) ); + } + } + + return false; + } + + /** + * Get the indentation of a token. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr The token position. + * + * @return string + */ + private function getIndentation( File $phpcsFile, $stackPtr ) { + $tokens = $phpcsFile->getTokens(); + + $lineStart = $stackPtr; + + while ( $lineStart > 0 && $tokens[ $lineStart - 1 ]['line'] === $tokens[ $stackPtr ]['line'] ) { + --$lineStart; + } + + if ( $tokens[ $lineStart ]['code'] === T_WHITESPACE ) { + return $tokens[ $lineStart ]['content']; + } + + return ''; + } + + /** + * Remove one level of indentation from code. + * + * @param string $code The code to dedent. + * + * @return string + */ + private function dedentCode( $code ) { + $lines = explode( "\n", $code ); + $result = array(); + + foreach ( $lines as $line ) { + if ( strpos( $line, "\t" ) === 0 ) { + $line = substr( $line, 1 ); + } elseif ( strpos( $line, ' ' ) === 0 ) { + $line = substr( $line, 4 ); + } + $result[] = $line; + } + + return implode( "\n", $result ); + } +} diff --git a/phpcs-sniffs/Formidable/ruleset.xml b/phpcs-sniffs/Formidable/ruleset.xml index 171cf44d31..d8d0f799fe 100644 --- a/phpcs-sniffs/Formidable/ruleset.xml +++ b/phpcs-sniffs/Formidable/ruleset.xml @@ -44,6 +44,7 @@ + diff --git a/square/controllers/FrmSquareLiteAppController.php b/square/controllers/FrmSquareLiteAppController.php index b25c955487..688e4e2215 100644 --- a/square/controllers/FrmSquareLiteAppController.php +++ b/square/controllers/FrmSquareLiteAppController.php @@ -225,16 +225,17 @@ private static function generate_false_entry() { $k = sanitize_text_field( stripslashes( $k ) ); $v = wp_unslash( $v ); - if ( $k === 'item_meta' ) { - if ( is_array( $v ) ) { - foreach ( $v as $f => $value ) { - FrmAppHelper::sanitize_value( 'wp_kses_post', $value ); - $entry->metas[ absint( $f ) ] = $value; - } - } - } else { + if ( $k !== 'item_meta' ) { FrmAppHelper::sanitize_value( 'wp_kses_post', $v ); $entry->{$k} = $v; + continue; + } + + if ( is_array( $v ) ) { + foreach ( $v as $f => $value ) { + FrmAppHelper::sanitize_value( 'wp_kses_post', $value ); + $entry->metas[ absint( $f ) ] = $value; + } } } diff --git a/square/controllers/FrmSquareLiteEventsController.php b/square/controllers/FrmSquareLiteEventsController.php index acb09e12ba..e324e888ad 100644 --- a/square/controllers/FrmSquareLiteEventsController.php +++ b/square/controllers/FrmSquareLiteEventsController.php @@ -76,13 +76,14 @@ private function process_event_ids( $event_ids ) { $this->event = FrmSquareLiteConnectHelper::get_event( $event_id ); - if ( is_object( $this->event ) ) { - $this->handle_event(); - $this->track_handled_event( $event_id ); - FrmSquareLiteConnectHelper::process_event( $event_id ); - } else { + if ( ! is_object( $this->event ) ) { $this->count_failed_event( $event_id ); + continue; } + + $this->handle_event(); + $this->track_handled_event( $event_id ); + FrmSquareLiteConnectHelper::process_event( $event_id ); } } diff --git a/stripe/controllers/FrmStrpLiteEventsController.php b/stripe/controllers/FrmStrpLiteEventsController.php index 88d6a828c9..a1d85cd8e9 100644 --- a/stripe/controllers/FrmStrpLiteEventsController.php +++ b/stripe/controllers/FrmStrpLiteEventsController.php @@ -478,13 +478,14 @@ private function process_event_ids( $event_ids ) { $this->event = FrmStrpLiteConnectHelper::get_event( $event_id ); - if ( is_object( $this->event ) ) { - $this->handle_event(); - $this->track_handled_event( $event_id ); - FrmStrpLiteConnectHelper::process_event( $event_id ); - } else { + if ( ! is_object( $this->event ) ) { $this->count_failed_event( $event_id ); + continue; } + + $this->handle_event(); + $this->track_handled_event( $event_id ); + FrmStrpLiteConnectHelper::process_event( $event_id ); } } diff --git a/stripe/models/FrmStrpLiteAuth.php b/stripe/models/FrmStrpLiteAuth.php index e415b861be..6a33ede442 100644 --- a/stripe/models/FrmStrpLiteAuth.php +++ b/stripe/models/FrmStrpLiteAuth.php @@ -421,14 +421,15 @@ private static function generate_false_entry() { $k = sanitize_text_field( stripslashes( $k ) ); $v = wp_unslash( $v ); - if ( $k === 'item_meta' ) { - foreach ( $v as $f => $value ) { - FrmAppHelper::sanitize_value( 'wp_kses_post', $value ); - $entry->metas[ absint( $f ) ] = $value; - } - } else { + if ( $k !== 'item_meta' ) { FrmAppHelper::sanitize_value( 'wp_kses_post', $v ); $entry->{$k} = $v; + continue; + } + + foreach ( $v as $f => $value ) { + FrmAppHelper::sanitize_value( 'wp_kses_post', $value ); + $entry->metas[ absint( $f ) ] = $value; } } @@ -450,14 +451,15 @@ private static function format_form_data( &$form ) { foreach ( $form as $input ) { $key = $input['name']; - if ( isset( $formatted[ $key ] ) ) { - if ( is_array( $formatted[ $key ] ) ) { - $formatted[ $key ][] = $input['value']; - } else { - $formatted[ $key ] = array( $formatted[ $key ], $input['value'] ); - } - } else { + if ( ! isset( $formatted[ $key ] ) ) { $formatted[ $key ] = $input['value']; + continue; + } + + if ( is_array( $formatted[ $key ] ) ) { + $formatted[ $key ][] = $input['value']; + } else { + $formatted[ $key ] = array( $formatted[ $key ], $input['value'] ); } } From 1a1fab04a08306fe0e27639beabd0da61e9f24c2 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Mon, 19 Jan 2026 16:11:10 -0400 Subject: [PATCH 2/2] Target older version of phpstan --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 48922e3189..7dc96016f2 100644 --- a/composer.json +++ b/composer.json @@ -35,7 +35,7 @@ "wp-coding-standards/wpcs": "^3.0", "automattic/vipwpcs": "^3.0", "squizlabs/php_codesniffer": "^3.10", - "phpstan/phpstan": "^2.1.33", + "phpstan/phpstan": "2.1.33", "phpstan/extension-installer": "^1.3", "slevomat/coding-standard": "~8.0", "friendsofphp/php-cs-fixer": "^3.54",