Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions classes/controllers/FrmEntriesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ public static function update_hidden_cols( $meta_id, $object_id, $meta_key, $met

foreach ( $meta_value as $mk => $mv ) {
// Remove blank values.
if ( empty( $mv ) ) {
if ( ! $mv ) {
unset( $meta_value[ $mk ] );
}
}
Expand All @@ -392,7 +392,7 @@ public static function update_hidden_cols( $meta_id, $object_id, $meta_key, $met

foreach ( (array) $frm_vars['prev_hidden_cols'] as $prev_hidden ) {
// phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict
if ( empty( $prev_hidden ) || in_array( $prev_hidden, $meta_value ) ) {
if ( ! $prev_hidden || in_array( $prev_hidden, $meta_value ) ) {
// Don't add blank cols or process included cols.
continue;
}
Expand Down Expand Up @@ -550,7 +550,7 @@ private static function user_hidden_columns_for_form( $form_id, $result ) {
$hidden = array();

foreach ( (array) $result as $r ) {
if ( ! empty( $r ) ) {
if ( $r ) {
list( $form_prefix, $field_key ) = explode( '_', $r );

if ( (int) $form_prefix === (int) $form_id ) {
Expand Down
2 changes: 1 addition & 1 deletion classes/controllers/FrmXMLController.php
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ private static function create_pages_for_import( $form ) {
$page_ids = array();

foreach ( (array) $form['pages'] as $for => $name ) {
if ( empty( $name ) ) {
if ( ! $name ) {
// Don't create a page if no title is given.
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion classes/helpers/FrmListHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ protected function get_column_info() {
$sortable = array();

foreach ( $_sortable as $id => $data ) {
if ( empty( $data ) ) {
if ( ! $data ) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion classes/models/FrmEntry.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public static function is_duplicate( $new_values, $values ) {
$diff = array_diff_assoc( $field_metas, $new_meta );

foreach ( $diff as $meta_value ) {
if ( ! empty( $meta_value ) ) {
if ( $meta_value ) {
$is_duplicate = false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion classes/models/FrmInbox.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public function filter_messages( &$messages, $type = 'unread' ) {
foreach ( $messages as $k => $message ) {
$dismissed = isset( $message['dismissed'] ) && isset( $message['dismissed'][ $user_id ] );

if ( empty( $k ) || ! $this->within_valid_timeframe( $message ) || ( $type === 'dismissed' ) !== $dismissed ) {
if ( ! $k || ! $this->within_valid_timeframe( $message ) || ( $type === 'dismissed' ) !== $dismissed ) {
unset( $messages[ $k ] );
} elseif ( ! $this->is_for_user( $message ) ) {
unset( $messages[ $k ] );
Expand Down
2 changes: 1 addition & 1 deletion classes/models/fields/FrmFieldCombo.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ protected function register_sub_fields( array $sub_fields ) {
$defaults = $this->get_default_sub_field();

foreach ( $sub_fields as $name => $sub_field ) {
if ( empty( $sub_field ) ) {
if ( ! $sub_field ) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion classes/views/frm-fields/back-end/layout-classes.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<ul class="frm_code_list frm-full-hover">
<?php
foreach ( FrmFormsHelper::css_classes() as $c => $d ) {
$title = ! empty( $d ) && is_array( $d ) && isset( $d['title'] ) ? $d['title'] : '';
$title = $d && is_array( $d ) && isset( $d['title'] ) ? $d['title'] : '';
?>
<li>
<a href="javascript:void(0);" data-code="<?php echo esc_attr( $c ); ?>" class="frm_insert_code show_frm_classes<?php echo esc_attr( ! empty( $title ) ? ' frm_help' : '' ); ?>" tabindex="0" <?php echo ( ! empty( $title ) ? ' title="' . esc_attr( $title ) . '"' : '' ); ?>>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,298 @@
<?php
/**
* Formidable_Sniffs_CodeAnalysis_RedundantEmptyOnLoopVariableSniff
*
* Detects redundant empty() calls on foreach loop variables.
* Since loop variables are always set when iterating, empty() is redundant
* and can be replaced with a simple falsy check.
*
* @package Formidable\Sniffs
*/

namespace Formidable\Sniffs\CodeAnalysis;

use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Files\File;

/**
* Detects redundant empty() on foreach loop variables.
*
* Bad:
* foreach ($items as $key => $value) {
* if (empty($value)) {
* // ...
* }
* }
*
* Good:
* foreach ($items as $key => $value) {
* if (! $value) {
* // ...
* }
* }
*
* The loop variable is always set during iteration, so empty() is redundant.
*/
class RedundantEmptyOnLoopVariableSniff implements Sniff {

/**
* Returns an array of tokens this test wants to listen for.
*
* @return array
*/
public function register() {
return array( T_EMPTY );
}

/**
* Processes this test, when one of its tokens is encountered.
*
* @param File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the current token in the stack passed in $tokens.
*
* @return void
*/
public function process( File $phpcsFile, $stackPtr ) {
$tokens = $phpcsFile->getTokens();

// Find the opening parenthesis after empty.
$openParen = $phpcsFile->findNext( T_WHITESPACE, $stackPtr + 1, null, true );

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

// Find the variable inside empty().
$variableToken = $phpcsFile->findNext( T_WHITESPACE, $openParen + 1, null, true );

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

$variableName = $tokens[ $variableToken ]['content'];

// Check if the next non-whitespace token is the closing parenthesis.
// This ensures we're dealing with a simple empty($var) call.
$closeParen = $phpcsFile->findNext( T_WHITESPACE, $variableToken + 1, null, true );

if ( false === $closeParen || $tokens[ $closeParen ]['code'] !== T_CLOSE_PARENTHESIS ) {
// Not a simple variable, could be empty($var['key']) or empty($var->prop).
return;
}

// Find the containing foreach loop.
$foreachToken = $this->findContainingForeach( $phpcsFile, $stackPtr );

if ( false === $foreachToken ) {
return;
}

// Check if this variable is a loop variable (key or value).
if ( ! $this->isLoopVariable( $phpcsFile, $foreachToken, $variableName ) ) {
return;
}

// Check if the variable was reassigned before this empty() call.
$foreachOpener = $tokens[ $foreachToken ]['scope_opener'];

if ( $this->wasVariableReassigned( $phpcsFile, $foreachOpener, $stackPtr, $variableName ) ) {
return;
}

$fix = $phpcsFile->addFixableError(
'Redundant empty() on loop variable %s. Use ! %s instead since loop variables are always set.',
$stackPtr,
'Found',
array( $variableName, $variableName )
);

if ( true === $fix ) {
$this->applyFix( $phpcsFile, $stackPtr, $openParen, $closeParen, $variableName );
}
}

/**
* Find the containing foreach loop.
*
* @param File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the current token.
*
* @return false|int The position of the foreach token, or false if not found.
*/
private function findContainingForeach( File $phpcsFile, $stackPtr ) {
$tokens = $phpcsFile->getTokens();

for ( $i = $stackPtr - 1; $i >= 0; $i-- ) {
if ( $tokens[ $i ]['code'] === T_FOREACH ) {
if ( isset( $tokens[ $i ]['scope_opener'], $tokens[ $i ]['scope_closer'] ) ) {
if ( $stackPtr > $tokens[ $i ]['scope_opener'] && $stackPtr < $tokens[ $i ]['scope_closer'] ) {
return $i;
}
}
}
}

return false;
}

/**
* Check if a variable is a loop variable (key or value) in a foreach.
*
* @param File $phpcsFile The file being scanned.
* @param int $foreachToken The foreach token position.
* @param string $variableName The variable name to check.
*
* @return bool True if the variable is a loop variable.
*/
private function isLoopVariable( File $phpcsFile, $foreachToken, $variableName ) {
$tokens = $phpcsFile->getTokens();

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

$parenOpener = $tokens[ $foreachToken ]['parenthesis_opener'];
$parenCloser = $tokens[ $foreachToken ]['parenthesis_closer'];

// Find the 'as' keyword.
$asToken = $phpcsFile->findNext( T_AS, $parenOpener + 1, $parenCloser );

if ( false === $asToken ) {
return false;
}

// Look for variables after 'as'.
$loopVariables = array();

for ( $i = $asToken + 1; $i < $parenCloser; $i++ ) {
if ( $tokens[ $i ]['code'] === T_VARIABLE ) {
$loopVariables[] = $tokens[ $i ]['content'];
}
}

return in_array( $variableName, $loopVariables, true );
}

/**
* Check if a variable was reassigned between two positions.
*
* @param File $phpcsFile The file being scanned.
* @param int $startPos The start position to search from.
* @param int $endPos The end position to search to.
* @param string $variableName The variable name to check.
*
* @return bool True if the variable was reassigned.
*/
private function wasVariableReassigned( File $phpcsFile, $startPos, $endPos, $variableName ) {
$tokens = $phpcsFile->getTokens();

for ( $i = $startPos + 1; $i < $endPos; $i++ ) {
if ( $tokens[ $i ]['code'] !== T_VARIABLE ) {
continue;
}

if ( $tokens[ $i ]['content'] !== $variableName ) {
continue;
}

// Check if this is an assignment.
$nextToken = $phpcsFile->findNext( T_WHITESPACE, $i + 1, null, true );

if ( false !== $nextToken && $tokens[ $nextToken ]['code'] === T_EQUAL ) {
return true;
}
}

return false;
}
Comment thread
Crabcyborg marked this conversation as resolved.

/**
* Apply the fix.
*
* @param File $phpcsFile The file being scanned.
* @param int $emptyToken The empty token position.
* @param int $openParen The opening parenthesis position.
* @param int $closeParen The closing parenthesis position.
* @param string $variableName The variable name.
*
* @return void
*/
private function applyFix( File $phpcsFile, $emptyToken, $openParen, $closeParen, $variableName ) {
$tokens = $phpcsFile->getTokens();
$fixer = $phpcsFile->fixer;

// Check if there's a ! before empty.
$beforeEmpty = $phpcsFile->findPrevious( T_WHITESPACE, $emptyToken - 1, null, true );
$hasNot = false !== $beforeEmpty && $tokens[ $beforeEmpty ]['code'] === T_BOOLEAN_NOT;

$fixer->beginChangeset();

if ( $hasNot ) {
// ! empty($var) becomes $var
// Remove the !
$fixer->replaceToken( $beforeEmpty, '' );

// Remove whitespace between ! and empty if any.
for ( $i = $beforeEmpty + 1; $i < $emptyToken; $i++ ) {
if ( $tokens[ $i ]['code'] === T_WHITESPACE ) {
$fixer->replaceToken( $i, '' );
}
}

// Replace empty( with just the variable.
$fixer->replaceToken( $emptyToken, '' );

for ( $i = $emptyToken + 1; $i <= $openParen; $i++ ) {
$fixer->replaceToken( $i, '' );
}

// Keep the variable, remove whitespace before it.
$variableToken = $phpcsFile->findNext( T_VARIABLE, $openParen + 1, $closeParen );

for ( $i = $openParen + 1; $i < $variableToken; $i++ ) {
$fixer->replaceToken( $i, '' );
}

// Remove whitespace after variable and closing paren.
for ( $i = $variableToken + 1; $i <= $closeParen; $i++ ) {
$fixer->replaceToken( $i, '' );
}
} else {
// empty($var) becomes ! $var
// Replace empty( with ! $var
$fixer->replaceToken( $emptyToken, '!' );

for ( $i = $emptyToken + 1; $i <= $openParen; $i++ ) {
$fixer->replaceToken( $i, '' );
}

// Keep the variable, but ensure proper spacing.
$variableToken = $phpcsFile->findNext( T_VARIABLE, $openParen + 1, $closeParen );

// Replace whitespace before variable with single space.
$hasWhitespace = false;

for ( $i = $openParen + 1; $i < $variableToken; $i++ ) {
if ( $tokens[ $i ]['code'] === T_WHITESPACE ) {
if ( ! $hasWhitespace ) {
$fixer->replaceToken( $i, ' ' );
$hasWhitespace = true;
} else {
$fixer->replaceToken( $i, '' );
}
}
}

if ( ! $hasWhitespace ) {
$fixer->addContentBefore( $variableToken, ' ' );
}

// Remove whitespace after variable and closing paren.
for ( $i = $variableToken + 1; $i <= $closeParen; $i++ ) {
$fixer->replaceToken( $i, '' );
}
}

$fixer->endChangeset();
}
}
3 changes: 2 additions & 1 deletion phpcs-sniffs/Formidable/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
<rule ref="Formidable.CodeAnalysis.SimplifyEmptyTernary" />
<rule ref="Formidable.CodeAnalysis.RedundantEmptyOnParameter" />
<rule ref="Formidable.CodeAnalysis.RedundantEmptyOnAssignedVariable" />
<rule ref="Formidable.CodeAnalysis.RedundantParentheses" />
<rule ref="Formidable.CodeAnalysis.RedundantEmptyOnLoopVariable" />
<rule ref="Formidable.CodeAnalysis.RedundantIntermediateVariable" />
<rule ref="Formidable.CodeAnalysis.RedundantIconEchoArg" />
<rule ref="Formidable.CodeAnalysis.RedundantTruthyBeforeStringComparison" />
<rule ref="Formidable.CodeAnalysis.RedundantIssetBeforeNotEmpty" />
Expand Down
2 changes: 1 addition & 1 deletion tests/phpunit/entries/test_FrmShowEntryShortcode.php
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,7 @@ protected function expected_array( $entry, $atts ) {
if ( empty( $atts['include_blank'] ) ) {
foreach ( $expected as $field_key => $value ) {
// phpcs:ignore Universal.Operators.StrictComparisons
if ( $value == '' || empty( $value ) ) {
if ( $value == '' || ! $value ) {
unset( $expected[ $field_key ] );
}
}
Expand Down
Loading