Add var comments to object properties#2611
Conversation
|
Warning Rate limit exceeded@Crabcyborg has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 9 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (3)
WalkthroughAdded PHPDoc annotations and several new property declarations across multiple models and controllers; removed two unused properties; updated phpstan.neon rules; and added a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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/models/FrmInbox.php (1)
167-198: Fix the pipeline failure: add null-check before foreach.Psalm correctly identifies that
$messagescan befalse(as documented in the PHPDoc on line 19), but theforeachon line 189 doesn't guard against this case.Apply this diff to fix the issue:
public function filter_messages( &$messages, $type = 'unread' ) { + if ( false === $messages || ! is_array( $messages ) ) { + return; + } $user_id = get_current_user_id(); foreach ( $messages as $k => $message ) {Alternatively, if
$messagesshould never actually be false when this method is called, consider changing the type annotation and ensuring$messagesis always initialized as an empty array instead of false.
🧹 Nitpick comments (1)
classes/models/FrmSettings.php (1)
629-640: Consider whetherstore()should be called or document the requirement.The
update_setting()method updates the property value but does not persist the change to the database by calling$this->store(). This means callers must explicitly callstore()after using this method.Verify this is intentional. If so, consider adding a
@returnPHPDoc note that callers must invokestore()separately to persist the changes.Consider adding documentation:
/** * Updates a single setting with specified sanitization. * * @since 6.9 * * @param string $key The setting key to update. * @param mixed $value The new value for the setting. * @param string $sanitize The name of the sanitization function to apply to the new value. - * @return bool True on success, false on failure. + * @return bool True on success, false on failure. Note: Callers must invoke store() to persist changes. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
classes/controllers/FrmFormActionsController.php(2 hunks)classes/helpers/FrmEntriesListHelper.php(1 hunks)classes/helpers/FrmFieldGridHelper.php(2 hunks)classes/helpers/FrmFormsListHelper.php(1 hunks)classes/helpers/FrmListHelper.php(1 hunks)classes/models/FrmAddon.php(1 hunks)classes/models/FrmCreateFile.php(1 hunks)classes/models/FrmDb.php(1 hunks)classes/models/FrmField.php(1 hunks)classes/models/FrmFieldFormHtml.php(1 hunks)classes/models/FrmFieldTypeOptionData.php(1 hunks)classes/models/FrmFormMigrator.php(1 hunks)classes/models/FrmFormTemplateApi.php(1 hunks)classes/models/FrmInbox.php(1 hunks)classes/models/FrmMigrate.php(1 hunks)classes/models/FrmOnSubmitAction.php(1 hunks)classes/models/FrmPersonalData.php(1 hunks)classes/models/FrmReviews.php(1 hunks)classes/models/FrmSettings.php(5 hunks)classes/models/FrmSolution.php(2 hunks)classes/models/FrmSpamCheckDenylist.php(1 hunks)phpstan.neon(1 hunks)square/controllers/FrmSquareLiteEventsController.php(1 hunks)stripe/controllers/FrmStrpLiteEventsController.php(1 hunks)stubs.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
classes/models/FrmAddon.php (3)
classes/models/FrmSolution.php (2)
download_id(765-767)plugin_name(138-140)classes/helpers/FrmAppHelper.php (1)
plugin_folder(51-53)stripe/helpers/FrmTransLiteAppHelper.php (1)
plugin_folder(30-32)
🪛 GitHub Actions: Psalm Code Analysis
classes/models/FrmInbox.php
[error] 167-167: Psalm: PossiblyFalseIterator: Cannot iterate over falsable var array<array-key, mixed>|false (see https://psalm.dev/164)
⏰ 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: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
🔇 Additional comments (27)
classes/models/FrmFieldFormHtml.php (1)
11-39: LGTM! PHPDoc annotations are accurate.The type annotations correctly document the property types:
int|stringfor$field_idappropriately handles both numeric IDs and string keys- Array types for
$formand$pass_argsmatch their initialization and usage patternsclasses/models/FrmPersonalData.php (1)
8-16: LGTM! Type annotations are correct.Both
$limitand$pageare correctly typed asint- they're initialized with integer defaults and$pageis explicitly cast viaabsint()when set.classes/models/FrmCreateFile.php (1)
8-46: LGTM! Comprehensive type documentation.All type annotations accurately reflect the property usage:
- String types for path/name properties
arrayfor$uploads(fromwp_upload_dir())intfor chmod values (octal literals are integers in PHP)boolfor the permission flagclasses/models/FrmFieldTypeOptionData.php (1)
15-18: LGTM!The
@var arrayannotation correctly documents the static cache property used to store field type options.classes/models/FrmSpamCheckDenylist.php (1)
19-27: LGTM!Type annotations correctly document:
$posted_fields: Populated fromFrmField::get_all_for_form()returning an array$denylist: Populated fromget_denylist_array()returning an array of denylist configurationsclasses/models/FrmFormTemplateApi.php (1)
8-21: LGTM! Clear type documentation for static properties.The PHPDoc annotations accurately document the types of these static properties, improving code clarity and static analysis capabilities.
classes/models/FrmField.php (1)
8-16: LGTM! Type annotations improve static analysis.The PHPDoc blocks correctly document these public static properties, making their types explicit for developers and static analysis tools.
classes/helpers/FrmFormsListHelper.php (1)
8-11: LGTM! Type documentation aligns with usage.The PHPDoc annotation correctly documents
$statusas a string, which is consistent with its initialization on line 16 usingself::get_param().classes/controllers/FrmFormActionsController.php (2)
8-11: LGTM! Type annotation for post type constant.The PHPDoc correctly documents this static property as a string, matching its initialization value.
787-790: LGTM! Type documentation for action registry.The PHPDoc correctly identifies
$actionsas an array, consistent with its initialization on line 790 and usage throughout the factory pattern implementation.phpstan.neon (1)
161-163: LGTM! Improved static analysis strictness.Removing the broad global ignore rule
"#has no type specified.#"and replacing it with a targeted ignore for a specific file improves the effectiveness of static analysis. This change aligns well with the PR's goal of adding explicit type documentation throughout the codebase.classes/helpers/FrmFieldGridHelper.php (2)
8-11: LGTM! Union type correctly documents dual usage.The
bool|stringtype annotation for$parent_liaccurately reflects its dual usage: initialized asfalse(line 67) and later set totrue(line 172).
58-61: LGTM! Property declaration with default value.The PHPDoc and explicit initialization of
$section_is_opentofalseimproves code clarity. This property is used throughout the class (e.g., lines 89, 149, 217) and the default value is appropriate.classes/models/FrmInbox.php (1)
13-21: LGTM! Type annotations are accurate.The PHPDoc blocks correctly document the types of these private properties. The
array|falsetype for$messagesaccurately reflects that it can be false before initialization (line 31).classes/helpers/FrmListHelper.php (1)
82-90: LGTM! Array type annotations added.The PHPDoc blocks correctly identify these protected properties as arrays, matching their initializations on lines 85 and 90-105.
classes/models/FrmReviews.php (1)
8-21: LGTM! Type annotations are accurate.The PHPDoc annotations correctly document the property types and match their initialized values.
stripe/controllers/FrmStrpLiteEventsController.php (1)
13-31: LGTM! Property type annotations are accurate.The PHPDoc blocks correctly document the nullable object and string types for these private properties based on their usage throughout the class.
square/controllers/FrmSquareLiteEventsController.php (1)
13-16: LGTM! Type annotation is correct.The
@var object|nullannotation accurately reflects the property usage, as evidenced by theis_object()check at line 77.classes/models/FrmOnSubmitAction.php (1)
15-18: LGTM! Type annotation is accurate.The
@var stringannotation correctly documents the static property type.classes/helpers/FrmEntriesListHelper.php (1)
8-11: LGTM! Type annotation is accurate.The
@var string|nullannotation correctly reflects the property usage, as it's assigned frompreg_replace()at line 364 which can return null.classes/models/FrmSolution.php (1)
16-37: LGTM! Type annotations are accurate.The PHPDoc blocks correctly document the string types for these protected properties, matching their initialized values.
stubs.php (2)
168-171: LGTM! Type annotation is appropriate.The
@var stringannotation for the version property is reasonable.
335-338: LGTM! Type annotation is accurate.The
@var stringannotation correctly matches the initialized value.classes/models/FrmAddon.php (1)
9-77: LGTM! Property type annotations are accurate.These PHPDoc blocks correctly document the property types based on their initialization values and usage throughout the class.
classes/models/FrmMigrate.php (1)
8-26: LGTM!The PHPDoc type annotations are accurate and align with the property initialization in the constructor.
classes/models/FrmFormMigrator.php (1)
8-51: LGTM!The PHPDoc type annotations accurately document the property types and align with their initialization and usage throughout the class.
classes/models/FrmSettings.php (1)
9-243: LGTM!The PHPDoc type annotations accurately document the property types and support static analysis.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
psalm.xmlis excluded by!**/*.xml
📒 Files selected for processing (4)
classes/helpers/FrmTipsHelper.php(1 hunks)classes/models/FrmAddon.php(1 hunks)classes/models/FrmDb.php(1 hunks)phpstan.neon(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- classes/models/FrmAddon.php
- classes/models/FrmDb.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). (3)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
🔇 Additional comments (1)
phpstan.neon (1)
158-176: Good approach: Moving to file-specific ignore rules.The transition from broad ignore rules to file-specific suppressions is a positive change. This allows PHPStan to catch real issues in other parts of the codebase while acknowledging known edge cases in specific files.
…roperties Add var comments to object properties
1 participant
No description provided.