From aabe2dfe2d1ca16d50cf15da3649daeefe134a5f Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Tue, 20 Jan 2026 17:41:19 -0400 Subject: [PATCH 1/4] New sniff to prevent empty on function responses --- classes/controllers/FrmAddonsController.php | 2 +- classes/models/FrmInstallPlugin.php | 2 +- classes/models/FrmSolution.php | 2 +- classes/views/styles/_buttons.php | 2 +- .../components/FrmSliderStyleComponent.php | 14 +- ...rNegationOverEmptyForFunctionCallSniff.php | 314 ++++++++++++++++++ phpcs-sniffs/Formidable/ruleset.xml | 1 + 7 files changed, 326 insertions(+), 11 deletions(-) create mode 100644 phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php diff --git a/classes/controllers/FrmAddonsController.php b/classes/controllers/FrmAddonsController.php index 08f9547130..3a1812967f 100644 --- a/classes/controllers/FrmAddonsController.php +++ b/classes/controllers/FrmAddonsController.php @@ -1362,7 +1362,7 @@ public static function install_addon_api() { } // If empty license, save it now. - if ( empty( self::get_pro_license() ) && function_exists( 'load_formidable_pro' ) ) { + if ( ! self::get_pro_license() && function_exists( 'load_formidable_pro' ) ) { load_formidable_pro(); $license = stripslashes( FrmAppHelper::get_param( 'key', '', 'request', 'sanitize_text_field' ) ); diff --git a/classes/models/FrmInstallPlugin.php b/classes/models/FrmInstallPlugin.php index fbba701816..c8162d99e7 100644 --- a/classes/models/FrmInstallPlugin.php +++ b/classes/models/FrmInstallPlugin.php @@ -82,7 +82,7 @@ public static function ajax_install_plugin() { // Get posted data. $plugin_slug = FrmAppHelper::get_post_param( 'plugin', '', 'sanitize_text_field' ); - if ( ! empty( get_plugins()[ $plugin_slug ] ) ) { + if ( (bool) get_plugins()[ $plugin_slug ] ) { $activate = activate_plugin( $plugin_slug ); } else { // Include necessary files for plugin installation. diff --git a/classes/models/FrmSolution.php b/classes/models/FrmSolution.php index 68aac5deec..9963a1cc15 100644 --- a/classes/models/FrmSolution.php +++ b/classes/models/FrmSolution.php @@ -726,7 +726,7 @@ protected function show_page_links( $step ) { */ protected function is_current_plugin() { $to_redirect = get_transient( FrmOnboardingWizardController::TRANSIENT_NAME ); - return $to_redirect === $this->plugin_slug && empty( $this->is_complete() ); + return $to_redirect === $this->plugin_slug && ! $this->is_complete(); } /** diff --git a/classes/views/styles/_buttons.php b/classes/views/styles/_buttons.php index 5283a3e519..3c4fcdabf0 100644 --- a/classes/views/styles/_buttons.php +++ b/classes/views/styles/_buttons.php @@ -254,7 +254,7 @@ class="frm-style-item-heading"> $frm_style->get_field_name( 'submit_style' ), array( 'div_class' => 'with_frm_style frm_toggle', - 'checked' => empty( $frm_style->get_field_name( 'submit_style' ) ), + 'checked' => ! $frm_style->get_field_name( 'submit_style' ), 'echo' => true, 'aria-label-attr' => __( 'Disable submit button styling', 'formidable' ), ) diff --git a/classes/views/styles/components/FrmSliderStyleComponent.php b/classes/views/styles/components/FrmSliderStyleComponent.php index 9e8c6b7923..9605b1e7e1 100644 --- a/classes/views/styles/components/FrmSliderStyleComponent.php +++ b/classes/views/styles/components/FrmSliderStyleComponent.php @@ -37,7 +37,7 @@ public function __construct( $field_name, $field_value, $data ) { $this->data['unit_measurement'] = $this->detect_unit_measurement(); $this->data['has-multiple-values'] = count( $this->get_values() ) > 1; $this->data['units'] = $this->get_units_list( $data ); - $this->data['value_label'] = empty( $this->detect_unit_measurement() ) ? $field_value : (float) $field_value; + $this->data['value_label'] = ! $this->detect_unit_measurement() ? $field_value : (float) $field_value; $this->init_defaults(); $this->init_icon(); @@ -97,32 +97,32 @@ private function init_multiple_values() { $this->data['vertical'] = array( 'unit' => $this->detect_unit_measurement( $top ), - 'value' => empty( $this->detect_unit_measurement( $top ) ) ? $top : (float) $top, + 'value' => ! $this->detect_unit_measurement( $top ) ? $top : (float) $top, ); $this->data['horizontal'] = array( 'unit' => $this->detect_unit_measurement( $right ), - 'value' => empty( $this->detect_unit_measurement( $right ) ) ? $right : (float) $right, + 'value' => ! $this->detect_unit_measurement( $right ) ? $right : (float) $right, ); $this->data['top'] = array( 'unit' => $this->detect_unit_measurement( $top ), - 'value' => empty( $this->detect_unit_measurement( $top ) ) ? $top : (float) $top, + 'value' => ! $this->detect_unit_measurement( $top ) ? $top : (float) $top, ); $this->data['bottom'] = array( 'unit' => $this->detect_unit_measurement( $bottom ), - 'value' => empty( $this->detect_unit_measurement( $bottom ) ) ? $bottom : (float) $bottom, + 'value' => ! $this->detect_unit_measurement( $bottom ) ? $bottom : (float) $bottom, ); $this->data['left'] = array( 'unit' => $this->detect_unit_measurement( $left ), - 'value' => empty( $this->detect_unit_measurement( $left ) ) ? $left : (float) $left, + 'value' => ! $this->detect_unit_measurement( $left ) ? $left : (float) $left, ); $this->data['right'] = array( 'unit' => $this->detect_unit_measurement( $right ), - 'value' => empty( $this->detect_unit_measurement( $right ) ) ? $right : (float) $right, + 'value' => ! $this->detect_unit_measurement( $right ) ? $right : (float) $right, ); } diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php new file mode 100644 index 0000000000..d7930321a5 --- /dev/null +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php @@ -0,0 +1,314 @@ +getTokens(); + + // Skip "! empty()" patterns - the alternative (bool) isn't better. + $prevToken = $phpcsFile->findPrevious( T_WHITESPACE, $stackPtr - 1, null, true ); + if ( false !== $prevToken && $tokens[ $prevToken ]['code'] === T_BOOLEAN_NOT ) { + return; + } + + // Find the opening parenthesis. + $openParen = $phpcsFile->findNext( T_WHITESPACE, $stackPtr + 1, null, true ); + + if ( false === $openParen || $tokens[ $openParen ]['code'] !== T_OPEN_PARENTHESIS ) { + return; + } + + // Find the matching closing parenthesis. + if ( ! isset( $tokens[ $openParen ]['parenthesis_closer'] ) ) { + return; + } + + $closeParen = $tokens[ $openParen ]['parenthesis_closer']; + + // Get the content inside empty(). + $innerStart = $openParen + 1; + $innerEnd = $closeParen - 1; + + // Skip leading whitespace. + $firstInner = $phpcsFile->findNext( T_WHITESPACE, $innerStart, $closeParen, true ); + + if ( false === $firstInner ) { + return; + } + + // Check if the inner content starts with a function call. + if ( ! $this->isFunctionCall( $phpcsFile, $firstInner, $innerEnd ) ) { + return; + } + + // Get the function call content. + $functionCallContent = $this->getContentBetween( $phpcsFile, $firstInner, $innerEnd ); + + $fix = $phpcsFile->addFixableError( + 'Use negation instead of empty() for function calls. Change "empty( %s )" to "! %s".', + $stackPtr, + 'Found', + array( $functionCallContent, $functionCallContent ) + ); + + if ( true === $fix ) { + $this->applyFix( $phpcsFile, $stackPtr, $openParen, $closeParen, $firstInner, $innerEnd ); + } + } + + /** + * Check if the content starting at $start is a function call. + * + * @param File $phpcsFile The file being scanned. + * @param int $start Start position. + * @param int $end End position. + * + * @return bool + */ + private function isFunctionCall( File $phpcsFile, $start, $end ) { + $tokens = $phpcsFile->getTokens(); + + // Check for regular function call: function_name( + if ( $tokens[ $start ]['code'] === T_STRING ) { + $next = $phpcsFile->findNext( T_WHITESPACE, $start + 1, null, true ); + + if ( false !== $next && $tokens[ $next ]['code'] === T_OPEN_PARENTHESIS ) { + return true; + } + } + + // Check for static method call: Class::method( or self::method( or static::method( + // Skip if the result is used for property access: Class::method()->property + if ( in_array( $tokens[ $start ]['code'], array( T_STRING, T_SELF, T_STATIC, T_PARENT ), true ) ) { + $next = $phpcsFile->findNext( T_WHITESPACE, $start + 1, null, true ); + + if ( false !== $next && $tokens[ $next ]['code'] === T_DOUBLE_COLON ) { + $methodName = $phpcsFile->findNext( T_WHITESPACE, $next + 1, null, true ); + + if ( false !== $methodName && $tokens[ $methodName ]['code'] === T_STRING ) { + $openParen = $phpcsFile->findNext( T_WHITESPACE, $methodName + 1, null, true ); + + if ( false !== $openParen && $tokens[ $openParen ]['code'] === T_OPEN_PARENTHESIS ) { + // Check if there's property/method access after the method call. + if ( isset( $tokens[ $openParen ]['parenthesis_closer'] ) ) { + $closeParen = $tokens[ $openParen ]['parenthesis_closer']; + $afterMethod = $phpcsFile->findNext( T_WHITESPACE, $closeParen + 1, $end + 1, true ); + + if ( false !== $afterMethod && $tokens[ $afterMethod ]['code'] === T_OBJECT_OPERATOR ) { + // Static method followed by ->; check if it ends with property access. + return $this->isMethodCallChain( $phpcsFile, $afterMethod, $end ); + } + } + + return true; + } + } + } + } + + // Check for object method call: $obj->method( + // Skip property access like $obj->property (no parentheses). + if ( $tokens[ $start ]['code'] === T_VARIABLE ) { + $next = $phpcsFile->findNext( T_WHITESPACE, $start + 1, $end + 1, true ); + + // If there's an object operator, this could be property access or method call. + // Only flag as function call if it ends with a method call (has parentheses). + if ( false !== $next && $tokens[ $next ]['code'] === T_OBJECT_OPERATOR ) { + // Walk through potential chained access to find if it ends with a method call. + return $this->isMethodCallChain( $phpcsFile, $next, $end ); + } + } + + return false; + } + + /** + * Check if an object operator chain ends with a method call (has parentheses). + * + * This distinguishes between: + * - $obj->property (property access - should NOT be flagged) + * - $obj->method() (method call - should be flagged) + * - $obj->prop->method() (chained ending in method - should be flagged) + * + * @param File $phpcsFile The file being scanned. + * @param int $objectOperator Position of the T_OBJECT_OPERATOR token. + * @param int $end End position to search within. + * + * @return bool True if the chain ends with a method call. + */ + private function isMethodCallChain( File $phpcsFile, $objectOperator, $end ) { + $tokens = $phpcsFile->getTokens(); + + $current = $objectOperator; + + while ( $current <= $end ) { + // After ->, expect a property/method name. + $memberName = $phpcsFile->findNext( T_WHITESPACE, $current + 1, $end + 1, true ); + + if ( false === $memberName || $tokens[ $memberName ]['code'] !== T_STRING ) { + return false; + } + + // Check what comes after the member name. + $afterMember = $phpcsFile->findNext( T_WHITESPACE, $memberName + 1, $end + 1, true ); + + if ( false === $afterMember ) { + // Nothing after member name - this is property access. + return false; + } + + if ( $tokens[ $afterMember ]['code'] === T_OPEN_PARENTHESIS ) { + // This is a method call. Check if there's more after the closing paren. + if ( ! isset( $tokens[ $afterMember ]['parenthesis_closer'] ) ) { + return true; + } + + $closeParen = $tokens[ $afterMember ]['parenthesis_closer']; + $afterMethod = $phpcsFile->findNext( T_WHITESPACE, $closeParen + 1, $end + 1, true ); + + if ( false === $afterMethod ) { + // Method call is the last thing - this is a function call. + return true; + } + + if ( $tokens[ $afterMethod ]['code'] === T_OBJECT_OPERATOR ) { + // Chained call, continue checking. + $current = $afterMethod; + continue; + } + + // Something else after method - still counts as method call. + return true; + } + + if ( $tokens[ $afterMember ]['code'] === T_OBJECT_OPERATOR ) { + // Chained property access, continue checking. + $current = $afterMember; + continue; + } + + // Something else (like end of expression) - this was property access. + return false; + } + + return false; + } + + /** + * Get the content between two positions. + * + * @param File $phpcsFile The file being scanned. + * @param int $start Start position. + * @param int $end End position. + * + * @return string + */ + private function getContentBetween( File $phpcsFile, $start, $end ) { + $tokens = $phpcsFile->getTokens(); + $content = ''; + + // Skip trailing whitespace. + while ( $end > $start && $tokens[ $end ]['code'] === T_WHITESPACE ) { + --$end; + } + + for ( $i = $start; $i <= $end; $i++ ) { + $content .= $tokens[ $i ]['content']; + } + + return trim( $content ); + } + + /** + * Apply the fix to replace empty(function()) with !function(). + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr The empty token position. + * @param int $openParen The opening parenthesis of empty(). + * @param int $closeParen The closing parenthesis of empty(). + * @param int $innerStart The start of the inner content. + * @param int $innerEnd The end of the inner content. + * + * @return void + */ + private function applyFix( File $phpcsFile, $stackPtr, $openParen, $closeParen, $innerStart, $innerEnd ) { + $tokens = $phpcsFile->getTokens(); + $fixer = $phpcsFile->fixer; + + // Get the function call content (trimmed). + $functionCall = $this->getContentBetween( $phpcsFile, $innerStart, $innerEnd ); + + $fixer->beginChangeset(); + + // Replace empty( with ! (with space for WordPress style). + $fixer->replaceToken( $stackPtr, '! ' ); + + // Remove the opening parenthesis and any whitespace after empty. + for ( $i = $stackPtr + 1; $i <= $openParen; $i++ ) { + $fixer->replaceToken( $i, '' ); + } + + // Keep the inner content but remove the WordPress-style spacing. + // Find the actual start of the function call (skip leading whitespace). + for ( $i = $openParen + 1; $i < $innerStart; $i++ ) { + $fixer->replaceToken( $i, '' ); + } + + // Remove trailing whitespace and closing paren. + // First, find where the actual content ends. + $actualEnd = $innerEnd; + + while ( $actualEnd > $innerStart && $tokens[ $actualEnd ]['code'] === T_WHITESPACE ) { + --$actualEnd; + } + + // Remove whitespace between content and closing paren. + for ( $i = $actualEnd + 1; $i < $closeParen; $i++ ) { + $fixer->replaceToken( $i, '' ); + } + + // Remove the closing parenthesis. + $fixer->replaceToken( $closeParen, '' ); + + $fixer->endChangeset(); + } +} diff --git a/phpcs-sniffs/Formidable/ruleset.xml b/phpcs-sniffs/Formidable/ruleset.xml index f090d57982..8d94960141 100644 --- a/phpcs-sniffs/Formidable/ruleset.xml +++ b/phpcs-sniffs/Formidable/ruleset.xml @@ -52,6 +52,7 @@ + From f674809fb539aedfe78afaed599a7b3b123bafd5 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Tue, 20 Jan 2026 17:42:57 -0400 Subject: [PATCH 2/4] revert change --- classes/models/FrmInstallPlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/models/FrmInstallPlugin.php b/classes/models/FrmInstallPlugin.php index c8162d99e7..fbba701816 100644 --- a/classes/models/FrmInstallPlugin.php +++ b/classes/models/FrmInstallPlugin.php @@ -82,7 +82,7 @@ public static function ajax_install_plugin() { // Get posted data. $plugin_slug = FrmAppHelper::get_post_param( 'plugin', '', 'sanitize_text_field' ); - if ( (bool) get_plugins()[ $plugin_slug ] ) { + if ( ! empty( get_plugins()[ $plugin_slug ] ) ) { $activate = activate_plugin( $plugin_slug ); } else { // Include necessary files for plugin installation. From ae35f88b040d8503120017c90e7aa54464246902 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Tue, 20 Jan 2026 17:43:24 -0400 Subject: [PATCH 3/4] Update sniff --- ...rNegationOverEmptyForFunctionCallSniff.php | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php index d7930321a5..3443dc30f9 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php @@ -80,6 +80,11 @@ public function process( File $phpcsFile, $stackPtr ) { return; } + // Skip if the expression ends with array access like function()['key'] - the key might not exist. + if ( $this->endsWithArrayAccess( $phpcsFile, $firstInner, $innerEnd ) ) { + return; + } + // Get the function call content. $functionCallContent = $this->getContentBetween( $phpcsFile, $firstInner, $innerEnd ); @@ -161,6 +166,32 @@ private function isFunctionCall( File $phpcsFile, $start, $end ) { return false; } + /** + * Check if the expression ends with array access like function()['key']. + * + * @param File $phpcsFile The file being scanned. + * @param int $start Start position. + * @param int $end End position. + * + * @return bool True if expression ends with array access. + */ + private function endsWithArrayAccess( File $phpcsFile, $start, $end ) { + $tokens = $phpcsFile->getTokens(); + + // Find the last non-whitespace token. + $lastToken = $end; + while ( $lastToken > $start && $tokens[ $lastToken ]['code'] === T_WHITESPACE ) { + --$lastToken; + } + + // Check if it ends with ] (closing square bracket). + if ( $tokens[ $lastToken ]['code'] === T_CLOSE_SQUARE_BRACKET ) { + return true; + } + + return false; + } + /** * Check if an object operator chain ends with a method call (has parentheses). * From 8c9c1ca9b404c14a7414b217651d10e4cd0cd690 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Tue, 20 Jan 2026 17:45:20 -0400 Subject: [PATCH 4/4] Run php cs fixer --- .../PreferNegationOverEmptyForFunctionCallSniff.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php index 3443dc30f9..1b8372fac7 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNegationOverEmptyForFunctionCallSniff.php @@ -46,6 +46,7 @@ public function process( File $phpcsFile, $stackPtr ) { // Skip "! empty()" patterns - the alternative (bool) isn't better. $prevToken = $phpcsFile->findPrevious( T_WHITESPACE, $stackPtr - 1, null, true ); + if ( false !== $prevToken && $tokens[ $prevToken ]['code'] === T_BOOLEAN_NOT ) { return; } @@ -180,6 +181,7 @@ private function endsWithArrayAccess( File $phpcsFile, $start, $end ) { // Find the last non-whitespace token. $lastToken = $end; + while ( $lastToken > $start && $tokens[ $lastToken ]['code'] === T_WHITESPACE ) { --$lastToken; }