Add missing block comments and type comments#2868
Conversation
|
Warning Rate limit exceeded
⌛ 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. 📝 WalkthroughWalkthroughThis PR introduces an automated PHP_CodeSniffer sniff for generating PHPDoc docblocks with type annotations, disables the conflicting phpdoc_add_missing_param_annotation rule, and adds comprehensive documentation blocks across helper, model, field, and test classes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/models/FrmEmailMonthly.php (1)
27-38: Fix return annotation to match the inherited signature (Psalm failing).Line 30 documents
@return bool, but the method returnsfalseorparent::get_content_args()(array|false). This mismatches the parent signature and is already breaking Psalm. Update the docblock to reflect the actual return type.✅ Suggested fix
- * `@return` bool + * `@return` array|false
🤖 Fix all issues with AI agents
In `@classes/models/FrmEmailYearly.php`:
- Around line 28-30: The docblock for get_content_args() incorrectly declares
"@return bool"; update it to match the parent return type (array|false) so the
method signature and PHPDoc align with parent::get_content_args(); change the
`@return` annotation to "array|false" on the get_content_args() docblock in class
FrmEmailYearly.
In `@phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingDocblockSniff.php`:
- Around line 214-271: isUsedAsArray currently scans the whole function body and
can mis-detect array usage from nested closures/functions; update isUsedAsArray
(used by getCertainParamTypes) to ignore tokens inside nested scopes by skipping
over any inner scope token (e.g., T_FUNCTION, T_CLOSURE, T_CLASS, T_ANON_CLASS)
encountered between $scopeOpener and $scopeCloser: when you find one, advance
the loop index to that token's ['scope_closer'] so variables inside that nested
scope are not considered, then continue the search for the target $varName
followed by T_OPEN_SQUARE_BRACKET.
In `@tests/phpunit/misc/test_FrmSpamCheckWPDisallowedWords.php`:
- Around line 63-65: The docblock in
tests/phpunit/misc/test_FrmSpamCheckWPDisallowedWords.php that currently
declares "@return bool" is incorrect because the method actually returns the
string values 'blacklist_keys' or 'disallowed_keys'; update the `@return`
annotation to "@return string" in that method's docblock so it correctly
reflects the return type.
🧹 Nitpick comments (4)
classes/models/fields/FrmFieldSelect.php (1)
94-97: Consider documenting the$valueparameter as well.The docblock only documents
$attsbut the method also has a$valueparameter. For completeness, consider adding documentation for both parameters.♻️ Suggested improvement
/** + * `@param` mixed $value * `@param` array $atts */ protected function prepare_import_value( $value, $atts ) {tests/phpunit/base/FrmUnitTest.php (1)
479-486: Consider using a union type for the$typeparameter.The
@param array $typeannotation is partially accurate, but the method also accepts a string since it casts the parameter to an array on line 486 with$type = (array) $type;. For completeness, consider using@param string|array $type.📝 Suggested documentation improvement
/** - * `@param` array $type + * `@param` string|array $type */ public static function generate_xml( $type, $xml_args ) {phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingDocblockSniff.php (2)
98-101: Remove unused local in processMissingAnnotations.
$tokensis never referenced.♻️ Proposed cleanup
- $tokens = $phpcsFile->getTokens(); - $params = $this->getParameters( $phpcsFile, $stackPtr );
540-572: Clean up unused locals in addMissingAnnotations.
$lastContentLine,$insertBefore, and$lastTagare unused; removing them simplifies the method.♻️ Proposed cleanup
- $lastContentLine = $tokens[ $closer ]['line'] - 1; - // Get indentation from docblock opener. $indent = ''; $lineStart = $docblock; @@ - // Find position to insert (before the closing tag). - $insertBefore = $closer; - - // Look for existing `@return` or `@param` to insert after. - $lastTag = null; - - for ( $i = $docblock; $i < $closer; $i++ ) { - if ( $tokens[ $i ]['code'] === T_DOC_COMMENT_TAG ) { - $lastTag = $i; - } - } - $fixer->beginChangeset();
…nd_type_comments Add missing block comments and type comments
1 participant
This requires turning off a php cs fixer rule for now so it doesn't add a ton of
mixedparam comments, messing up Psalm and PHPStan.Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.