New sniff to remove unecessary var declarations for used once vars#3045
Merged
Crabcyborg merged 16 commits intoMar 26, 2026
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis pull request performs a widespread refactoring across the codebase to remove intermediate single-use variables and inline function calls at their point of usage. Additionally, a new PHPCS sniff is introduced to automatically detect and enforce this inlining pattern going forward. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene  |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP |  |
Mar 26, 2026 2:21p.m. | Review ↗ |
| JavaScript |  |
Mar 26, 2026 2:21p.m. | Review ↗ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
stripe/helpers/FrmTransLiteListHelper.php (1)
230-254:⚠️ Potential issue | 🔴 CriticalCritical bug:
$form_idsis undefined when used bycompact().The refactoring removed
$form_ids = $this->get_form_ids()but line 234 still usescompact('form_ids', ...), which requires the variable to exist. This causes:
- PHP E_NOTICE for undefined variable
form_idskey missing from$argsarrayget_form_id_column()always returns empty string (form column broken)The
$form_idsvariable cannot be inlined here becausecompact()needs the variable name to exist in scope.🐛 Proposed fix to restore the required variable
public function display_rows() { $date_format = FrmAppHelper::get_date_format(); $gateways = FrmTransLiteAppHelper::get_gateways(); $alt = 0; + $form_ids = $this->get_form_ids(); $args = compact( 'form_ids', 'date_format', 'gateways' ); - // $form_ids is indexed by entry ID. - $this->valid_entry_ids = array_keys( $this->get_form_ids() ); + $this->valid_entry_ids = array_keys( $form_ids );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stripe/helpers/FrmTransLiteListHelper.php` around lines 230 - 254, In display_rows(), compact('form_ids', 'date_format', 'gateways') references an undefined $form_ids (causing notices and a missing form_ids arg); fix by restoring a local $form_ids variable (assign $form_ids = $this->get_form_ids() before building $args) or otherwise ensure $args includes a 'form_ids' key populated from $this->get_form_ids(); this will restore correct behavior for get_form_id_column() and display_columns().classes/controllers/FrmFormsController.php (1)
3043-3066:⚠️ Potential issue | 🔴 CriticalKeep
$doing_ajaxas a local for the JS redirect fallback.Line 3066 passes
compact( 'success_url', 'doing_ajax' )but there is no$doing_ajaxassignment in the method scope. This breaks AJAX submissions reachingredirect_after_submit_using_js()and causes a PsalmUndefinedVariableerror since the method checks$args['doing_ajax']at line 3187. Add the assignment before the first condition and reuse it instead of callingFrmAppHelper::doing_ajax()twice:$doing_ajax = FrmAppHelper::doing_ajax();Then replace
FrmAppHelper::doing_ajax()in the condition with$doing_ajax.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/controllers/FrmFormsController.php` around lines 3043 - 3066, The method in FrmFormsController uses FrmAppHelper::doing_ajax() inline and later calls redirect_after_submit_using_js( $args + compact( 'success_url', 'doing_ajax' ) ) without a local $doing_ajax variable, causing an undefined variable and incorrect args; fix by declaring a local $doing_ajax = FrmAppHelper::doing_ajax(); before the first conditional, replace subsequent FrmAppHelper::doing_ajax() checks with $doing_ajax, ensure get_ajax_redirect_response_data and redirect_after_submit_using_js receive $args + compact('success_url','doing_ajax') so $args['doing_ajax'] is set for redirect_after_submit_using_js.
🧹 Nitpick comments (8)
classes/models/FrmSettings.php (1)
624-630: Consider eliminating the unused$detailsvariable.The
$detailsvariable is destructured fromget_editable_roles()but never used within the loop. Only the role names are needed for the capability checks.♻️ Optional refactor to eliminate unused variable
- foreach ( get_editable_roles() as $role => $details ) { + foreach ( array_keys( get_editable_roles() ) as $role ) { if ( in_array( $role, $this->$frm_role, true ) ) { $wp_roles->add_cap( $role, $frm_role ); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/models/FrmSettings.php` around lines 624 - 630, The foreach currently destructures get_editable_roles() into $role and an unused $details; remove the unused variable by iterating only the role keys (e.g. use foreach (array_keys(get_editable_roles()) as $role) { ... }) so the loop keeps checking $this->$frm_role and calling $wp_roles->add_cap / remove_cap without the unused $details variable.classes/helpers/FrmEntriesListHelper.php (1)
365-369: Method called inside loop, consider caching.
$this->get_action_columns()is called on each iteration until$action_colis set. While the method returns a small static array and the short-circuit logic limits calls, caching it before the loop would be more efficient.This is a minor concern given the small array size and early exit.
🔧 Suggested fix
list( $columns, $hidden, , $primary ) = $this->get_column_info(); $action_col = false; +$action_columns = $this->get_action_columns(); foreach ( $columns as $column_name => $column_display_name ) { // ... if ( in_array( $column_name, $hidden, true ) ) { $class .= ' frm_hidden'; - } elseif ( ! $action_col && ! in_array( $column_name, $this->get_action_columns(), true ) ) { + } elseif ( ! $action_col && ! in_array( $column_name, $action_columns, true ) ) { $action_col = $column_name; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/helpers/FrmEntriesListHelper.php` around lines 365 - 369, Cache the result of $this->get_action_columns() before the loop and use that cached array inside the loop instead of calling get_action_columns() repeatedly; update the in_array check that currently uses $this->get_action_columns() to use the cached $action_columns variable while keeping the existing short-circuit logic that sets $action_col when not already set and adds ' frm_hidden' when $column_name is in $hidden.classes/controllers/FrmCronController.php (1)
63-69: Unused$recurrencevariable in loop.The static analysis tool correctly identifies that
$recurrenceis never used within this loop body. Since only the event name is needed here, consider usingarray_keys()instead.🔧 Suggested fix
public static function remove_crons() { - foreach ( self::get_events() as $event => $recurrence ) { + foreach ( array_keys( self::get_events() ) as $event ) { $timestamp = wp_next_scheduled( $event ); if ( false !== $timestamp ) { wp_unschedule_event( $timestamp, $event ); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/controllers/FrmCronController.php` around lines 63 - 69, The foreach in FrmCronController::get_events() iteration declares an unused $recurrence variable; change the loop to iterate only event names (e.g. foreach (array_keys(self::get_events()) as $event)) so you remove the unused $recurrence and keep the existing calls to wp_next_scheduled($event) and wp_unschedule_event($timestamp, $event); this eliminates the unused variable while preserving behavior.tests/phpunit/misc/test_FrmCSVExportHelper.php (1)
28-30:array_keys()called repeatedly inside loop.The previous code likely had
$keys = array_keys($headings)computed once before the loop. Nowarray_keys($headings)is called on every iteration, which is the opposite of the optimization pattern this PR aims to achieve.While this is test code and performance isn't critical, consider keeping the cached variable here for consistency:
🔧 Suggested fix
+ $keys = array_keys( $headings ); foreach ( $expected as $key => $label ) { - $this->assertContains( $key, array_keys( $headings ), "{$label} is not present in CSV Headings" ); + $this->assertContains( $key, $keys, "{$label} is not present in CSV Headings" ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpunit/misc/test_FrmCSVExportHelper.php` around lines 28 - 30, The loop calls array_keys($headings) on every iteration; compute and cache the keys once before the foreach (e.g., $keys = array_keys($headings)) and then use that cached $keys inside the loop with $this->assertContains($key, $keys, ...) to avoid repeated calls while keeping the test semantics the same (references: $expected, $headings, array_keys, $this->assertContains).phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php (4)
276-283: Empty if-block with only a comment is a code smell.The condition is evaluated but no action is taken. Consider restructuring to make the intent clearer—either invert the condition and return early if invalid, or remove the conditional entirely if it's purely informational.
♻️ Proposed fix: Remove the empty block
$afterClose = $phpcsFile->findNext( T_WHITESPACE, $closeParen + 1, $end + 1, true ); - if ( false !== $afterClose && $afterClose !== $end ) { - // There's something after the function call before the semicolon. - // This could be a chained call, array access, etc. - // Still valid but we use the full expression as the "call". - } + // Note: If there's something after the function call before the semicolon + // (chained call, array access, etc.), we still use the full expression as the "call". // Extract just the function/method name (last meaningful part).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php` around lines 276 - 283, The if statement checking the result of $phpcsFile->findNext (using $afterClose, $closeParen and $end) contains only comments and no behavior; remove this empty conditional block to avoid the code smell and leave the surrounding logic as-is in the InlineSingleUseVariableSniff class (InlineSingleUseVariableSniff::process or the enclosing method where $afterClose is computed) so the subsequent code uses $afterClose directly; if early-return logic is actually required, invert the condition to return early instead of leaving an empty block.
335-345: Unused parameter$phpcsFile.The
$phpcsFileparameter is never used in this method. You could remove it and update the call site at line 117.♻️ Proposed fix
-private function hasRequireOrInclude( File $phpcsFile, array $tokens, $functionOpener, $functionCloser ) { +private function hasRequireOrInclude( array $tokens, $functionOpener, $functionCloser ) { $includeTokens = array( T_REQUIRE, T_REQUIRE_ONCE, T_INCLUDE, T_INCLUDE_ONCE ); for ( $i = $functionOpener + 1; $i < $functionCloser; $i++ ) {And at line 117:
-if ( $this->hasRequireOrInclude( $phpcsFile, $tokens, $functionOpener, $functionCloser ) ) { +if ( $this->hasRequireOrInclude( $tokens, $functionOpener, $functionCloser ) ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php` around lines 335 - 345, Remove the unused $phpcsFile parameter from the hasRequireOrInclude method signature and update all call sites to stop passing that argument: change "private function hasRequireOrInclude( File $phpcsFile, array $tokens, $functionOpener, $functionCloser )" to "private function hasRequireOrInclude( array $tokens, $functionOpener, $functionCloser )" and adjust any callers (the place that currently invokes hasRequireOrInclude(...) with a File object and tokens) to call hasRequireOrInclude($tokens, $functionOpener, $functionCloser) instead; ensure parameter order matches the new signature and run tests/static analysis to confirm no other references remain.
237-257: Remove unused variable$lastNameToken.The variable
$lastNameTokenis assigned at line 246 but never used anywhere in the method. This was flagged by static analysis (PHPMD).♻️ Proposed fix
// Collect the function/method name parts. $nameParts = array(); - $lastNameToken = $current; while ( $current < $end ) { $code = $tokens[ $current ]['code']; if ( in_array( $code, array( T_STRING, T_VARIABLE, T_SELF, T_STATIC, T_PARENT ), true ) ) { $nameParts[] = $tokens[ $current ]['content']; - $lastNameToken = $current; } elseif ( in_array( $code, array( T_DOUBLE_COLON, T_OBJECT_OPERATOR, T_NULLSAFE_OBJECT_OPERATOR ), true ) ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php` around lines 237 - 257, In the InlineSingleUseVariableSniff method that collects function/method name parts, remove the unused variable $lastNameToken: delete its initial declaration/assignment ($lastNameToken = $current) and the in-loop update (++$lastNameToken = $current) so only $nameParts, $tokens, $current and $end remain; ensure there are no other references to $lastNameToken in the method before committing the change.
566-573: Consider handling Windows line endings (\r\n).The check
strpos( $wsContent, "\n" ) === 0handles Unix line endings but not Windows-style\r\n, where the newline character is at position 1, not 0. This could leave a stray\rcharacter.♻️ Proposed fix
// Remove the newline after the semicolon. if ( isset( $tokens[ $assignmentEnd + 1 ] ) && $tokens[ $assignmentEnd + 1 ]['code'] === T_WHITESPACE ) { $wsContent = $tokens[ $assignmentEnd + 1 ]['content']; - if ( strpos( $wsContent, "\n" ) === 0 ) { - $phpcsFile->fixer->replaceToken( $assignmentEnd + 1, substr( $wsContent, 1 ) ); + if ( strpos( $wsContent, "\r\n" ) === 0 ) { + $phpcsFile->fixer->replaceToken( $assignmentEnd + 1, substr( $wsContent, 2 ) ); + } elseif ( strpos( $wsContent, "\n" ) === 0 ) { + $phpcsFile->fixer->replaceToken( $assignmentEnd + 1, substr( $wsContent, 1 ) ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php` around lines 566 - 573, The current check only strips a leading "\n" from $wsContent after $assignmentEnd, missing Windows "\r\n" and leaving a stray "\r"; update InlineSingleUseVariableSniff to detect and handle CRLF by checking $wsContent for a leading "\r\n" (remove first two characters) else for a leading "\n" or "\r" (remove first character) before calling $phpcsFile->fixer->replaceToken($assignmentEnd + 1, ...); use $assignmentEnd, $wsContent and $phpcsFile->fixer->replaceToken to locate and apply the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@classes/controllers/FrmFormsController.php`:
- Around line 268-275: The current code calls FrmForm::update($id, $_POST)
before checking antispam state, so antispam_was_on($id) reads the post-update
state; capture the previous antispam setting into a variable (e.g.,
$old_antispam = self::antispam_was_on($id)) before calling FrmForm::update, then
compute $antispam_is_on from $_POST and compare $antispam_is_on !==
$old_antispam to decide whether to call FrmAntiSpam::clear_caches(); ensure you
reference the existing methods FrmForm::update, self::antispam_was_on, and
FrmAntiSpam::clear_caches when making the change.
- Around line 3426-3431: The else branch in front_head() uses $suffix but
$suffix is never defined, causing an undefined variable notice; define $suffix
by assigning FrmAppHelper::js_suffix() near the start of the function (e.g.,
before the if) and then use that $suffix in the wp_register_script fallback line
that registers "formidable" (the line calling wp_register_script with
"/js/formidable{$suffix}.js"), keeping the existing condition that calls
self::has_combo_js_file() and the primary registration of 'formidable' with
frm.min.js.
In `@classes/controllers/FrmWelcomeTourController.php`:
- Around line 625-627: The loop is iterating the wrapper returned by
self::get_steps() (which contains keys like 'keys' and 'steps') instead of the
actual step IDs, causing wrong usage-tracking keys; change the iteration to loop
over the real steps array (e.g., the 'steps' element returned by
self::get_steps()) and then use each step's ID/key when building
$usage_data['completed_step_'.$key]; update the foreach in
FrmWelcomeTourController to pull the steps array from self::get_steps() (and
guard that $option['completed_steps'] exists) so the correct step IDs are used
for usage tracking.
In `@classes/controllers/FrmXMLController.php`:
- Around line 768-772: Cache the result of FrmField::no_save_fields() before
iterating $csv_fields to avoid calling the method on every loop iteration; e.g.
assign $no_save_fields = FrmField::no_save_fields() once prior to the foreach in
FrmXMLController and replace in_array($f->type, FrmField::no_save_fields(),
true) with in_array($f->type, $no_save_fields, true) so the lookup uses the
cached array.
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php`:
- Around line 378-412: The isComplexUsage function misses
pre-increment/pre-decrement because it only inspects tokens after the variable;
update it to also look at the token immediately before $usagePtr (skipping
whitespace) and treat T_INC or T_DEC there as complex usage. Use
$phpcsFile->findPrevious to find the previous non-whitespace token and check
$tokens[ $prevPtr ]['code'] with in_array for T_INC and T_DEC (same strict check
as post-check) so ++$var/--$var are correctly classified as complex in
isComplexUsage.
In `@tests/phpunit/misc/test_FrmAppHelper.php`:
- Around line 408-409: The test reads the query var via
FrmAppHelper::get_query_var('', 'p') before setting up the new post context with
go_to_new_post(), causing a stale value; swap the operations so go_to_new_post()
(the call that sets the new post context) runs first and then capture the result
of FrmAppHelper::get_query_var('', 'p') into $get_post_id and assert it equals
the value returned by go_to_new_post().
---
Outside diff comments:
In `@classes/controllers/FrmFormsController.php`:
- Around line 3043-3066: The method in FrmFormsController uses
FrmAppHelper::doing_ajax() inline and later calls
redirect_after_submit_using_js( $args + compact( 'success_url', 'doing_ajax' ) )
without a local $doing_ajax variable, causing an undefined variable and
incorrect args; fix by declaring a local $doing_ajax =
FrmAppHelper::doing_ajax(); before the first conditional, replace subsequent
FrmAppHelper::doing_ajax() checks with $doing_ajax, ensure
get_ajax_redirect_response_data and redirect_after_submit_using_js receive $args
+ compact('success_url','doing_ajax') so $args['doing_ajax'] is set for
redirect_after_submit_using_js.
In `@stripe/helpers/FrmTransLiteListHelper.php`:
- Around line 230-254: In display_rows(), compact('form_ids', 'date_format',
'gateways') references an undefined $form_ids (causing notices and a missing
form_ids arg); fix by restoring a local $form_ids variable (assign $form_ids =
$this->get_form_ids() before building $args) or otherwise ensure $args includes
a 'form_ids' key populated from $this->get_form_ids(); this will restore correct
behavior for get_form_id_column() and display_columns().
---
Nitpick comments:
In `@classes/controllers/FrmCronController.php`:
- Around line 63-69: The foreach in FrmCronController::get_events() iteration
declares an unused $recurrence variable; change the loop to iterate only event
names (e.g. foreach (array_keys(self::get_events()) as $event)) so you remove
the unused $recurrence and keep the existing calls to wp_next_scheduled($event)
and wp_unschedule_event($timestamp, $event); this eliminates the unused variable
while preserving behavior.
In `@classes/helpers/FrmEntriesListHelper.php`:
- Around line 365-369: Cache the result of $this->get_action_columns() before
the loop and use that cached array inside the loop instead of calling
get_action_columns() repeatedly; update the in_array check that currently uses
$this->get_action_columns() to use the cached $action_columns variable while
keeping the existing short-circuit logic that sets $action_col when not already
set and adds ' frm_hidden' when $column_name is in $hidden.
In `@classes/models/FrmSettings.php`:
- Around line 624-630: The foreach currently destructures get_editable_roles()
into $role and an unused $details; remove the unused variable by iterating only
the role keys (e.g. use foreach (array_keys(get_editable_roles()) as $role) {
... }) so the loop keeps checking $this->$frm_role and calling
$wp_roles->add_cap / remove_cap without the unused $details variable.
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php`:
- Around line 276-283: The if statement checking the result of
$phpcsFile->findNext (using $afterClose, $closeParen and $end) contains only
comments and no behavior; remove this empty conditional block to avoid the code
smell and leave the surrounding logic as-is in the InlineSingleUseVariableSniff
class (InlineSingleUseVariableSniff::process or the enclosing method where
$afterClose is computed) so the subsequent code uses $afterClose directly; if
early-return logic is actually required, invert the condition to return early
instead of leaving an empty block.
- Around line 335-345: Remove the unused $phpcsFile parameter from the
hasRequireOrInclude method signature and update all call sites to stop passing
that argument: change "private function hasRequireOrInclude( File $phpcsFile,
array $tokens, $functionOpener, $functionCloser )" to "private function
hasRequireOrInclude( array $tokens, $functionOpener, $functionCloser )" and
adjust any callers (the place that currently invokes hasRequireOrInclude(...)
with a File object and tokens) to call hasRequireOrInclude($tokens,
$functionOpener, $functionCloser) instead; ensure parameter order matches the
new signature and run tests/static analysis to confirm no other references
remain.
- Around line 237-257: In the InlineSingleUseVariableSniff method that collects
function/method name parts, remove the unused variable $lastNameToken: delete
its initial declaration/assignment ($lastNameToken = $current) and the in-loop
update (++$lastNameToken = $current) so only $nameParts, $tokens, $current and
$end remain; ensure there are no other references to $lastNameToken in the
method before committing the change.
- Around line 566-573: The current check only strips a leading "\n" from
$wsContent after $assignmentEnd, missing Windows "\r\n" and leaving a stray
"\r"; update InlineSingleUseVariableSniff to detect and handle CRLF by checking
$wsContent for a leading "\r\n" (remove first two characters) else for a leading
"\n" or "\r" (remove first character) before calling
$phpcsFile->fixer->replaceToken($assignmentEnd + 1, ...); use $assignmentEnd,
$wsContent and $phpcsFile->fixer->replaceToken to locate and apply the fix.
In `@tests/phpunit/misc/test_FrmCSVExportHelper.php`:
- Around line 28-30: The loop calls array_keys($headings) on every iteration;
compute and cache the keys once before the foreach (e.g., $keys =
array_keys($headings)) and then use that cached $keys inside the loop with
$this->assertContains($key, $keys, ...) to avoid repeated calls while keeping
the test semantics the same (references: $expected, $headings, array_keys,
$this->assertContains).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3b220963-49c1-46b6-aeaf-555f190d4c8b
📒 Files selected for processing (51)
classes/controllers/FrmAddonsController.phpclasses/controllers/FrmAppController.phpclasses/controllers/FrmCronController.phpclasses/controllers/FrmDashboardController.phpclasses/controllers/FrmEmailStylesController.phpclasses/controllers/FrmEntriesAJAXSubmitController.phpclasses/controllers/FrmEntriesController.phpclasses/controllers/FrmFormsController.phpclasses/controllers/FrmInboxController.phpclasses/controllers/FrmSMTPController.phpclasses/controllers/FrmStylesController.phpclasses/controllers/FrmWelcomeTourController.phpclasses/controllers/FrmXMLController.phpclasses/helpers/FrmAppHelper.phpclasses/helpers/FrmEntriesHelper.phpclasses/helpers/FrmEntriesListHelper.phpclasses/helpers/FrmFieldGdprHelper.phpclasses/helpers/FrmFieldGridHelper.phpclasses/helpers/FrmFieldsHelper.phpclasses/helpers/FrmShortcodeHelper.phpclasses/helpers/FrmStylesCardHelper.phpclasses/helpers/FrmXMLHelper.phpclasses/models/FrmAddon.phpclasses/models/FrmCreateFile.phpclasses/models/FrmEntryMeta.phpclasses/models/FrmField.phpclasses/models/FrmFormState.phpclasses/models/FrmInbox.phpclasses/models/FrmSettings.phpclasses/models/FrmStyle.phpclasses/models/FrmStyleApi.phpclasses/models/fields/FrmFieldCaptcha.phpclasses/models/fields/FrmFieldName.phpclasses/models/fields/FrmFieldType.phpclasses/views/styles/components/FrmStyleComponent.phpphpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.phpphpcs-sniffs/Formidable/ruleset.xmlsquare/helpers/FrmSquareLiteAppHelper.phpsquare/helpers/FrmSquareLiteConnectHelper.phpstripe/helpers/FrmStrpLiteAppHelper.phpstripe/helpers/FrmStrpLiteConnectApiAdapter.phpstripe/helpers/FrmStrpLiteConnectHelper.phpstripe/helpers/FrmTransLiteListHelper.phpstripe/models/FrmTransLiteDb.phptests/phpunit/base/FrmUnitTest.phptests/phpunit/entries/test_FrmShowEntryShortcode.phptests/phpunit/forms/test_FrmFormActionsController.phptests/phpunit/misc/test_FrmAppController.phptests/phpunit/misc/test_FrmAppHelper.phptests/phpunit/misc/test_FrmCSVExportHelper.phptests/phpunit/misc/test_FrmHooksController.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php (1)
503-536:⚠️ Potential issue | 🔴 Critical
isComplexUsage()is still too narrow for an autofix.It still misses prefix
++$var/--$var, and it also treats dereference usages like$result[0]or$result->idas safe even though the injected RHS is not parenthesized. That can yield invalid rewrites like++get_count()or precedence changes likeget_result() ?: array()[0].🐛 Conservative guard
private function isComplexUsage( File $phpcsFile, array $tokens, $usagePtr ) { + $beforeUsage = $phpcsFile->findPrevious( T_WHITESPACE, $usagePtr - 1, null, true ); + + if ( false !== $beforeUsage && in_array( $tokens[ $beforeUsage ]['code'], array( T_INC, T_DEC ), true ) ) { + return true; + } + $afterUsage = $phpcsFile->findNext( T_WHITESPACE, $usagePtr + 1, null, true ); @@ if ( in_array( $tokens[ $afterUsage ]['code'], array( T_INC, T_DEC ), true ) ) { return true; } + + if ( in_array( $tokens[ $afterUsage ]['code'], array( T_OPEN_SQUARE_BRACKET, T_OBJECT_OPERATOR, T_NULLSAFE_OBJECT_OPERATOR ), true ) ) { + return true; + } return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php` around lines 503 - 536, The isComplexUsage() check must be expanded: detect prefix increments/decrements by checking the previous non-whitespace token before $usagePtr for T_INC or T_DEC and treat that as complex, and treat any dereference/prop access as complex by also checking the token immediately after the variable (skipping whitespace) for bracket/property operators (T_OPEN_SQUARE_BRACKET, T_OBJECT_OPERATOR, T_NULLSAFE_OBJECT_OPERATOR, and similar tokens like T_DOUBLE_COLON if relevant) so that usages like ++$var, --$var, $result[0], $result->id, or $result?->id are considered complex and prevented from autofix; update isComplexUsage() to include these additional token checks (refer to function isComplexUsage, $usagePtr, $afterUsage and the token codes named above).
🧹 Nitpick comments (1)
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php (1)
255-263: Remove the dead$lastNameTokenstate.It’s assigned here but never read, so PHPMD will keep flagging this helper until it’s dropped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php` around lines 255 - 263, Remove the dead state variable $lastNameToken: it is assigned inside the loop but never read elsewhere, so delete its declaration and all assignments to it (the initial "$lastNameToken = $current;" and the "$lastNameToken = $current;" inside the loop) in InlineSingleUseVariableSniff (the loop that iterates using $current, $end and $tokens while collecting $nameParts); keep the rest of the logic that builds $nameParts unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@classes/helpers/FrmListHelper.php`:
- Line 831: The foreach loop is declaring an unused variable $column_name in the
context of FrmListHelper; change the loop that currently reads foreach (
$this->get_columns() as $col => $column_name ) to only capture the used key
(e.g., foreach ( $this->get_columns() as $col ) ), removing $column_name and
ensuring no other references to $column_name exist in the surrounding code or
methods like get_columns(); keep behavior identical while eliminating the unused
variable.
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php`:
- Around line 153-157: The current guard in InlineSingleUseVariableSniff (the
isUsedInsideLoop check using $tokens, $stackPtr, $usagePtr) is too narrow;
instead ensure the assignment and the usage remain in the same execution context
before offering an autofix: locate the nearest common AST/block/container for
$stackPtr and $usagePtr (not just loop bodies), and reject fixes if they have
different parent statements/blocks, if either sits inside a conditional
predicate, loop predicate, ternary, or different control-structure header, or if
either is inside an expression that could change evaluation frequency or be
skipped (e.g. for/while conditions, if/elseif/else headers, ternary,
boolean-short-circuit contexts). Update the logic that currently calls
isUsedInsideLoop to perform this common-ancestor/same-context check (or add a
new helper method referenced from InlineSingleUseVariableSniff) and return
without autofix when contexts differ.
- Around line 676-689: The fixer currently sets $lineStart by walking back to
the start of the source line and then removes everything through $assignmentEnd,
which can delete unrelated code when other statements share the same line;
modify the logic to anchor deletion to the assignment statement itself by either
(a) finding an $assignmentStart token (search backwards from $varStart for a
statement boundary such as a previous semicolon/closer or the token that begins
the assignment) and use that as the deletion start instead of $lineStart, or (b)
detect if any non-whitespace/comment tokens exist between $lineStart and
$varStart and, if so, skip the autofix (do not call
$phpcsFile->fixer->replaceToken) to avoid removing preceding code; update the
block that computes $lineStart and the removal loop that uses $assignmentEnd
accordingly (references: $varStart, $lineStart, $assignmentEnd, the fixer
replaceToken loop).
---
Duplicate comments:
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php`:
- Around line 503-536: The isComplexUsage() check must be expanded: detect
prefix increments/decrements by checking the previous non-whitespace token
before $usagePtr for T_INC or T_DEC and treat that as complex, and treat any
dereference/prop access as complex by also checking the token immediately after
the variable (skipping whitespace) for bracket/property operators
(T_OPEN_SQUARE_BRACKET, T_OBJECT_OPERATOR, T_NULLSAFE_OBJECT_OPERATOR, and
similar tokens like T_DOUBLE_COLON if relevant) so that usages like ++$var,
--$var, $result[0], $result->id, or $result?->id are considered complex and
prevented from autofix; update isComplexUsage() to include these additional
token checks (refer to function isComplexUsage, $usagePtr, $afterUsage and the
token codes named above).
---
Nitpick comments:
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php`:
- Around line 255-263: Remove the dead state variable $lastNameToken: it is
assigned inside the loop but never read elsewhere, so delete its declaration and
all assignments to it (the initial "$lastNameToken = $current;" and the
"$lastNameToken = $current;" inside the loop) in InlineSingleUseVariableSniff
(the loop that iterates using $current, $end and $tokens while collecting
$nameParts); keep the rest of the logic that builds $nameParts unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3198b973-39ed-45ec-a10f-e0945a0efa42
📒 Files selected for processing (5)
classes/controllers/FrmFormsController.phpclasses/helpers/FrmFormsHelper.phpclasses/helpers/FrmListHelper.phpclasses/models/FrmSolution.phpphpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineSingleUseVariableSniff.php
✅ Files skipped from review due to trivial changes (3)
- classes/models/FrmSolution.php
- classes/helpers/FrmFormsHelper.php
- classes/controllers/FrmFormsController.php
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3045 +/- ##
============================================
- Coverage 26.62% 26.61% -0.02%
- Complexity 8915 8917 +2
============================================
Files 145 145
Lines 30074 30020 -54
============================================
- Hits 8007 7989 -18
+ Misses 22067 22031 -36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Crabcyborg
deleted the
new_sniff_to_remove_unnecessary_var_declartions_for_used_once_vars
branch
stephywells
pushed a commit
that referenced
this pull request
Apr 4, 2026
…ary_var_declartions_for_used_once_vars New sniff to remove unecessary var declarations for used once vars
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit