New sniff to catch more code that can be replaced with array key firs…#2820
Conversation
📝 WalkthroughWalkthroughThis PR introduces a PHP CodeSniffer sniff to detect and automatically fix inefficient Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php`:
- Around line 184-195: The autofix is marked for the count()-1 last-index case
but there's no handler branch to apply the fix; add an elseif branch for the
'last_index' usage type (analogous to the existing 'first_index' handler) inside
the same method that checks $usageCount and $lastIndexUsage so that when $fix is
true you call $this->applyFix($phpcsFile, $varToken, $semicolon,
$lastIndexUsage, 'array_key_last', $arrayArg); ensure the new branch mirrors the
logic and context used by the 'first_index' handler in
PreferArrayKeyFirstLastSniff.php so phpcbf can perform the fix.
- Around line 228-240: The detection currently labels any array access with
count(...) as 'last_index' without verifying the "- 1" suffix; update the logic
in PreferArrayKeyFirstLastSniff (around the block using $tokens, $nextToken,
$indexToken, $usage and $phpcsFile) to only set $usage['type']='last_index' (and
set $usage['bracket']/$usage['closeBracket']) when the token sequence after the
count(...) call exactly matches the "- 1" pattern (i.e., after the count(...)
closing parenthesis there is a T_WHITESPACE* then a T_MINUS token followed by
optional whitespace and a T_LNUMBER with content '1'); otherwise do not mark as
'last_index'. Ensure you use $tokens[$nextToken]['bracket_closer'] (or the
identified closing parenthesis token) to locate the token after the count(...)
call when performing this validation.
- Around line 314-365: The findPrevious(T_EQUAL) calls are unbounded and can
match an '=' from a prior statement; before each findPrevious(T_EQUAL) (the one
using $usage['funcPtr'] and the one using $usagePtr) locate the previous
statement boundary with $phpcsFile->findPrevious(T_SEMICOLON, <ptr> - 1) and
pass that position (plus one as the lower bound) as the third argument to
findPrevious so the search is constrained to the current statement; update the
two occurrences around $usage['funcPtr'] and $usagePtr accordingly and keep the
existing false checks for $assignPtr/$resultVarPtr.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
classes/helpers/FrmCSVExportHelper.phpphpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpphpcs-sniffs/Formidable/ruleset.xml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
⚙️ CodeRabbit configuration file
**/*.php: - Do not make suggestions when we use x.x for a version placeholder. This is used like "@SInCE x.x" in a PHP doc comment, but also like 'x.x' when functions like _deprecated_function() are used.
- Treat these placeholders as acceptable in this codebase.
Files:
classes/helpers/FrmCSVExportHelper.phpphpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php
🧬 Code graph analysis (1)
classes/helpers/FrmCSVExportHelper.php (2)
classes/models/fields/FrmFieldType.php (1)
get_field(194-196)classes/models/FrmFieldValue.php (1)
get_field(142-144)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
🔇 Additional comments (2)
classes/helpers/FrmCSVExportHelper.php (1)
606-608: Nice simplification witharray_key_last().
Cleaner and aligns with the new sniff’s intent.phpcs-sniffs/Formidable/ruleset.xml (1)
41-41: Ruleset update looks good.
The new sniff is properly registered in the Code Analysis block.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Check if the only usage is $keys[ count( $keys ) - 1 ] (last key). | ||
| if ( $usageCount === 2 && null !== $lastIndexUsage ) { | ||
| $fix = $phpcsFile->addFixableError( | ||
| 'Use array_key_last( %s ) instead of array_keys with count - 1 index.', | ||
| $stackPtr, | ||
| 'UseArrayKeyLastCount', | ||
| array( $arrayArg ) | ||
| ); | ||
|
|
||
| if ( true === $fix ) { | ||
| $this->applyFix( $phpcsFile, $varToken, $semicolon, $lastIndexUsage, 'array_key_last', $arrayArg ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 151
🏁 Script executed:
sed -n '180,200p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 598
🏁 Script executed:
# Find the applyFix method to verify it lacks last_index handling
rg -n "applyFix\|last_index|first_index" phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php -A 5Repository: Strategy11/formidable-forms
Length of output: 1006
🏁 Script executed:
# Get a broader view of the applyFix method
rg -n "function applyFix" phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 182
🏁 Script executed:
sed -n '305,372p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 2424
🏁 Script executed:
# Also check what's at lines 305-368 mentioned in the review
sed -n '305,368p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 2392
Missing autofix handler for count()-1 pattern.
Line 185 marks the last_index case as fixable and calls applyFix(), but the method lacks an elseif branch to handle 'last_index' type. This prevents phpcbf from applying the fix when users run it. Add a branch mirroring the first_index handler (lines 342-359) to complete the autofix for this pattern.
Proposed fix
} elseif ( $usage['type'] === 'first_index' ) {
// Pattern: $result = $keys[0];
$assignPtr = $phpcsFile->findPrevious( T_EQUAL, $usagePtr - 1, null );
if ( false !== $assignPtr ) {
$resultVarPtr = $phpcsFile->findPrevious( T_VARIABLE, $assignPtr - 1, null );
if ( false !== $resultVarPtr ) {
$resultVar = $tokens[ $resultVarPtr ]['content'];
$usageSemicolon = $phpcsFile->findNext( T_SEMICOLON, $usage['closeBracket'] + 1 );
for ( $i = $varToken; $i <= $semicolon; $i++ ) {
$fixer->replaceToken( $i, '' );
}
for ( $i = $resultVarPtr; $i <= $usageSemicolon; $i++ ) {
$fixer->replaceToken( $i, '' );
}
$fixer->addContent( $resultVarPtr, $resultVar . ' = ' . $replacementFunc . '( ' . $arrayArg . ' );' );
}
}
+ } elseif ( $usage['type'] === 'last_index' ) {
+ // Pattern: $result = $keys[ count( $keys ) - 1 ];
+ $assignPtr = $phpcsFile->findPrevious( T_EQUAL, $usagePtr - 1, null );
+
+ if ( false !== $assignPtr ) {
+ $resultVarPtr = $phpcsFile->findPrevious( T_VARIABLE, $assignPtr - 1, null );
+
+ if ( false !== $resultVarPtr ) {
+ $resultVar = $tokens[ $resultVarPtr ]['content'];
+ $usageSemicolon = $phpcsFile->findNext( T_SEMICOLON, $usage['closeBracket'] + 1 );
+
+ for ( $i = $varToken; $i <= $semicolon; $i++ ) {
+ $fixer->replaceToken( $i, '' );
+ }
+ for ( $i = $resultVarPtr; $i <= $usageSemicolon; $i++ ) {
+ $fixer->replaceToken( $i, '' );
+ }
+
+ $fixer->addContent( $resultVarPtr, $resultVar . ' = ' . $replacementFunc . '( ' . $arrayArg . ' );' );
+ }
+ }
}🤖 Prompt for AI Agents
In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php`
around lines 184 - 195, The autofix is marked for the count()-1 last-index case
but there's no handler branch to apply the fix; add an elseif branch for the
'last_index' usage type (analogous to the existing 'first_index' handler) inside
the same method that checks $usageCount and $lastIndexUsage so that when $fix is
true you call $this->applyFix($phpcsFile, $varToken, $semicolon,
$lastIndexUsage, 'array_key_last', $arrayArg); ensure the new branch mirrors the
logic and context used by the 'first_index' handler in
PreferArrayKeyFirstLastSniff.php so phpcbf can perform the fix.
| if ( $tokens[ $nextToken ]['code'] === T_OPEN_SQUARE_BRACKET ) { | ||
| $indexToken = $phpcsFile->findNext( T_WHITESPACE, $nextToken + 1, null, true ); | ||
|
|
||
| if ( false !== $indexToken && $tokens[ $indexToken ]['code'] === T_LNUMBER && $tokens[ $indexToken ]['content'] === '0' ) { | ||
| $usage['type'] = 'first_index'; | ||
| $usage['bracket'] = $nextToken; | ||
| $usage['closeBracket'] = $tokens[ $nextToken ]['bracket_closer'] ?? null; | ||
| } elseif ( false !== $indexToken && $tokens[ $indexToken ]['code'] === T_STRING && $tokens[ $indexToken ]['content'] === 'count' ) { | ||
| // Check for count( $var ) - 1 pattern. | ||
| $usage['type'] = 'last_index'; | ||
| $usage['bracket'] = $nextToken; | ||
| $usage['closeBracket'] = $tokens[ $nextToken ]['bracket_closer'] ?? null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 151
🏁 Script executed:
sed -n '210,260p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 2185
🏁 Script executed:
sed -n '1,50p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1250
🏁 Script executed:
grep -n "last_index" phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 156
🏁 Script executed:
sed -n '240,310p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 2195
🏁 Script executed:
sed -n '110,140p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1000
🏁 Script executed:
sed -n '90,130p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1363
🏁 Script executed:
sed -n '130,170p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1250
🏁 Script executed:
sed -n '170,210p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1298
🏁 Script executed:
sed -n '310,350p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1608
🏁 Script executed:
sed -n '295,330p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1497
🏁 Script executed:
sed -n '330,370p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 1430
🏁 Script executed:
sed -n '360,372p' phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 395
Validate the - 1 suffix before labeling last_index.
The code currently marks any $keys[count(...)] as last_index, even if the expression is count(...) + 1, missing entirely the - 1 suffix, or any other variation. Since the error message explicitly references "count - 1 index" (line 187), the detection must validate this exact pattern before applying the fix.
🛠️ Suggested validation
@@
- } elseif ( false !== $indexToken && $tokens[ $indexToken ]['code'] === T_STRING && $tokens[ $indexToken ]['content'] === 'count' ) {
- // Check for count( $var ) - 1 pattern.
- $usage['type'] = 'last_index';
- $usage['bracket'] = $nextToken;
- $usage['closeBracket'] = $tokens[ $nextToken ]['bracket_closer'] ?? null;
+ } elseif ( false !== $indexToken && $tokens[ $indexToken ]['code'] === T_STRING && $tokens[ $indexToken ]['content'] === 'count' ) {
+ $closeBracket = $tokens[ $nextToken ]['bracket_closer'] ?? null;
+ if ( null !== $closeBracket ) {
+ $indexExpr = trim( $phpcsFile->getTokensAsString( $indexToken, $closeBracket - $indexToken ) );
+ $pattern = '/^count\\s*\\(\\s*' . preg_quote( $variableName, '/' ) . '\\s*\\)\\s*-\\s*1$/';
+ if ( preg_match( $pattern, $indexExpr ) ) {
+ $usage['type'] = 'last_index';
+ $usage['bracket'] = $nextToken;
+ $usage['closeBracket'] = $closeBracket;
+ }
+ }
}🤖 Prompt for AI Agents
In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php`
around lines 228 - 240, The detection currently labels any array access with
count(...) as 'last_index' without verifying the "- 1" suffix; update the logic
in PreferArrayKeyFirstLastSniff (around the block using $tokens, $nextToken,
$indexToken, $usage and $phpcsFile) to only set $usage['type']='last_index' (and
set $usage['bracket']/$usage['closeBracket']) when the token sequence after the
count(...) call exactly matches the "- 1" pattern (i.e., after the count(...)
closing parenthesis there is a T_WHITESPACE* then a T_MINUS token followed by
optional whitespace and a T_LNUMBER with content '1'); otherwise do not mark as
'last_index'. Ensure you use $tokens[$nextToken]['bracket_closer'] (or the
identified closing parenthesis token) to locate the token after the count(...)
call when performing this validation.
| // Find what variable the result is assigned to. | ||
| if ( $usage['type'] === 'end_call' || $usage['type'] === 'reset_call' ) { | ||
| // Pattern: $result = end( $keys ); | ||
| // Find the assignment before the function call. | ||
| $assignPtr = $phpcsFile->findPrevious( T_EQUAL, $usage['funcPtr'] - 1, null ); | ||
|
|
||
| if ( false !== $assignPtr ) { | ||
| $resultVarPtr = $phpcsFile->findPrevious( T_VARIABLE, $assignPtr - 1, null ); | ||
|
|
||
| if ( false !== $resultVarPtr ) { | ||
| $resultVar = $tokens[ $resultVarPtr ]['content']; | ||
|
|
||
| // Find the semicolon after the function call. | ||
| $usageSemicolon = $phpcsFile->findNext( T_SEMICOLON, $usage['closeParen'] + 1 ); | ||
|
|
||
| // Remove the original array_keys line. | ||
| for ( $i = $varToken; $i <= $semicolon; $i++ ) { | ||
| $fixer->replaceToken( $i, '' ); | ||
| } | ||
|
|
||
| // Replace the end/reset line with the new function. | ||
| for ( $i = $resultVarPtr; $i <= $usageSemicolon; $i++ ) { | ||
| $fixer->replaceToken( $i, '' ); | ||
| } | ||
|
|
||
| $fixer->addContent( $resultVarPtr, $resultVar . ' = ' . $replacementFunc . '( ' . $arrayArg . ' );' ); | ||
| } | ||
| } | ||
| } elseif ( $usage['type'] === 'first_index' ) { | ||
| // Pattern: $result = $keys[0]; | ||
| $assignPtr = $phpcsFile->findPrevious( T_EQUAL, $usagePtr - 1, null ); | ||
|
|
||
| if ( false !== $assignPtr ) { | ||
| $resultVarPtr = $phpcsFile->findPrevious( T_VARIABLE, $assignPtr - 1, null ); | ||
|
|
||
| if ( false !== $resultVarPtr ) { | ||
| $resultVar = $tokens[ $resultVarPtr ]['content']; | ||
|
|
||
| // Find the semicolon after the index access. | ||
| $usageSemicolon = $phpcsFile->findNext( T_SEMICOLON, $usage['closeBracket'] + 1 ); | ||
|
|
||
| // Remove the original array_keys line. | ||
| for ( $i = $varToken; $i <= $semicolon; $i++ ) { | ||
| $fixer->replaceToken( $i, '' ); | ||
| } | ||
|
|
||
| // Replace the index access line with the new function. | ||
| for ( $i = $resultVarPtr; $i <= $usageSemicolon; $i++ ) { | ||
| $fixer->replaceToken( $i, '' ); | ||
| } | ||
|
|
||
| $fixer->addContent( $resultVarPtr, $resultVar . ' = ' . $replacementFunc . '( ' . $arrayArg . ' );' ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 151
🏁 Script executed:
cat -n phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php | sed -n '280,380p'Repository: Strategy11/formidable-forms
Length of output: 3832
🏁 Script executed:
cat -n phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php | sed -n '1,100p'Repository: Strategy11/formidable-forms
Length of output: 3427
🏁 Script executed:
cat -n phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php | sed -n '200,280p'Repository: Strategy11/formidable-forms
Length of output: 3512
🏁 Script executed:
find . -name "*PreferArrayKeyFirstLast*" -type fRepository: Strategy11/formidable-forms
Length of output: 149
🏁 Script executed:
cat -n phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php | sed -n '100,200p'Repository: Strategy11/formidable-forms
Length of output: 3736
🏁 Script executed:
find . -path "*/tests/*" -name "*PreferArrayKeyFirstLast*" -type fRepository: Strategy11/formidable-forms
Length of output: 53
🏁 Script executed:
find . -path "*/tests/*" -name "*" -type f | grep -i "prefer\|array.*key"Repository: Strategy11/formidable-forms
Length of output: 53
🏁 Script executed:
find . -name "*Test.php" -o -name "*test.php" | head -20Repository: Strategy11/formidable-forms
Length of output: 148
🏁 Script executed:
cat -n phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php | sed -n '45,150p'Repository: Strategy11/formidable-forms
Length of output: 4038
🏁 Script executed:
cat -n phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php | sed -n '100,170p'Repository: Strategy11/formidable-forms
Length of output: 2722
🏁 Script executed:
grep -n "applyFix" phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.phpRepository: Strategy11/formidable-forms
Length of output: 607
🏁 Script executed:
cat -n phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php | sed -n '125,175p'Repository: Strategy11/formidable-forms
Length of output: 1859
🏁 Script executed:
cat -n phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php | sed -n '150,180p'Repository: Strategy11/formidable-forms
Length of output: 1173
🏁 Script executed:
cat -n phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php | sed -n '310,372p'Repository: Strategy11/formidable-forms
Length of output: 2660
🏁 Script executed:
python3 << 'EOF'
# Simulate the problematic scenario
code_before_usage = """
$x = 5; // <- This = at position ~10
$keys = array_keys($array);
// ... more code ...
return end($keys); // <- usage detected here at position ~50
"""
# When applyFix is called with usagePtr ~50:
# findPrevious(T_EQUAL, 49, null) searches backwards from 49
# It will find: T_EQUAL at position ~10 (the $x = 5 line)
# NOT the one we want!
# The fix suggests:
# First find previous T_SEMICOLON from usagePtr
# In this case: T_SEMICOLON after $keys = array_keys($array); at ~30
# Then search for T_EQUAL between ~50 and ~30
# Since there's no T_EQUAL in that range, findPrevious returns false
# And the fix doesn't apply (correctly!)
print("Scenario 1: return end($keys);")
print(" - findPrevious(T_EQUAL, 49, null) -> finds = at position 10 (WRONG!)")
print(" - Fixed: findPrevious(T_EQUAL, 49, 30) -> returns false (CORRECT!)")
print()
print("Scenario 2: $result = end($keys);")
print(" - Both approaches find the = in same statement (CORRECT!)")
EOFRepository: Strategy11/formidable-forms
Length of output: 157
Bound the unbounded findPrevious(T_EQUAL) search to the current statement to prevent removal of unrelated code.
The sniff detects usage patterns (end($keys), $keys[0], reset($keys)) within the current scope but does not verify these are part of assignments. When code like return end($keys); or foo(end($keys)); is encountered, applyFix is still called. An unbounded findPrevious(T_EQUAL) search can then match an = from a previous statement and remove unrelated code when clearing lines.
Constrain both findPrevious(T_EQUAL) calls to the current statement by finding the previous T_SEMICOLON as a lower boundary:
Suggested fix
@@
- $assignPtr = $phpcsFile->findPrevious( T_EQUAL, $usage['funcPtr'] - 1, null );
+ $statementStart = $phpcsFile->findPrevious( T_SEMICOLON, $usage['funcPtr'] - 1 );
+ $assignPtr = $phpcsFile->findPrevious(
+ T_EQUAL,
+ $usage['funcPtr'] - 1,
+ false === $statementStart ? null : $statementStart
+ );
@@
- $assignPtr = $phpcsFile->findPrevious( T_EQUAL, $usagePtr - 1, null );
+ $statementStart = $phpcsFile->findPrevious( T_SEMICOLON, $usagePtr - 1 );
+ $assignPtr = $phpcsFile->findPrevious(
+ T_EQUAL,
+ $usagePtr - 1,
+ false === $statementStart ? null : $statementStart
+ );📝 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.
| // Find what variable the result is assigned to. | |
| if ( $usage['type'] === 'end_call' || $usage['type'] === 'reset_call' ) { | |
| // Pattern: $result = end( $keys ); | |
| // Find the assignment before the function call. | |
| $assignPtr = $phpcsFile->findPrevious( T_EQUAL, $usage['funcPtr'] - 1, null ); | |
| if ( false !== $assignPtr ) { | |
| $resultVarPtr = $phpcsFile->findPrevious( T_VARIABLE, $assignPtr - 1, null ); | |
| if ( false !== $resultVarPtr ) { | |
| $resultVar = $tokens[ $resultVarPtr ]['content']; | |
| // Find the semicolon after the function call. | |
| $usageSemicolon = $phpcsFile->findNext( T_SEMICOLON, $usage['closeParen'] + 1 ); | |
| // Remove the original array_keys line. | |
| for ( $i = $varToken; $i <= $semicolon; $i++ ) { | |
| $fixer->replaceToken( $i, '' ); | |
| } | |
| // Replace the end/reset line with the new function. | |
| for ( $i = $resultVarPtr; $i <= $usageSemicolon; $i++ ) { | |
| $fixer->replaceToken( $i, '' ); | |
| } | |
| $fixer->addContent( $resultVarPtr, $resultVar . ' = ' . $replacementFunc . '( ' . $arrayArg . ' );' ); | |
| } | |
| } | |
| } elseif ( $usage['type'] === 'first_index' ) { | |
| // Pattern: $result = $keys[0]; | |
| $assignPtr = $phpcsFile->findPrevious( T_EQUAL, $usagePtr - 1, null ); | |
| if ( false !== $assignPtr ) { | |
| $resultVarPtr = $phpcsFile->findPrevious( T_VARIABLE, $assignPtr - 1, null ); | |
| if ( false !== $resultVarPtr ) { | |
| $resultVar = $tokens[ $resultVarPtr ]['content']; | |
| // Find the semicolon after the index access. | |
| $usageSemicolon = $phpcsFile->findNext( T_SEMICOLON, $usage['closeBracket'] + 1 ); | |
| // Remove the original array_keys line. | |
| for ( $i = $varToken; $i <= $semicolon; $i++ ) { | |
| $fixer->replaceToken( $i, '' ); | |
| } | |
| // Replace the index access line with the new function. | |
| for ( $i = $resultVarPtr; $i <= $usageSemicolon; $i++ ) { | |
| $fixer->replaceToken( $i, '' ); | |
| } | |
| $fixer->addContent( $resultVarPtr, $resultVar . ' = ' . $replacementFunc . '( ' . $arrayArg . ' );' ); | |
| // Find what variable the result is assigned to. | |
| if ( $usage['type'] === 'end_call' || $usage['type'] === 'reset_call' ) { | |
| // Pattern: $result = end( $keys ); | |
| // Find the assignment before the function call. | |
| $statementStart = $phpcsFile->findPrevious( T_SEMICOLON, $usage['funcPtr'] - 1 ); | |
| $assignPtr = $phpcsFile->findPrevious( | |
| T_EQUAL, | |
| $usage['funcPtr'] - 1, | |
| false === $statementStart ? null : $statementStart | |
| ); | |
| if ( false !== $assignPtr ) { | |
| $resultVarPtr = $phpcsFile->findPrevious( T_VARIABLE, $assignPtr - 1, null ); | |
| if ( false !== $resultVarPtr ) { | |
| $resultVar = $tokens[ $resultVarPtr ]['content']; | |
| // Find the semicolon after the function call. | |
| $usageSemicolon = $phpcsFile->findNext( T_SEMICOLON, $usage['closeParen'] + 1 ); | |
| // Remove the original array_keys line. | |
| for ( $i = $varToken; $i <= $semicolon; $i++ ) { | |
| $fixer->replaceToken( $i, '' ); | |
| } | |
| // Replace the end/reset line with the new function. | |
| for ( $i = $resultVarPtr; $i <= $usageSemicolon; $i++ ) { | |
| $fixer->replaceToken( $i, '' ); | |
| } | |
| $fixer->addContent( $resultVarPtr, $resultVar . ' = ' . $replacementFunc . '( ' . $arrayArg . ' );' ); | |
| } | |
| } | |
| } elseif ( $usage['type'] === 'first_index' ) { | |
| // Pattern: $result = $keys[0]; | |
| $statementStart = $phpcsFile->findPrevious( T_SEMICOLON, $usagePtr - 1 ); | |
| $assignPtr = $phpcsFile->findPrevious( | |
| T_EQUAL, | |
| $usagePtr - 1, | |
| false === $statementStart ? null : $statementStart | |
| ); | |
| if ( false !== $assignPtr ) { | |
| $resultVarPtr = $phpcsFile->findPrevious( T_VARIABLE, $assignPtr - 1, null ); | |
| if ( false !== $resultVarPtr ) { | |
| $resultVar = $tokens[ $resultVarPtr ]['content']; | |
| // Find the semicolon after the index access. | |
| $usageSemicolon = $phpcsFile->findNext( T_SEMICOLON, $usage['closeBracket'] + 1 ); | |
| // Remove the original array_keys line. | |
| for ( $i = $varToken; $i <= $semicolon; $i++ ) { | |
| $fixer->replaceToken( $i, '' ); | |
| } | |
| // Replace the index access line with the new function. | |
| for ( $i = $resultVarPtr; $i <= $usageSemicolon; $i++ ) { | |
| $fixer->replaceToken( $i, '' ); | |
| } | |
| $fixer->addContent( $resultVarPtr, $resultVar . ' = ' . $replacementFunc . '( ' . $arrayArg . ' );' ); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferArrayKeyFirstLastSniff.php`
around lines 314 - 365, The findPrevious(T_EQUAL) calls are unbounded and can
match an '=' from a prior statement; before each findPrevious(T_EQUAL) (the one
using $usage['funcPtr'] and the one using $usagePtr) locate the previous
statement boundary with $phpcsFile->findPrevious(T_SEMICOLON, <ptr> - 1) and
pass that position (plus one as the lower bound) as the third argument to
findPrevious so the search is constrained to the current statement; update the
two occurrences around $usage['funcPtr'] and $usagePtr accordingly and keep the
existing false checks for $assignPtr/$resultVarPtr.
1 participant
…t/last functions
Rector caught most uses, but not this one.
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.