New sniff to make sure unmodified params match filter types#2890
Conversation
📝 WalkthroughWalkthroughA new PHP_CodeSniffer sniff is introduced to detect and fix misaligned Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. ✨ 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: 1
🤖 Fix all issues with AI agents
In
`@phpcs-sniffs/Formidable/Sniffs/Commenting/AlignFilterParamTypesWithFunctionSniff.php`:
- Around line 35-48: The $assignmentTokens array in
AlignFilterParamTypesWithFunctionSniff.php is missing the exponentiation and
null coalescing assignment tokens which can cause false positives; update the
private $assignmentTokens property (the array defined in
AlignFilterParamTypesWithFunctionSniff) to include T_POW_EQUAL and
T_COALESCE_EQUAL alongside the existing tokens so those operators (**= and ??=)
are recognized as assignments by the sniff.
🧹 Nitpick comments (3)
phpcs-sniffs/Formidable/ruleset.xml (1)
51-51: Rule placed in wrong section.This
Formidable.Commenting.*rule is placed in the Code Analysis section, but the Commenting section starts at line 73. Consider moving it there for consistency with the file's organizational structure.<rule ref="Formidable.CodeAnalysis.InlineFrmSettingsReturn" /> - <rule ref="Formidable.Commenting.AlignFilterParamTypesWithFunction" /> <rule ref="Formidable.CodeAnalysis.PreferPositiveTernaryCondition" />Then add it to the Commenting section (around line 77):
<rule ref="Formidable.Commenting.AlignFilterParamTypesWithFunction" />phpcs-sniffs/Formidable/Sniffs/Commenting/AlignFilterParamTypesWithFunctionSniff.php (2)
355-362: Missingfalsealias for consistency.The aliases map
truetoboolbut notfalse. For consistency,falseshould also be mapped tobool.Suggested fix
$aliases = array( 'integer' => 'int', 'boolean' => 'bool', 'double' => 'float', 'real' => 'float', 'number' => 'float', 'true' => 'bool', + 'false' => 'bool', );
159-168: MissingT_READONLYtoken in ignore list.For PHP 8.1+ compatibility,
T_READONLYshould be included in the ignore list. Without it, the sniff may fail to find function docblocks when thereadonlymodifier is used on constructor promoted properties or class methods.Suggested fix
$ignore = array( T_WHITESPACE, T_STATIC, T_PUBLIC, T_PRIVATE, T_PROTECTED, T_ABSTRACT, T_FINAL, T_ATTRIBUTE, + T_READONLY, );PHP 8.1 readonly keyword with functions
| private $assignmentTokens = array( | ||
| T_EQUAL, | ||
| T_PLUS_EQUAL, | ||
| T_MINUS_EQUAL, | ||
| T_MUL_EQUAL, | ||
| T_DIV_EQUAL, | ||
| T_CONCAT_EQUAL, | ||
| T_MOD_EQUAL, | ||
| T_AND_EQUAL, | ||
| T_OR_EQUAL, | ||
| T_XOR_EQUAL, | ||
| T_SL_EQUAL, | ||
| T_SR_EQUAL, | ||
| ); |
There was a problem hiding this comment.
Missing assignment operators could cause false positives.
The assignment tokens list is missing T_POW_EQUAL (**=) and T_COALESCE_EQUAL (??=). If a variable is modified using these operators, the sniff won't detect it and may incorrectly report a type mismatch.
Suggested fix
private $assignmentTokens = array(
T_EQUAL,
T_PLUS_EQUAL,
T_MINUS_EQUAL,
T_MUL_EQUAL,
T_DIV_EQUAL,
T_CONCAT_EQUAL,
T_MOD_EQUAL,
T_AND_EQUAL,
T_OR_EQUAL,
T_XOR_EQUAL,
T_SL_EQUAL,
T_SR_EQUAL,
+ T_POW_EQUAL,
+ T_COALESCE_EQUAL,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private $assignmentTokens = array( | |
| T_EQUAL, | |
| T_PLUS_EQUAL, | |
| T_MINUS_EQUAL, | |
| T_MUL_EQUAL, | |
| T_DIV_EQUAL, | |
| T_CONCAT_EQUAL, | |
| T_MOD_EQUAL, | |
| T_AND_EQUAL, | |
| T_OR_EQUAL, | |
| T_XOR_EQUAL, | |
| T_SL_EQUAL, | |
| T_SR_EQUAL, | |
| ); | |
| private $assignmentTokens = array( | |
| T_EQUAL, | |
| T_PLUS_EQUAL, | |
| T_MINUS_EQUAL, | |
| T_MUL_EQUAL, | |
| T_DIV_EQUAL, | |
| T_CONCAT_EQUAL, | |
| T_MOD_EQUAL, | |
| T_AND_EQUAL, | |
| T_OR_EQUAL, | |
| T_XOR_EQUAL, | |
| T_SL_EQUAL, | |
| T_SR_EQUAL, | |
| T_POW_EQUAL, | |
| T_COALESCE_EQUAL, | |
| ); |
🤖 Prompt for AI Agents
In
`@phpcs-sniffs/Formidable/Sniffs/Commenting/AlignFilterParamTypesWithFunctionSniff.php`
around lines 35 - 48, The $assignmentTokens array in
AlignFilterParamTypesWithFunctionSniff.php is missing the exponentiation and
null coalescing assignment tokens which can cause false positives; update the
private $assignmentTokens property (the array defined in
AlignFilterParamTypesWithFunctionSniff) to include T_POW_EQUAL and
T_COALESCE_EQUAL alongside the existing tokens so those operators (**= and ??=)
are recognized as assignments by the sniff.
…ified_params_match_filter_types New sniff to make sure unmodified params match filter types
This came up in review for https://github.com/Strategy11/formidable-forms/pull/2513/files.
It isn't really an issue in any pre-existing code, but it should help automate another possible code review issue.