Use strict in more in_array checks#2734
Conversation
WalkthroughMultiple controller, helper, model, and view files update Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
classes/controllers/FrmFormActionsController.php (1)
138-162: Strict membership check for action grouping is appropriate
$groupedcontains action slugs and$action->id_baseis also a slug, so switching toin_array( $action->id_base, $grouped, true )keeps the grouping logic the same for normal usage while avoiding any cross‑type matches. The only behavior change would be for custom filters that mix non‑string types intoactions, which seems unlikely and would be better aligned by normalizing those to strings.classes/controllers/FrmAddonsController.php (1)
633-681: Stricter license de‑duplication is fine; consider normalizing license valuesUsing
in_array( $new_license, $checked_licenses, true )keeps the “one API call per license” behavior intact as long as licenses are consistently stored as strings, and removes any chance of type‑juggling collisions.If you ever expect license values to come in with differing types or whitespace (e.g.,
123vs'123'vs' 123 '), consider normalizing before the check:
- Cast to string and trim (and/or uppercase if needed) prior to adding/checking in
$checked_licenses.This would keep the stricter comparison while ensuring logically identical licenses are still de‑duplicated.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
classes/controllers/FrmAddonsController.phpclasses/controllers/FrmEntriesController.phpclasses/controllers/FrmFormActionsController.phpclasses/helpers/FrmAppHelper.phpclasses/helpers/FrmEntriesListHelper.phpclasses/helpers/FrmXMLHelper.phpclasses/models/FrmDb.phpclasses/models/FrmFormMigrator.phpclasses/views/frm-form-actions/settings.php
🧰 Additional context used
🧬 Code graph analysis (2)
classes/views/frm-form-actions/settings.php (1)
classes/controllers/FrmFormActionsController.php (1)
action_control(444-450)
classes/controllers/FrmEntriesController.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-5050)is_admin_page(495-509)
⏰ 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: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: Cypress
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
🔇 Additional comments (11)
classes/helpers/FrmAppHelper.php (1)
3348-3357: Stricter unit lookup inget_unitis correct and non‑breaking
$unitand the values in$stringsare all strings, so adding the strict flag toin_arraypreserves existing behavior while preventing any accidental cross‑type matches in the future. No issues here.classes/models/FrmDb.php (1)
547-566: Strictin_arrayinesc_ordersafely tightens the whitelist check
$orderis always a string token and$safe_ordercontains only'count(*)', so usingin_array(..., true)keeps behavior the same for valid input and makes the whitelist more robust against any future cross‑type values. Looks good.classes/helpers/FrmEntriesListHelper.php (1)
299-307: Primary column detection remains unchanged with strictin_array
$column_keyand the entries in$hiddenare all strings from WordPress core, so adding the strict flag keeps the primary‑column selection identical and just avoids any accidental matches if non‑string values ever appeared in$hidden.classes/controllers/FrmEntriesController.php (1)
193-193: LGTM: Strict comparison improves type safety.The strict comparison for the
in_arraycheck is appropriate here. The$actionvariable fromsanitize_titlereturns a string, and you're comparing it against an array of strings ('','list','destroy'), so type coercion is not needed.classes/views/frm-form-actions/settings.php (2)
43-43: LGTM: Strict type checking for action identification.Both
in_arraychecks on this line now use strict comparison, which is correct for comparing action identifiers. This ensures exact string matches without type coercion.
52-52: LGTM: Consistent strict comparison.The strict comparison aligns with the check on line 43 and ensures type-safe membership testing for displayed actions.
classes/helpers/FrmXMLHelper.php (2)
666-666: LGTM: Type-safe field type checking.The strict comparison is appropriate for checking field types. Since
$f['type']is a string field type identifier being compared against an array of string literals, this eliminates any risk of unexpected type coercion during XML import.
2307-2307: LGTM: Consistent strict comparison in notification setup.The strict comparison properly checks if
$add_fieldis exactly'plain_text'or'inc_user_info', avoiding any type coercion issues. This is part of the broader pattern in this PR to enforce type-safe comparisons.classes/models/FrmFormMigrator.php (3)
306-306: LGTM: Type-safe section membership check.The strict comparison ensures that field types are checked with exact string matching against the array returned by
fields_with_end(). This prevents any unexpected behavior from type coercion when determining if a field should be placed in a section.
314-314: LGTM: Consistent strict comparison for section tracking.This strict comparison correctly identifies fields that require an end section marker, maintaining consistency with the check on line 306.
378-387: LGTM: Type-safe field type validation.Both
in_arraychecks on lines 378 and 387 now use strict comparison when checking field types against the$with_endarray. This ensures type-safe matching when automatically adding end section fields during form migration.
…checks Use strict in more in_array checks
No description provided.