New sniff to make type comments more accurate#2870
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. 📝 WalkthroughWalkthroughAdds a new PHP_CodeSniffer sniff that infers and fixes missing Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,220,255,0.5)
participant Dev as Developer/CI
end
rect rgba(200,255,200,0.5)
participant PHPCS as PHP_CodeSniffer Engine
end
rect rgba(255,220,200,0.5)
participant Sniff as AddMissingParamTypeSniff
end
rect rgba(255,255,200,0.5)
participant File as Source File (PHP)
end
Dev->>PHPCS: run phpcs with Formidable ruleset
PHPCS->>Sniff: invoke AddMissingParamTypeSniff.process(file)
Sniff->>File: parse tokens & extract functions + docblocks
Sniff->>File: scan function body for is_* checks
Sniff->>Sniff: normalize detected types vs existing `@param` types
alt missing/extra types found
Sniff->>PHPCS: report fixable error (missing type)
Sniff->>File: apply fixer -> update `@param` type union in docblock
else nothing to change
Sniff->>PHPCS: no issues reported
end
PHPCS->>Dev: output results (and fixed files if autofix run)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@classes/controllers/FrmFormActionsController.php`:
- Around line 640-643: Reorder the phpdoc union types for the parameters in the
FrmFormActionsController docblock so they follow the phpdoc_types_order rule and
are consistent: change the $entry annotation from "int|string|object" to
"int|object|string" (matching the existing $form "int|object|string"), ensuring
both `@param` lines use the same ordered union types.
In `@classes/controllers/FrmXMLController.php`:
- Around line 445-448: Replace the malformed PHPDoc type for the $type parameter
in FrmXMLController (the docblock showing "array<string>|string|array") with a
non-redundant, properly ordered union type such as "string|array" (or
"array<string>|string" if you need to enforce string elements); update the
`@param` line for $type accordingly so the phpdoc_types_order linter no longer
fails.
In `@classes/helpers/FrmXMLHelper.php`:
- Around line 1892-1893: The phpdoc union type for the parameter $str is ordered
as "string|array" which triggers phpdoc_types_order; update the docblock to use
the canonical order "array|string" for the `@param` declaration in
FrmXMLHelper.php (the docblock associated with the function/method that accepts
$str) so the linter (phpdoc_types_order) is satisfied.
In `@classes/models/FrmDb.php`:
- Around line 208-210: Update the PHPDoc for the parameter documented as
"int|string|float" in classes/models/FrmDb.php to match the project's
php-cs-fixer ordering by reordering the union types to "float|int|string"
(search for the PHPDoc block that contains the `@param` annotation with
$key/$value/$where and change the $value type ordering accordingly).
In `@classes/models/FrmEmail.php`:
- Around line 659-661: The phpdoc union types for the parameter and return are
ordered as "string|array" and "array|string" inconsistently and trigger
phpdoc_types_order; update the docblock so the union types follow the configured
ordering (e.g., change "@param string|array $emails" to "@param array|string
$emails" and ensure the "@return" is also "array|string" where applicable) —
locate the docblock around the $emails parameter in FrmEmail (the method that
declares $emails) and reorder the union types accordingly.
In `@classes/models/FrmSettings.php`:
- Around line 636-638: The PHPDoc for the parameter $sanitize uses
"string|callable" which triggers phpdoc_types_order; update the `@param`
annotation to "callable|string" (swap the types) in the method's docblock (the
docblock that documents $sanitize in FrmSettings.php) so the types are
alphabetically ordered and satisfy the PHP CS Fixer rule.
In `@phpcs-sniffs/Formidable/Sniffs/Commenting/AddMissingParamTypeSniff.php`:
- Around line 26-42: The mapping for is_numeric in the $isCheckToType lookup
inside AddMissingParamTypeSniff is too broad (currently 'string'); update the
'is_numeric' entry to map to the narrower union "int|float|numeric-string" so
the sniff infers numeric unions correctly (locate the $isCheckToType array and
change the value for the 'is_numeric' key).
- Around line 264-305: The loop in findIsChecksForParam incorrectly scans into
nested T_FUNCTION/T_FN bodies causing is_* checks in inner closures to be
attributed to the outer parameter; detect when $tokens[$i]['code'] is T_FUNCTION
or T_FN and, if that token has a 'scope_closer' (or
'scope_opener'/'scope_closer' pair), advance $i to that scope_closer (skip the
inner scope) before continuing so nested functions/arrow functions are not
scanned for is_* checks for the outer $varName.
…nts_more_accurate New sniff to make type comments more accurate
This sniff aims to make type comments more complete.
If a function checks
is_*, then the comments should reflect that the param could be a variable of that type.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.