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
2 changes: 1 addition & 1 deletion classes/controllers/FrmFormsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -2355,7 +2355,7 @@ private static function add_forms_to_admin_bar( $actions ) {
array(
'parent' => 'frm-forms',
'id' => 'edit_form_' . $form_id,
'title' => empty( $name ) ? FrmFormsHelper::get_no_title_text() : $name,
'title' => $name ? $name : FrmFormsHelper::get_no_title_text(),
'href' => FrmForm::get_edit_link( $form_id ),
)
);
Expand Down
2 changes: 1 addition & 1 deletion classes/helpers/FrmFieldsHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ public static function get_error_msg( $field, $error ) {
);

$msg = FrmField::get_option( $field, $error );
$msg = empty( $msg ) ? $defaults[ $error ]['part'] : $msg;
$msg = $msg ? $msg : $defaults[ $error ]['part'];
$msg = do_shortcode( $msg );

return self::maybe_replace_substrings_with_field_name( $msg, $error, $field );
Expand Down
4 changes: 2 additions & 2 deletions classes/helpers/FrmXMLHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public static function import_xml_terms( $terms, $imported ) {
array(
'slug' => (string) $t->term_slug,
'description' => (string) $t->term_description,
'parent' => empty( $parent ) ? 0 : $parent,
'parent' => $parent ? $parent : 0,
)
);

Expand Down Expand Up @@ -2385,7 +2385,7 @@ private static function migrate_autoresponder_to_action( $form_options, $form_id
}

if ( $reply_to || $reply_to_name ) {
$new_notification2['post_content']['from'] = ( empty( $reply_to_name ) ? '[sitename]' : $reply_to_name ) . ' <' . ( empty( $reply_to ) ? '[admin_email]' : $reply_to ) . '>'; // phpcs:ignore SlevomatCodingStandard.Files.LineLength.LineTooLong
$new_notification2['post_content']['from'] = ( $reply_to_name ? $reply_to_name : '[sitename]' ) . ' <' . ( $reply_to ? $reply_to : '[admin_email]' ) . '>'; // phpcs:ignore SlevomatCodingStandard.Files.LineLength.LineTooLong
}

$notifications[] = $new_notification2;
Expand Down
2 changes: 1 addition & 1 deletion classes/models/fields/FrmFieldCheckbox.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ protected function extra_field_opts() {
$form_id = $this->get_field_column( 'form_id' );

return array(
'align' => FrmStylesController::get_style_val( 'check_align', ( empty( $form_id ) ? 'default' : $form_id ) ),
'align' => FrmStylesController::get_style_val( 'check_align', ( $form_id ? $form_id : 'default' ) ),
);
}

Expand Down
2 changes: 1 addition & 1 deletion classes/models/fields/FrmFieldRadio.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ protected function extra_field_opts() {
$form_id = $this->get_field_column( 'form_id' );

return array(
'align' => FrmStylesController::get_style_val( 'radio_align', ( empty( $form_id ) ? 'default' : $form_id ) ),
'align' => FrmStylesController::get_style_val( 'radio_align', ( $form_id ? $form_id : 'default' ) ),
);
}

Expand Down
2 changes: 1 addition & 1 deletion classes/models/fields/FrmFieldType.php
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ protected function builder_text_field( $name = '' ) {
* @return string
*/
protected function html_name( $name = '' ) {
$prefix = empty( $name ) ? 'item_meta' : $name;
$prefix = $name ? $name : 'item_meta';
return $prefix . '[' . $this->get_field_column( 'id' ) . ']';
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
<?php
/**
* Sniff to simplify empty() ternaries with function parameters.
*
* @package Formidable\Sniffs\CodeAnalysis
*/

namespace Formidable\Sniffs\CodeAnalysis;

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

/**
* Detects empty($param) ? default : $param and converts to $param ? $param : default.
*
* Bad:
* $prefix = empty( $name ) ? 'item_meta' : $name;
*
* Good:
* $prefix = $name ? $name : 'item_meta';
*
* This works because function parameters are always set, so empty() is redundant.
*/
class SimplifyEmptyTernarySniff 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 closing parenthesis.
if ( ! isset( $tokens[ $openParen ]['parenthesis_closer'] ) ) {
return;
}

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

// Get the variable inside empty().
$varToken = $phpcsFile->findNext( T_WHITESPACE, $openParen + 1, $closeParen, true );

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

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

// Check if there's only the variable inside empty() (no array access, etc.).
$nextInParen = $phpcsFile->findNext( T_WHITESPACE, $varToken + 1, $closeParen, true );

if ( false !== $nextInParen ) {
// There's something else inside empty(), skip.
return;
}
Comment on lines +60 to +75

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard against non-parameter variables before auto-fix.

The sniff’s docstring states it’s for function parameters, but Line 67+ accepts any simple variable. Auto-fixing non-parameter locals can introduce “undefined variable” notices. Consider restricting fixes to variables that are declared as parameters in the current function/closure.

♻️ Proposed fix
@@
-		$variableName = $tokens[ $varToken ]['content'];
+		$variableName = $tokens[ $varToken ]['content'];
+
+		// Only simplify when the variable is a parameter of the current function/closure.
+		if ( ! $this->isFunctionParameter( $phpcsFile, $varToken ) ) {
+			return;
+		}
@@
 	}
+
+	private function isFunctionParameter( File $phpcsFile, $varToken ) {
+		$tokens     = $phpcsFile->getTokens();
+		$conditions = $tokens[ $varToken ]['conditions'] ?? array();
+		$functionPtr = null;
+
+		foreach ( array_reverse( $conditions, true ) as $ptr => $type ) {
+			if ( in_array( $type, array( T_FUNCTION, T_CLOSURE, T_FN ), true ) ) {
+				$functionPtr = $ptr;
+				break;
+			}
+		}
+
+		if ( null === $functionPtr || ! isset( $tokens[ $functionPtr ]['parenthesis_opener'] ) ) {
+			return false;
+		}
+
+		$paramStart = $tokens[ $functionPtr ]['parenthesis_opener'] + 1;
+		$paramEnd   = $tokens[ $functionPtr ]['parenthesis_closer'];
+
+		for ( $param = $phpcsFile->findNext( T_VARIABLE, $paramStart, $paramEnd ); false !== $param; $param = $phpcsFile->findNext( T_VARIABLE, $param + 1, $paramEnd ) ) {
+			if ( $tokens[ $param ]['content'] === $tokens[ $varToken ]['content'] ) {
+				return true;
+			}
+		}
+
+		return false;
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Get the variable inside empty().
$varToken = $phpcsFile->findNext( T_WHITESPACE, $openParen + 1, $closeParen, true );
if ( false === $varToken || $tokens[ $varToken ]['code'] !== T_VARIABLE ) {
return;
}
$variableName = $tokens[ $varToken ]['content'];
// Check if there's only the variable inside empty() (no array access, etc.).
$nextInParen = $phpcsFile->findNext( T_WHITESPACE, $varToken + 1, $closeParen, true );
if ( false !== $nextInParen ) {
// There's something else inside empty(), skip.
return;
}
// Get the variable inside empty().
$varToken = $phpcsFile->findNext( T_WHITESPACE, $openParen + 1, $closeParen, true );
if ( false === $varToken || $tokens[ $varToken ]['code'] !== T_VARIABLE ) {
return;
}
$variableName = $tokens[ $varToken ]['content'];
// Only simplify when the variable is a parameter of the current function/closure.
if ( ! $this->isFunctionParameter( $phpcsFile, $varToken ) ) {
return;
}
// Check if there's only the variable inside empty() (no array access, etc.).
$nextInParen = $phpcsFile->findNext( T_WHITESPACE, $varToken + 1, $closeParen, true );
if ( false !== $nextInParen ) {
// There's something else inside empty(), skip.
return;
}
}
private function isFunctionParameter( File $phpcsFile, $varToken ) {
$tokens = $phpcsFile->getTokens();
$conditions = $tokens[ $varToken ]['conditions'] ?? array();
$functionPtr = null;
foreach ( array_reverse( $conditions, true ) as $ptr => $type ) {
if ( in_array( $type, array( T_FUNCTION, T_CLOSURE, T_FN ), true ) ) {
$functionPtr = $ptr;
break;
}
}
if ( null === $functionPtr || ! isset( $tokens[ $functionPtr ]['parenthesis_opener'] ) ) {
return false;
}
$paramStart = $tokens[ $functionPtr ]['parenthesis_opener'] + 1;
$paramEnd = $tokens[ $functionPtr ]['parenthesis_closer'];
for ( $param = $phpcsFile->findNext( T_VARIABLE, $paramStart, $paramEnd ); false !== $param; $param = $phpcsFile->findNext( T_VARIABLE, $param + 1, $paramEnd ) ) {
if ( $tokens[ $param ]['content'] === $tokens[ $varToken ]['content'] ) {
return true;
}
}
return false;
}
🤖 Prompt for AI Agents
In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyEmptyTernarySniff.php`
around lines 60 - 75, The current check accepts any simple variable inside
empty() (using $varToken, $variableName and $openParen/$closeParen) but the
auto-fix should only run for parameters; change the guard so before returning
you resolve the surrounding function/closure and verify $variableName is
declared in its parameter list (i.e. find the enclosing T_FUNCTION/T_CLOSURE
token and inspect its parameter tokens) and only allow the sniff/fixer to
proceed when the variable matches one of those parameters; otherwise return
without attempting an auto-fix to avoid introducing undefined variable notices.


// Find the ternary operator after empty().
$ternaryOp = $phpcsFile->findNext( T_WHITESPACE, $closeParen + 1, null, true );

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

// Find the colon.
$colonOp = $phpcsFile->findNext( T_INLINE_ELSE, $ternaryOp + 1 );

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

// Get the "then" part (between ? and :).
$thenStart = $phpcsFile->findNext( T_WHITESPACE, $ternaryOp + 1, $colonOp, true );

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

// Find the end of the ternary (semicolon or other terminator).
$ternaryEnd = $phpcsFile->findNext( array( T_SEMICOLON, T_COMMA, T_CLOSE_PARENTHESIS, T_CLOSE_SQUARE_BRACKET ), $colonOp + 1 );

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

// Get the "else" part (between : and end).
$elseStart = $phpcsFile->findNext( T_WHITESPACE, $colonOp + 1, $ternaryEnd, true );

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

// Check if the else part is just the same variable.
if ( $tokens[ $elseStart ]['code'] !== T_VARIABLE || $tokens[ $elseStart ]['content'] !== $variableName ) {
return;
}

// Check there's nothing else in the else part.
$nextInElse = $phpcsFile->findNext( T_WHITESPACE, $elseStart + 1, $ternaryEnd, true );

if ( false !== $nextInElse ) {
// There's something else in the else part, skip.
return;
}

// Get the default value (the "then" part).
$defaultValue = $phpcsFile->getTokensAsString( $thenStart, $colonOp - $thenStart );
$defaultValue = trim( $defaultValue );

$fix = $phpcsFile->addFixableError(
'Simplify empty( %s ) ? %s : %s to %s ? %s : %s.',
$stackPtr,
'SimplifyEmptyTernary',
array( $variableName, $defaultValue, $variableName, $variableName, $variableName, $defaultValue )
);

if ( true === $fix ) {
$phpcsFile->fixer->beginChangeset();

// Remove everything from empty to the end of the ternary.
for ( $i = $stackPtr; $i < $ternaryEnd; $i++ ) {
$phpcsFile->fixer->replaceToken( $i, '' );
}

// Add the new simplified ternary.
$phpcsFile->fixer->addContentBefore( $ternaryEnd, $variableName . ' ? ' . $variableName . ' : ' . $defaultValue );

$phpcsFile->fixer->endChangeset();
}
}
}
1 change: 1 addition & 0 deletions phpcs-sniffs/Formidable/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
<rule ref="Formidable.CodeAnalysis.PreferKsesEcho" />
<rule ref="Formidable.CodeAnalysis.MoveVariableBelowEarlyReturn" />
<rule ref="Formidable.CodeAnalysis.PreferEscHtmlE" />
<rule ref="Formidable.CodeAnalysis.SimplifyEmptyTernary" />
<rule ref="Formidable.CodeAnalysis.StrictComparisonForIntFunctions" />
<rule ref="Formidable.CodeAnalysis.FlipNegativeTernary" />
<rule ref="Formidable.CodeAnalysis.FlipIfToEarlyReturn" />
Expand Down