Skip to content

Update redundant empty on param sniff to catch boolean assignemnts an… #2858

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.

Sign up for GitHub

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

Merged
Crabcyborg merged 1 commit into from
Jan 22, 2026
Merged

Update redundant empty on param sniff to catch boolean assignemnts an… #2858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion classes/helpers/FrmAppHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3226,7 +3226,7 @@ public static function get_localized_date( $date_format, $date ) {
* @return string Time ago.
*/
public static function human_time_diff( $from, $to = '', $levels = 1 ) {
$now = empty( $to ) && 0 !== $to ? new DateTime() : new DateTime( '@' . $to );
$now = ! $to && 0 !== $to ? new DateTime() : new DateTime( '@' . $to );
$ago = new DateTime( '@' . $from );

// Get the time difference
Expand Down
2 changes: 1 addition & 1 deletion classes/helpers/FrmFormMigratorsHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ private static function is_dismissed( $form, $dismissed = null ) {
$dismissed = get_option( 'frm_dismissed' );
}
// phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict
return ! empty( $dismissed ) && in_array( $form['class'], $dismissed );
return $dismissed && in_array( $form['class'], $dismissed );
}

/**
Expand Down
4 changes: 2 additions & 2 deletions classes/models/FrmField.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1024,14 +1024,14 @@ private static function maybe_include_repeating_fields( $inc_repeat, &$where ) {
* @return void
*/
public static function include_sub_fields( &$results, $inc_embed, $type = 'all', $form_id = '' ) {
$no_sub_forms = empty( $results ) && $type === 'all';
$no_sub_forms = ! $results && $type === 'all';

if ( 'include' !== $inc_embed || $no_sub_forms ) {
return;
}

$form_fields = $results;
$should_get_subforms = $type !== 'all' && $type !== 'form' && ! empty( $form_id );
$should_get_subforms = $type !== 'all' && $type !== 'form' && $form_id;

if ( $should_get_subforms ) {
$form_fields = self::get_all_types_in_form( $form_id, 'form' );
Expand Down
2 changes: 1 addition & 1 deletion classes/models/FrmMigrate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ private function revert_widget_field_size() {
* @return void
*/
private function maybe_convert_migrated_size( &$size ) {
$has_px_size = ! empty( $size ) && str_contains( $size, 'px' );
$has_px_size = $size && str_contains( $size, 'px' );

if ( ! $has_px_size ) {
return;
Expand Down
67 changes: 62 additions & 5 deletions phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnParameterSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ public function register() {
public function process( File $phpcsFile, $stackPtr ) {
$tokens = $phpcsFile->getTokens();

// Only process if empty() is directly inside an if/elseif condition.
if ( ! $this->isInIfCondition( $phpcsFile, $stackPtr ) ) {
// Only process if empty() is in a boolean context (if/elseif condition or with || / &&).
if ( ! $this->isInBooleanContext( $phpcsFile, $stackPtr ) ) {
return;
Comment on lines +54 to 56
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Boolean-context gate still misses plain assignments/ternaries.

The new logic only returns true for if/elseif or when empty() is adjacent to &&/||. Cases like $flag = empty( $param ); or empty( $param ) ? 'yes' : 'no' won’t be flagged, which conflicts with the PR intent to catch boolean assignments/ternaries. Consider broadening isInBooleanContext to cover ternary/assignment usage or dropping the boolean-context guard for parameter-only empty().

Also applies to: 229-369

🤖 Prompt for AI Agents 
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnParameterSniff.php`
around lines 54 - 56, The current boolean-context guard (call to
isInBooleanContext in RedundantEmptyOnParameterSniff) skips cases like
assignments and ternaries (e.g., "$flag = empty($param);" or "empty($param) ?
'a' : 'b'"); update isInBooleanContext to also detect assignment operators ('=')
and ternary tokens ('?' / ':') as valid boolean contexts (or remove the guard
entirely to always check parameter-only empty() usage), then adjust the earlier
return that uses isInBooleanContext to allow these cases; apply the same change
to the other occurrences in the file (the logic used between the ranges
referenced around lines 229-369) so assignments and ternaries are flagged too.

}

Expand Down Expand Up @@ -227,16 +227,21 @@ private function getFunctionParameters( File $phpcsFile, $functionToken ) {
}

/**
* Check if the empty() call is directly inside an if/elseif condition.
* Check if the empty() call is in a boolean context (if/elseif condition or with || / &&).
*
* @param File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the empty token.
*
* @return bool True if inside an if/elseif condition, false otherwise.
* @return bool True if in a boolean context, false otherwise.
*/
private function isInIfCondition( File $phpcsFile, $stackPtr ) {
private function isInBooleanContext( File $phpcsFile, $stackPtr ) {
$tokens = $phpcsFile->getTokens();

// Check if empty() is used with || or && (before or after).
if ( $this->hasAdjacentLogicalOperator( $phpcsFile, $stackPtr ) ) {
return true;
}

// Track parenthesis nesting to find the outermost condition parenthesis.
$parenDepth = 0;

Expand Down Expand Up @@ -310,4 +315,56 @@ private function isInIfCondition( File $phpcsFile, $stackPtr ) {

return false;
}

/**
* Check if empty() has an adjacent logical operator (|| or &&).
*
* @param File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the empty token.
*
* @return bool True if there's an adjacent logical operator.
*/
private function hasAdjacentLogicalOperator( File $phpcsFile, $stackPtr ) {
$tokens = $phpcsFile->getTokens();

// Find the closing paren of empty().
$openParen = $phpcsFile->findNext( T_WHITESPACE, $stackPtr + 1, null, true );

if ( false === $openParen || $tokens[ $openParen ]['code'] !== T_OPEN_PARENTHESIS ) {
return false;
}

if ( ! isset( $tokens[ $openParen ]['parenthesis_closer'] ) ) {
return false;
}

$closeParen = $tokens[ $openParen ]['parenthesis_closer'];

$logicalOperators = array(
T_BOOLEAN_AND,
T_BOOLEAN_OR,
T_LOGICAL_AND,
T_LOGICAL_OR,
);

// Check before empty() (skip ! if present).
$beforeEmpty = $phpcsFile->findPrevious( T_WHITESPACE, $stackPtr - 1, null, true );

if ( false !== $beforeEmpty && $tokens[ $beforeEmpty ]['code'] === T_BOOLEAN_NOT ) {
$beforeEmpty = $phpcsFile->findPrevious( T_WHITESPACE, $beforeEmpty - 1, null, true );
}

if ( false !== $beforeEmpty && in_array( $tokens[ $beforeEmpty ]['code'], $logicalOperators, true ) ) {
return true;
}

// Check after empty().
$afterEmpty = $phpcsFile->findNext( T_WHITESPACE, $closeParen + 1, null, true );

if ( false !== $afterEmpty && in_array( $tokens[ $afterEmpty ]['code'], $logicalOperators, true ) ) {
return true;
}

return false;
}
}
Loading