New sniff to encourage using the best fit wp esc function#2816
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new PHP_CodeSniffer sniff that detects when developers combine echo with HTML escaping and translation functions, recommending a more concise alternative. An existing file is updated to follow this pattern, and the new sniff is registered in the codebase's ruleset. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
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: 1
🤖 Fix all issues with AI agents
In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferEscHtmlESniff.php`:
- Around line 120-134: The sniff currently assumes nothing meaningful exists
between $translateCloseParen and $escapeCloseParen and can incorrectly match
expressions with concatenation (e.g. ". ' extra'"); update the validation to
iterate tokens between $translateCloseParen + 1 and $escapeCloseParen - 1 and
bail out (return) if any token other than T_WHITESPACE (or T_COMMENT if you
prefer to allow comments) is found. Locate this check near the existing
$translateCloseParen and $escapeCloseParen logic in PreferEscHtmlESniff.php and
perform the token-scan before computing $semicolon so the fixer will not
auto-fix when extra non-whitespace content is present.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
classes/views/xml/import_form.phpphpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferEscHtmlESniff.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:
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferEscHtmlESniff.phpclasses/views/xml/import_form.php
🧬 Code graph analysis (1)
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferEscHtmlESniff.php (1)
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipNegativeTernarySniff.php (1)
getTokensAsString(297-306)
⏰ 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). (6)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
🔇 Additional comments (4)
classes/views/xml/import_form.php (1)
72-72: LGTM!The change correctly converts
echo esc_html( __( ... ) )to the combinedesc_html_e( ... )function, aligning with the new sniff's enforcement. This is consistent with the existing patterns elsewhere in this file.phpcs-sniffs/Formidable/ruleset.xml (1)
34-34: LGTM!The new sniff rule reference is correctly added in the Code Analysis section and follows the existing naming convention.
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferEscHtmlESniff.php (2)
29-47: LGTM!The replacements mapping is well-structured, covering all valid combinations of escape and translation functions. The register function correctly listens for
T_ECHOtokens.
136-158: LGTM!The argument extraction using
getTokensAsString()with correct start position and length is properly implemented. The fix logic correctly removes the original tokens and inserts the combined function call with trimmed arguments.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| $translateCloseParen = $tokens[ $translateOpenParen ]['parenthesis_closer']; | ||
|
|
||
| // Find the closing parenthesis of the escape function. | ||
| if ( ! isset( $tokens[ $openParen ]['parenthesis_closer'] ) ) { | ||
| return; | ||
| } | ||
|
|
||
| $escapeCloseParen = $tokens[ $openParen ]['parenthesis_closer']; | ||
|
|
||
| // Find the semicolon. | ||
| $semicolon = $phpcsFile->findNext( T_WHITESPACE, $escapeCloseParen + 1, null, true ); | ||
|
|
||
| if ( false === $semicolon || $tokens[ $semicolon ]['code'] !== T_SEMICOLON ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Missing validation may cause incorrect auto-fixes.
The sniff doesn't verify that there's nothing between $translateCloseParen and $escapeCloseParen except whitespace. For code like:
echo esc_html( __( 'text', 'domain' ) . ' extra' );The sniff would incorrectly match and produce esc_html_e( 'text', 'domain' );, silently losing . ' extra'.
🐛 Proposed fix: Add validation after line 127
$escapeCloseParen = $tokens[ $openParen ]['parenthesis_closer'];
+ // Ensure nothing exists between translation close paren and escape close paren except whitespace.
+ $tokenBetween = $phpcsFile->findNext( T_WHITESPACE, $translateCloseParen + 1, $escapeCloseParen, true );
+
+ if ( false !== $tokenBetween ) {
+ return;
+ }
+
// Find the semicolon.
$semicolon = $phpcsFile->findNext( T_WHITESPACE, $escapeCloseParen + 1, null, true );🤖 Prompt for AI Agents
In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferEscHtmlESniff.php` around
lines 120 - 134, The sniff currently assumes nothing meaningful exists between
$translateCloseParen and $escapeCloseParen and can incorrectly match expressions
with concatenation (e.g. ". ' extra'"); update the validation to iterate tokens
between $translateCloseParen + 1 and $escapeCloseParen - 1 and bail out (return)
if any token other than T_WHITESPACE (or T_COMMENT if you prefer to allow
comments) is found. Locate this check near the existing $translateCloseParen and
$escapeCloseParen logic in PreferEscHtmlESniff.php and perform the token-scan
before computing $semicolon so the fixer will not auto-fix when extra
non-whitespace content is present.
1 participant
This sniff encourages changing
echo esc_html( __( ...type of code to useesc_html_einstead for example.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.