From 20b67e65de4dbd5aea61180287581dce7db54685 Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Tue, 27 Jan 2026 09:48:29 -0400 Subject: [PATCH 1/3] New sniff to prefer accessing properties from FrmAppHelper::get_settings directly --- classes/controllers/FrmUsageController.php | 3 +- classes/models/FrmHoneypot.php | 3 +- .../CodeAnalysis/FlipNegativeTernarySniff.php | 4 +- .../InlineFrmSettingsReturnSniff.php | 299 ++++++++++++++++++ phpcs-sniffs/Formidable/ruleset.xml | 1 + 5 files changed, 304 insertions(+), 6 deletions(-) create mode 100644 phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php diff --git a/classes/controllers/FrmUsageController.php b/classes/controllers/FrmUsageController.php index c00b5f2ff6..ee2f2de850 100644 --- a/classes/controllers/FrmUsageController.php +++ b/classes/controllers/FrmUsageController.php @@ -78,8 +78,7 @@ public static function add_schedules( $schedules = array() ) { * @return bool */ public static function tracking_allowed() { - $settings = FrmAppHelper::get_settings(); - return $settings->tracking; + return (bool) FrmAppHelper::get_settings()->tracking; } /** diff --git a/classes/models/FrmHoneypot.php b/classes/models/FrmHoneypot.php index 892e2190cb..3b273fd408 100644 --- a/classes/models/FrmHoneypot.php +++ b/classes/models/FrmHoneypot.php @@ -34,8 +34,7 @@ protected function get_option_key() { * @return bool */ private static function is_enabled() { - $frm_settings = FrmAppHelper::get_settings(); - return $frm_settings->honeypot; + return (bool) FrmAppHelper::get_settings()->honeypot; } /** diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipNegativeTernarySniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipNegativeTernarySniff.php index daf19ebf3c..cf0ac1e96c 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipNegativeTernarySniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipNegativeTernarySniff.php @@ -3,9 +3,9 @@ * Sniff to detect ternary conditions using !== and flip them to use ===. * * Detects patterns like: - * return null !== $var ? $value1 : $value2; + * return 'value' !== $var ? $value1 : $value2; * - * These should be flipped to: return null === $var ? $value2 : $value1; + * These should be flipped to: return 'value' === $var ? $value2 : $value1; * * @package Formidable\Sniffs\CodeAnalysis */ diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php new file mode 100644 index 0000000000..a167427e66 --- /dev/null +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php @@ -0,0 +1,299 @@ +foo; + * + * And replaces them with: + * + * return FrmAppHelper::get_settings()->foo; + */ +class InlineFrmSettingsReturnSniff implements Sniff { + + /** + * {@inheritDoc} + */ + public function register() { + return array( T_EQUAL ); + } + + /** + * {@inheritDoc} + */ + public function process( File $phpcsFile, $stackPtr ) { + $tokens = $phpcsFile->getTokens(); + + $variablePtr = $this->getAssignedVariablePointer( $phpcsFile, $stackPtr ); + + if ( false === $variablePtr ) { + return; + } + + $settingsCall = $this->getFrmSettingsCallInfo( $phpcsFile, $stackPtr ); + + if ( false === $settingsCall ) { + return; + } + + $assignmentEnd = $phpcsFile->findNext( T_SEMICOLON, $settingsCall['close_paren'] + 1 ); + + if ( false === $assignmentEnd ) { + return; + } + + $returnPtr = $this->findNextEffective( $phpcsFile, $assignmentEnd + 1 ); + + if ( false === $returnPtr || $tokens[ $returnPtr ]['code'] !== T_RETURN ) { + return; + } + + $returnVarPtr = $this->findNextEffective( $phpcsFile, $returnPtr + 1 ); + + if ( false === $returnVarPtr || $tokens[ $returnVarPtr ]['code'] !== T_VARIABLE ) { + return; + } + + if ( $tokens[ $returnVarPtr ]['content'] !== $tokens[ $variablePtr ]['content'] ) { + return; + } + + $operatorPtr = $this->findNextEffective( $phpcsFile, $returnVarPtr + 1 ); + + if ( false === $operatorPtr || ! in_array( + $tokens[ $operatorPtr ]['code'], + array( T_OBJECT_OPERATOR, T_NULLSAFE_OBJECT_OPERATOR ), + true + ) ) { + return; + } + + $returnEnd = $phpcsFile->findNext( T_SEMICOLON, $returnPtr ); + + if ( false === $returnEnd ) { + return; + } + + if ( $this->variableUsedInRange( $phpcsFile, $tokens[ $returnVarPtr ]['content'], $returnVarPtr + 1, $returnEnd - 1 ) ) { + return; + } + + $suffix = $phpcsFile->getTokensAsString( $returnVarPtr + 1, $returnEnd - $returnVarPtr - 1 ); + $lineStart = $this->getLineStartPtr( $tokens, $variablePtr ); + $indent = $this->getIndentForLine( $tokens, $lineStart ); + + $fixable = $phpcsFile->addFixableError( + 'Return FrmAppHelper::get_settings() inline instead of assigning to %s first.', + $variablePtr, + 'InlineReturn', + array( $tokens[ $variablePtr ]['content'] ) + ); + + if ( false === $fixable ) { + return; + } + + $newContent = $indent . 'return FrmAppHelper::get_settings()' . $suffix . ';'; + + $phpcsFile->fixer->beginChangeset(); + + $phpcsFile->fixer->replaceToken( $lineStart, $newContent ); + + for ( $i = $lineStart + 1; $i <= $returnEnd; $i++ ) { + $phpcsFile->fixer->replaceToken( $i, '' ); + } + + $phpcsFile->fixer->endChangeset(); + } + + /** + * Find the pointer to the variable being assigned. + * + * @param File $phpcsFile File instance. + * @param int $stackPtr The equals token pointer. + * + * @return false|int + */ + private function getAssignedVariablePointer( File $phpcsFile, $stackPtr ) { + $tokens = $phpcsFile->getTokens(); + $variable = $phpcsFile->findPrevious( T_WHITESPACE, $stackPtr - 1, null, true ); + + if ( false === $variable || $tokens[ $variable ]['code'] !== T_VARIABLE ) { + return false; + } + + return $variable; + } + + /** + * Check if the assignment is exactly FrmAppHelper::get_settings(). + * + * @param File $phpcsFile File instance. + * @param int $stackPtr The equals token pointer. + * + * @return array|false + */ + private function getFrmSettingsCallInfo( File $phpcsFile, $stackPtr ) { + $tokens = $phpcsFile->getTokens(); + $start = $this->findNextEffective( $phpcsFile, $stackPtr + 1 ); + + if ( false === $start ) { + return false; + } + + if ( $tokens[ $start ]['code'] === T_NS_SEPARATOR ) { + $start = $this->findNextEffective( $phpcsFile, $start + 1 ); + } + + if ( false === $start || $tokens[ $start ]['code'] !== T_STRING || 'FrmAppHelper' !== $tokens[ $start ]['content'] ) { + return false; + } + + $doubleColon = $this->findNextEffective( $phpcsFile, $start + 1 ); + + if ( false === $doubleColon || $tokens[ $doubleColon ]['code'] !== T_DOUBLE_COLON ) { + return false; + } + + $method = $this->findNextEffective( $phpcsFile, $doubleColon + 1 ); + + if ( false === $method || $tokens[ $method ]['code'] !== T_STRING || 'get_settings' !== $tokens[ $method ]['content'] ) { + return false; + } + + $openParen = $this->findNextEffective( $phpcsFile, $method + 1 ); + + if ( false === $openParen || $tokens[ $openParen ]['code'] !== T_OPEN_PARENTHESIS ) { + return false; + } + + if ( ! isset( $tokens[ $openParen ]['parenthesis_closer'] ) ) { + return false; + } + + $closeParen = $tokens[ $openParen ]['parenthesis_closer']; + + if ( ! $this->argumentsAreEmpty( $phpcsFile, $openParen, $closeParen ) ) { + return false; + } + + return array( + 'open_paren' => $openParen, + 'close_paren' => $closeParen, + ); + } + + /** + * Check whether there are non-whitespace tokens between parentheses. + * + * @param File $phpcsFile File reference. + * @param int $openParen Opening parenthesis index. + * @param int $closeParen Closing parenthesis index. + * + * @return bool + */ + private function argumentsAreEmpty( File $phpcsFile, $openParen, $closeParen ) { + for ( $i = $openParen + 1; $i < $closeParen; $i++ ) { + $code = $phpcsFile->getTokens()[ $i ]['code']; + + if ( ! in_array( $code, array( T_WHITESPACE, T_COMMENT, T_DOC_COMMENT ), true ) ) { + return false; + } + } + + return true; + } + + /** + * Find the first token on the given token's line. + * + * @param array $tokens Token stack. + * @param int $stackPtr Pointer within the stack. + * + * @return int + */ + private function getLineStartPtr( array $tokens, $stackPtr ) { + $line = $tokens[ $stackPtr ]['line']; + + while ( $stackPtr > 0 && $tokens[ $stackPtr - 1 ]['line'] === $line ) { + --$stackPtr; + } + + return $stackPtr; + } + + /** + * Retrieve the indentation for the given line start pointer. + * + * @param array $tokens Token stack. + * @param int $lineStart Pointer to the first token on the line. + * + * @return string + */ + private function getIndentForLine( array $tokens, $lineStart ) { + if ( $tokens[ $lineStart ]['code'] === T_WHITESPACE ) { + return $tokens[ $lineStart ]['content']; + } + + return ''; + } + + /** + * Find the next non-empty token. + * + * @param File $phpcsFile File reference. + * @param int $stackPtr Starting position. + * @param integer $end Optional end pointer. + * + * @return false|int + */ + private function findNextEffective( File $phpcsFile, $stackPtr, $end = null ) { + $tokens = $phpcsFile->getTokens(); + $skip = array( T_WHITESPACE, T_COMMENT, T_DOC_COMMENT ); + + for ( $i = $stackPtr; null === $end ? isset( $tokens[ $i ] ) : $i <= $end; $i++ ) { + if ( ! isset( $tokens[ $i ] ) ) { + break; + } + + if ( ! in_array( $tokens[ $i ]['code'], $skip, true ) ) { + return $i; + } + } + + return false; + } + + /** + * Check if the variable appears again within a range. + * + * @param File $phpcsFile File reference. + * @param string $variable Variable name (including $). + * @param int $start Start pointer. + * @param int $end End pointer. + * + * @return bool + */ + private function variableUsedInRange( File $phpcsFile, $variable, $start, $end ) { + for ( $i = $start; $i <= $end; $i++ ) { + $token = $phpcsFile->getTokens()[ $i ]; + + if ( $token['code'] === T_VARIABLE && $token['content'] === $variable ) { + return true; + } + } + + return false; + } +} diff --git a/phpcs-sniffs/Formidable/ruleset.xml b/phpcs-sniffs/Formidable/ruleset.xml index c4c25220f2..bae2d5326e 100644 --- a/phpcs-sniffs/Formidable/ruleset.xml +++ b/phpcs-sniffs/Formidable/ruleset.xml @@ -47,6 +47,7 @@ + From 3d26bc6641a24402107dc3e0bc7d9e37ade83bee Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Tue, 27 Jan 2026 09:48:46 -0400 Subject: [PATCH 2/3] Run php cs fixer --- .../Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php index a167427e66..cf6b7621f6 100644 --- a/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php +++ b/phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php @@ -254,7 +254,7 @@ private function getIndentForLine( array $tokens, $lineStart ) { * * @param File $phpcsFile File reference. * @param int $stackPtr Starting position. - * @param integer $end Optional end pointer. + * @param int $end Optional end pointer. * * @return false|int */ From 50f2f5a9dfd61c34ff4a025cc892263d4d5866bd Mon Sep 17 00:00:00 2001 From: Mike Letellier Date: Tue, 27 Jan 2026 09:50:57 -0400 Subject: [PATCH 3/3] Set real types --- classes/models/FrmSettings.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/models/FrmSettings.php b/classes/models/FrmSettings.php index 0e8ac0a720..949c7a686d 100644 --- a/classes/models/FrmSettings.php +++ b/classes/models/FrmSettings.php @@ -174,7 +174,7 @@ class FrmSettings { public $current_form = 0; /** - * @var bool + * @var bool|int */ public $tracking; @@ -211,7 +211,7 @@ class FrmSettings { public $custom_css; /** - * @var string + * @var int */ public $honeypot;