From 021fcd54cd8d6dfcda648d5b6f1c1047114b4272 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Mon, 12 Jan 2026 22:46:09 -0400 Subject: [PATCH 1/2] Add new custom sniffs for catching redundant truthy and empty checks --- classes/controllers/FrmUsageController.php | 2 +- classes/helpers/FrmAppHelper.php | 2 +- ...ndantTruthyBeforeStringComparisonSniff.php | 445 ++++++++++++++++++ .../SimplifyIssetEmptyCheckSniff.php | 169 +++++++ phpcs.xml | 2 + 5 files changed, 618 insertions(+), 2 deletions(-) create mode 100644 phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantTruthyBeforeStringComparisonSniff.php create mode 100644 phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyIssetEmptyCheckSniff.php diff --git a/classes/controllers/FrmUsageController.php b/classes/controllers/FrmUsageController.php index 1d8e5524c4..c00b5f2ff6 100644 --- a/classes/controllers/FrmUsageController.php +++ b/classes/controllers/FrmUsageController.php @@ -119,7 +119,7 @@ private static function is_forms_list_page() { // Exclude Trash page. $form_type = FrmAppHelper::simple_get( 'form_type' ); - return $form_type && 'published' === $form_type; + return 'published' === $form_type; } /** diff --git a/classes/helpers/FrmAppHelper.php b/classes/helpers/FrmAppHelper.php index e113fe8588..cdb37706a4 100644 --- a/classes/helpers/FrmAppHelper.php +++ b/classes/helpers/FrmAppHelper.php @@ -543,7 +543,7 @@ public static function is_preview_page() { global $pagenow; $action = self::simple_get( 'action', 'sanitize_title' ); - return $pagenow && $pagenow === 'admin-ajax.php' && $action === 'frm_forms_preview'; + return $pagenow === 'admin-ajax.php' && $action === 'frm_forms_preview'; } /** diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantTruthyBeforeStringComparisonSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantTruthyBeforeStringComparisonSniff.php new file mode 100644 index 0000000000..d55134458b --- /dev/null +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantTruthyBeforeStringComparisonSniff.php @@ -0,0 +1,445 @@ +getTokens(); + + // Find the expression before &&. + $beforeAnd = $this->getExpressionBefore( $phpcsFile, $stackPtr ); + + if ( false === $beforeAnd ) { + return; + } + + // Check if the expression before && is a simple variable (truthy check). + if ( ! $this->isSimpleVariable( $phpcsFile, $beforeAnd ) ) { + return; + } + + $variableName = $this->getVariableName( $phpcsFile, $beforeAnd ); + + if ( empty( $variableName ) ) { + return; + } + + // Find the expression after &&. + $afterAnd = $this->getExpressionAfter( $phpcsFile, $stackPtr ); + + if ( false === $afterAnd ) { + return; + } + + // Check if the expression after && is a string comparison with the same variable. + $comparisonInfo = $this->isStringComparisonWithVariable( $phpcsFile, $afterAnd, $variableName ); + + if ( false === $comparisonInfo ) { + return; + } + + // We have a match! Report the error. + $fix = $phpcsFile->addFixableError( + 'Redundant truthy check before string comparison. Simplify "%s && %s === \'%s\'" to "%s === \'%s\'"', + $stackPtr, + 'Found', + array( + $variableName, + $variableName, + $comparisonInfo['string'], + $variableName, + $comparisonInfo['string'], + ) + ); + + if ( true === $fix ) { + $phpcsFile->fixer->beginChangeset(); + + // Remove the truthy check and the && operator. + // Remove from start of the variable to end of &&. + for ( $i = $beforeAnd['start']; $i <= $stackPtr; $i++ ) { + $phpcsFile->fixer->replaceToken( $i, '' ); + } + + // Also remove whitespace after &&. + $afterAndToken = $phpcsFile->findNext( T_WHITESPACE, $stackPtr + 1, null, true ); + for ( $i = $stackPtr + 1; $i < $afterAndToken; $i++ ) { + $phpcsFile->fixer->replaceToken( $i, '' ); + } + + $phpcsFile->fixer->endChangeset(); + } + } + + /** + * Get the expression before the && operator. + * + * @param File $phpcsFile The file being scanned. + * @param int $andPtr The position of the && operator. + * + * @return array|false Array with 'start' and 'end' keys, or false on failure. + */ + private function getExpressionBefore( File $phpcsFile, $andPtr ) { + $tokens = $phpcsFile->getTokens(); + + // Find the last non-whitespace token before &&. + $end = $phpcsFile->findPrevious( T_WHITESPACE, $andPtr - 1, null, true ); + + if ( false === $end ) { + return false; + } + + // Find the start of the expression. + // Look backwards for tokens that could be part of a variable expression. + $start = $end; + $validTokens = array( + T_VARIABLE, + T_STRING, + T_OBJECT_OPERATOR, + T_NULLSAFE_OBJECT_OPERATOR, + T_OPEN_SQUARE_BRACKET, + T_CLOSE_SQUARE_BRACKET, + T_CONSTANT_ENCAPSED_STRING, + T_LNUMBER, + T_DNUMBER, + ); + $bracketDepth = 0; + $stopTokens = array( + T_OPEN_PARENTHESIS, + T_BOOLEAN_AND, + T_BOOLEAN_OR, + T_LOGICAL_AND, + T_LOGICAL_OR, + T_SEMICOLON, + T_OPEN_CURLY_BRACKET, + T_RETURN, + T_IF, + T_ELSEIF, + T_COMMA, + ); + + for ( $i = $end; $i >= 0; $i-- ) { + $code = $tokens[ $i ]['code']; + + // Skip whitespace. + if ( $code === T_WHITESPACE ) { + continue; + } + + // Track bracket depth. + if ( $code === T_CLOSE_SQUARE_BRACKET ) { + ++$bracketDepth; + $start = $i; + continue; + } + + if ( $code === T_OPEN_SQUARE_BRACKET ) { + --$bracketDepth; + $start = $i; + continue; + } + + // If we're inside brackets, continue. + if ( $bracketDepth > 0 ) { + $start = $i; + continue; + } + + // Stop at certain tokens. + if ( in_array( $code, $stopTokens, true ) ) { + break; + } + + // Accept valid expression tokens. + if ( in_array( $code, $validTokens, true ) ) { + $start = $i; + continue; + } + + // Unknown token, stop. + break; + } + + return array( + 'start' => $start, + 'end' => $end, + ); + } + + /** + * Get the expression after the && operator. + * + * @param File $phpcsFile The file being scanned. + * @param int $andPtr The position of the && operator. + * + * @return array|false Array with 'start' and 'end' keys, or false on failure. + */ + private function getExpressionAfter( File $phpcsFile, $andPtr ) { + $tokens = $phpcsFile->getTokens(); + + // Find the first non-whitespace token after &&. + $start = $phpcsFile->findNext( T_WHITESPACE, $andPtr + 1, null, true ); + + if ( false === $start ) { + return false; + } + + // Find the end of the expression (look for the comparison and value). + $end = $start; + $stopTokens = array( + T_BOOLEAN_AND, + T_BOOLEAN_OR, + T_LOGICAL_AND, + T_LOGICAL_OR, + T_SEMICOLON, + T_CLOSE_PARENTHESIS, + T_CLOSE_CURLY_BRACKET, + T_INLINE_THEN, + T_COMMA, + ); + + $parenDepth = 0; + $bracketDepth = 0; + + for ( $i = $start; $i < count( $tokens ); $i++ ) { + $code = $tokens[ $i ]['code']; + + // Track parenthesis depth. + if ( $code === T_OPEN_PARENTHESIS ) { + ++$parenDepth; + $end = $i; + continue; + } + + if ( $code === T_CLOSE_PARENTHESIS ) { + if ( $parenDepth === 0 ) { + break; + } + --$parenDepth; + $end = $i; + continue; + } + + // Track bracket depth. + if ( $code === T_OPEN_SQUARE_BRACKET ) { + ++$bracketDepth; + $end = $i; + continue; + } + + if ( $code === T_CLOSE_SQUARE_BRACKET ) { + --$bracketDepth; + $end = $i; + continue; + } + + // If we're inside parens or brackets, continue. + if ( $parenDepth > 0 || $bracketDepth > 0 ) { + $end = $i; + continue; + } + + // Skip whitespace. + if ( $code === T_WHITESPACE ) { + continue; + } + + // Stop at certain tokens. + if ( in_array( $code, $stopTokens, true ) ) { + break; + } + + $end = $i; + } + + return array( + 'start' => $start, + 'end' => $end, + ); + } + + /** + * Check if the expression is a simple variable (not array access or method call). + * + * @param File $phpcsFile The file being scanned. + * @param array $expr The expression info with 'start' and 'end'. + * + * @return bool + */ + private function isSimpleVariable( File $phpcsFile, $expr ) { + $tokens = $phpcsFile->getTokens(); + + // The expression should be just a single variable token. + $nonWhitespaceCount = 0; + $hasVariable = false; + + for ( $i = $expr['start']; $i <= $expr['end']; $i++ ) { + if ( $tokens[ $i ]['code'] === T_WHITESPACE ) { + continue; + } + + ++$nonWhitespaceCount; + + if ( $tokens[ $i ]['code'] === T_VARIABLE ) { + $hasVariable = true; + } + } + + // Must be exactly one token and it must be a variable. + return $nonWhitespaceCount === 1 && $hasVariable; + } + + /** + * Get the variable name from an expression. + * + * @param File $phpcsFile The file being scanned. + * @param array $expr The expression info with 'start' and 'end'. + * + * @return string|false + */ + private function getVariableName( File $phpcsFile, $expr ) { + $tokens = $phpcsFile->getTokens(); + + for ( $i = $expr['start']; $i <= $expr['end']; $i++ ) { + if ( $tokens[ $i ]['code'] === T_VARIABLE ) { + return $tokens[ $i ]['content']; + } + } + + return false; + } + + /** + * Check if the expression is a string comparison with the given variable. + * + * Returns info about the comparison if it matches, false otherwise. + * + * @param File $phpcsFile The file being scanned. + * @param array $expr The expression info with 'start' and 'end'. + * @param string $variableName The variable name to look for. + * + * @return array|false Array with 'string' key containing the compared string, or false. + */ + private function isStringComparisonWithVariable( File $phpcsFile, $expr, $variableName ) { + $tokens = $phpcsFile->getTokens(); + + // Look for === or == in the expression. + $comparisonPos = false; + + for ( $i = $expr['start']; $i <= $expr['end']; $i++ ) { + $code = $tokens[ $i ]['code']; + + if ( $code === T_IS_IDENTICAL || $code === T_IS_EQUAL ) { + $comparisonPos = $i; + break; + } + } + + if ( false === $comparisonPos ) { + return false; + } + + // Check if the variable is on one side of the comparison. + $leftSide = $phpcsFile->findPrevious( T_WHITESPACE, $comparisonPos - 1, $expr['start'] - 1, true ); + $rightSide = $phpcsFile->findNext( T_WHITESPACE, $comparisonPos + 1, $expr['end'] + 1, true ); + + $variableOnLeft = false !== $leftSide && $tokens[ $leftSide ]['code'] === T_VARIABLE && $tokens[ $leftSide ]['content'] === $variableName; + $variableOnRight = false !== $rightSide && $tokens[ $rightSide ]['code'] === T_VARIABLE && $tokens[ $rightSide ]['content'] === $variableName; + + if ( ! $variableOnLeft && ! $variableOnRight ) { + return false; + } + + // Ensure the variable is used alone (not with property/array access). + // Check for tokens like -> or [ after the variable. + $variablePos = $variableOnLeft ? $leftSide : $rightSide; + $afterVar = $phpcsFile->findNext( T_WHITESPACE, $variablePos + 1, $expr['end'] + 1, true ); + + if ( false !== $afterVar && $afterVar !== $comparisonPos ) { + $afterCode = $tokens[ $afterVar ]['code']; + + // If there's something between the variable and the comparison operator, + // check if it's property/array access. + if ( in_array( $afterCode, array( T_OBJECT_OPERATOR, T_NULLSAFE_OBJECT_OPERATOR, T_OPEN_SQUARE_BRACKET, T_DOUBLE_COLON ), true ) ) { + return false; + } + } + + // Also check before the variable (for cases like 'string' === $var->prop). + if ( $variableOnRight ) { + $beforeVar = $phpcsFile->findPrevious( T_WHITESPACE, $variablePos - 1, $comparisonPos, true ); + + if ( false !== $beforeVar && $beforeVar !== $comparisonPos ) { + return false; + } + } + + // Find the string on the other side. + $stringSide = $variableOnLeft ? $rightSide : $leftSide; + + if ( false === $stringSide || $tokens[ $stringSide ]['code'] !== T_CONSTANT_ENCAPSED_STRING ) { + return false; + } + + $stringValue = $tokens[ $stringSide ]['content']; + + // Remove quotes to get the actual string value. + $stringContent = substr( $stringValue, 1, -1 ); + + // Skip empty strings - truthy check is NOT redundant for empty string comparison. + if ( $stringContent === '' ) { + return false; + } + + // Skip numeric strings - truthy check might not be redundant (e.g., '0' is falsy). + if ( is_numeric( $stringContent ) ) { + return false; + } + + return array( + 'string' => $stringContent, + ); + } +} diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyIssetEmptyCheckSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyIssetEmptyCheckSniff.php new file mode 100644 index 0000000000..ea002751d1 --- /dev/null +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyIssetEmptyCheckSniff.php @@ -0,0 +1,169 @@ +getTokens(); + + // Find the opening parenthesis after isset. + $issetOpenParen = $phpcsFile->findNext( T_WHITESPACE, $stackPtr + 1, null, true ); + + if ( false === $issetOpenParen || $tokens[ $issetOpenParen ]['code'] !== T_OPEN_PARENTHESIS ) { + return; + } + + if ( ! isset( $tokens[ $issetOpenParen ]['parenthesis_closer'] ) ) { + return; + } + + $issetCloseParen = $tokens[ $issetOpenParen ]['parenthesis_closer']; + + // Get the variable/expression inside isset(). + $issetContent = $this->getParenthesesContent( $phpcsFile, $issetOpenParen, $issetCloseParen ); + + if ( empty( $issetContent ) ) { + return; + } + + // Find the && operator after isset(). + $andOperator = $phpcsFile->findNext( T_WHITESPACE, $issetCloseParen + 1, null, true ); + + if ( false === $andOperator || $tokens[ $andOperator ]['code'] !== T_BOOLEAN_AND ) { + return; + } + + // Find empty() after &&. + $emptyToken = $phpcsFile->findNext( T_WHITESPACE, $andOperator + 1, null, true ); + + if ( false === $emptyToken || $tokens[ $emptyToken ]['code'] !== T_EMPTY ) { + return; + } + + // Find the opening parenthesis after empty. + $emptyOpenParen = $phpcsFile->findNext( T_WHITESPACE, $emptyToken + 1, null, true ); + + if ( false === $emptyOpenParen || $tokens[ $emptyOpenParen ]['code'] !== T_OPEN_PARENTHESIS ) { + return; + } + + if ( ! isset( $tokens[ $emptyOpenParen ]['parenthesis_closer'] ) ) { + return; + } + + $emptyCloseParen = $tokens[ $emptyOpenParen ]['parenthesis_closer']; + + // Get the variable/expression inside empty(). + $emptyContent = $this->getParenthesesContent( $phpcsFile, $emptyOpenParen, $emptyCloseParen ); + + if ( empty( $emptyContent ) ) { + return; + } + + // Check if the variables match. + if ( $emptyContent['content'] !== $issetContent['content'] ) { + return; + } + + // We have a match! Report the error. + $fix = $phpcsFile->addFixableError( + 'Simplify "isset( %s ) && empty( %s )" to "isset( %s ) && ! %s"', + $emptyToken, + 'Found', + array( $issetContent['content'], $emptyContent['content'], $issetContent['content'], $issetContent['content'] ) + ); + + if ( true === $fix ) { + $phpcsFile->fixer->beginChangeset(); + + // Replace "empty" with "! $variable" (the whole expression). + $phpcsFile->fixer->replaceToken( $emptyToken, '! ' . $emptyContent['content'] ); + + // Remove everything from after "empty" to end of closing paren. + for ( $i = $emptyToken + 1; $i <= $emptyCloseParen; $i++ ) { + $phpcsFile->fixer->replaceToken( $i, '' ); + } + + $phpcsFile->fixer->endChangeset(); + } + } + + /** + * Get the content inside parentheses as a normalized string. + * + * @param File $phpcsFile The file being scanned. + * @param int $openParen The position of the opening parenthesis. + * @param int $closeParen The position of the closing parenthesis. + * + * @return array|false Array with 'content', 'start', 'end' keys, or false on failure. + */ + private function getParenthesesContent( File $phpcsFile, $openParen, $closeParen ) { + $tokens = $phpcsFile->getTokens(); + $content = ''; + $start = null; + $end = null; + + for ( $i = $openParen + 1; $i < $closeParen; $i++ ) { + if ( $tokens[ $i ]['code'] === T_WHITESPACE ) { + continue; + } + + if ( null === $start ) { + $start = $i; + } + + $end = $i; + $content .= $tokens[ $i ]['content']; + } + + if ( empty( $content ) ) { + return false; + } + + return array( + 'content' => $content, + 'start' => $start, + 'end' => $end, + ); + } +} diff --git a/phpcs.xml b/phpcs.xml index 5833e15d9e..45ff01337e 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -246,7 +246,9 @@ + + From ccd2b50d1b19f247132435d6c2b64d74a2114fd7 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Mon, 12 Jan 2026 22:48:28 -0400 Subject: [PATCH 2/2] Run sniffs through php cs fixer --- .../RedundantTruthyBeforeStringComparisonSniff.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantTruthyBeforeStringComparisonSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantTruthyBeforeStringComparisonSniff.php index d55134458b..0dcf92c4c7 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantTruthyBeforeStringComparisonSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantTruthyBeforeStringComparisonSniff.php @@ -105,6 +105,7 @@ public function process( File $phpcsFile, $stackPtr ) { // Also remove whitespace after &&. $afterAndToken = $phpcsFile->findNext( T_WHITESPACE, $stackPtr + 1, null, true ); + for ( $i = $stackPtr + 1; $i < $afterAndToken; $i++ ) { $phpcsFile->fixer->replaceToken( $i, '' ); } @@ -337,7 +338,7 @@ private function isSimpleVariable( File $phpcsFile, $expr ) { * @param File $phpcsFile The file being scanned. * @param array $expr The expression info with 'start' and 'end'. * - * @return string|false + * @return false|string */ private function getVariableName( File $phpcsFile, $expr ) { $tokens = $phpcsFile->getTokens();