Prefer str_contains over strpos where applicable#2727
Conversation
Crabcyborg
changed the title
str_contains to strpos where applicable
Crabcyborg
changed the title
str_contains to strpos where applicablestr_contains to strpos where applicable
WalkthroughThis pull request systematically replaces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/helpers/FrmAppHelper.php (1)
736-743: Critical: str_contains() requires PHP 8.0+ but plugin declares PHP 7.0 minimumThe use of
str_contains()at lines 737 and 1393 creates a fatal compatibility issue. This function is native to PHP 8.0+ only and does not exist in PHP 7.x. The plugin's readme.txt declares "Requires PHP: 7.0", meaning installations on PHP 7.x will fail at runtime.Either:
- Update readme.txt to require PHP 8.0 or higher, or
- Add a polyfill:
if ( ! function_exists( 'str_contains' ) ) { function str_contains( $haystack, $needle ) { return strpos( $haystack, $needle ) !== false; } }
🤖 Fix all issues with AI Agents
In @classes/helpers/FrmXMLHelper.php:
- Line 2270: The code uses PHP 8's str_contains() on $email_field which breaks
PHP 7 compatibility; replace occurrences of str_contains( $email_field, '|' )
with the PHP 7-compatible check strpos( $email_field, '|' ) !== false (also
update the other instance noted around the code location referenced as 2358).
Ensure you change both usages of str_contains to the strpos !== false pattern so
behavior remains identical.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
classes/controllers/FrmAddonsController.phpclasses/controllers/FrmDeactivationFeedbackController.phpclasses/controllers/FrmFieldsController.phpclasses/controllers/FrmFormsController.phpclasses/helpers/FrmAppHelper.phpclasses/helpers/FrmEmailSummaryHelper.phpclasses/helpers/FrmEntriesListHelper.phpclasses/helpers/FrmFieldsHelper.phpclasses/helpers/FrmFormsHelper.phpclasses/helpers/FrmXMLHelper.phpclasses/models/FrmCreateFile.phpclasses/models/FrmEmail.phpclasses/models/FrmEntryMeta.phpclasses/models/FrmEntryValidate.phpclasses/models/FrmFieldFormHtml.phpclasses/models/FrmMigrate.phpclasses/models/fields/FrmFieldNumber.phpformidable.phpstripe/models/FrmStrpLiteAuth.php
⏰ 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: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: Cypress
🔇 Additional comments (25)
classes/controllers/FrmDeactivationFeedbackController.php (1)
61-64: LGTM!The migration to
str_contains()correctly handles plugin path detection, fixing the edge case where the plugin filename could appear at position 0.classes/models/FrmCreateFile.php (1)
265-265: LGTM!The migration to
str_contains()correctly handles colon detection for hostname:port parsing, fixing the edge case where a malformed hostname starting with ':' would be treated as not containing a colon.stripe/models/FrmStrpLiteAuth.php (1)
120-120: LGTM!The migration to
str_contains()correctly handles form ID detection in HTML, fixing the edge case where the hidden input appears at the start of the HTML string.classes/controllers/FrmAddonsController.php (1)
454-458: LGTM!The migration to
str_contains()correctly handles hyphen detection in license keys for normalization, fixing the edge case where a hyphen at position 0 would be falsy. The added blank line improves readability.classes/controllers/FrmFormsController.php (1)
3528-3533: str_contains-based defer check is correct and saferThe updated condition using
str_contains( $tag, 'defer' )preserves the original intent (only inject attributes once) and avoids the classicstrpos() === 0pitfall. No behavioral or compatibility concerns within this method.classes/helpers/FrmEntriesListHelper.php (1)
519-557: Embedded field parsing with str_contains is correctUsing
str_contains( $col_name, '-_-' )before theexplode()keeps the previous behavior, handles delimiters at any position (including index 0), and keeps$embedded_field_idguarded by the existingisset()check. No issues here.classes/models/fields/FrmFieldNumber.php (1)
165-170: Comma detection for numeric fields now uses str_contains correctly
str_contains( $args['value'], ',' )is an appropriate replacement for the old comma check and works as expected given$args['value']is treated as a string throughout validation. No behavioral regressions introduced.classes/models/FrmEmail.php (1)
716-741: Yahoo sender detection via str_contains is correctThe updated Yahoo-domain check using
str_contains( $from_email, '@yahoo.com' )keeps the same intent—rewriting problematic Yahoo From addresses to the WordPress default—while simplifying the conditional. No functional issues introduced.formidable.php (1)
101-101: LGTM: Correctly handles edge case for class names starting with "Field".The change from
strpos()tostr_contains()ensures correct behavior when a class name starts with "Field" (e.g.,FieldCustomType). The previous implementation would return0(falsy) for such cases, potentially skipping the fields directory check.classes/models/FrmMigrate.php (3)
379-379: LGTM: Improved dash detection in version strings.The
str_contains()replacement correctly handles the edge case where a dash might appear at the beginning of$old_db_version.
670-670: LGTM: Improved 'px' detection in size values.The
str_contains()replacement correctly handles the edge case where 'px' might appear at position 0 in the$sizestring.
869-869: LGTM: Improved 'save_draft' detection in submit HTML.The
str_contains()replacement correctly handles the edge case where 'save_draft' might appear at the beginning of the submit HTML string.classes/controllers/FrmFieldsController.php (1)
1005-1005: LGTM: Correctly handles shortcode values starting with '='.The change from
strpos()tostr_contains()ensures correct detection of the equals sign in shortcode values, including the edge case where '=' appears at position 0.classes/helpers/FrmFieldsHelper.php (3)
23-23: LGTM: Improved pipe detection in field types.The
str_contains()replacement correctly handles the edge case where '|' might appear at the beginning of the$typestring.
1336-1336: LGTM: Improved encoded bracket detection.The
str_contains()replacement correctly handles the edge case where '[' (HTML entity for '[') might appear at position 0 in the parameter.
2381-2381: LGTM: Improved class detection in icon strings.The
str_contains()replacement correctly handles the edge case where ' frm_show_upgrade' might appear at the beginning of the icon class string.classes/models/FrmEntryValidate.php (1)
853-853: LGTM: Improved '@' detection in email validation.The
str_contains()replacement correctly handles the edge case where '@' might appear at position 0 in the value string. Whileis_email()would catch invalid cases like '@example.com', usingstr_contains()is semantically clearer and more consistent with the rest of this PR.classes/helpers/FrmXMLHelper.php (2)
2270-2270: LGTM! Edge case fix for delimiter detection.The change from
strpos()tostr_contains()correctly handles the edge case where the delimiter|appears at position 0. The previous implementation would have incorrectly skipped delimiter parsing in that scenario.
2358-2358: LGTM! Consistent edge case fix.This change mirrors the fix at line 2270, ensuring delimiter detection works correctly when
|appears at the start of the string.classes/helpers/FrmEmailSummaryHelper.php (1)
534-542: LGTM! Correct HTML attribute detection.Both changes properly handle edge cases in button HTML processing:
- Line 534: Detects existing inline styles to avoid duplication
- Line 540: Detects button class to conditionally add styling
The
str_contains()implementation ensures correct detection regardless of substring position.classes/models/FrmFieldFormHtml.php (3)
271-271: LGTM! Prevents duplicate ID attributes.The change correctly detects existing ID attributes to avoid duplication. The
str_contains()implementation ensures detection works even if the attribute appears at the beginning of the HTML segment.
410-411: LGTM! Shortcode detection fix.The change ensures the collapse shortcode is removed regardless of its position in the HTML string.
546-548: LGTM! Prevents duplicate CSS class.The change correctly detects the existing
frm_form_fieldclass to prevent duplication. The implementation properly handles cases where the class appears at the start of the HTML.classes/helpers/FrmFormsHelper.php (2)
554-557: LGTM! Correct shortcode detection.The change ensures the
[button_action]shortcode is properly detected and processed regardless of its position in the button HTML. The previous implementation would have incorrectly skipped processing if the shortcode appeared at the beginning.
934-953: LGTM! Comprehensive shortcode detection fixes.All four changes correctly handle shortcode detection:
- Line 934: Enables button label processing when shortcode is present
- Lines 943, 947, 951: Conditionally remove button-related shortcodes
The
str_contains()implementation ensures proper detection when shortcodes appear at the start of the HTML string, fixing potential edge cases where button labels or conditional buttons would malfunction.
Crabcyborg
changed the title
str_contains to strpos where applicablestr_contains over strpos where applicable
Prefer `str_contains` over `strpos` where applicable
1 participant
No description provided.