New sniff to remove more redundant uses of empty#3005
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. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a new PHP_CodeSniffer sniff to detect and auto-fix redundant Changes
Possibly related PRs
Poem
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
DeepSource Code ReviewWe reviewed changes in See full review on DeepSource ↗ Code Review Summary
|
|
|
Overall Grade Focus Area: Hygiene |
Security Reliability Complexity Hygiene |
Feedback
- Implicit constant visibility is copy-pasted
- Multiple files declare REQUIRED_CAPABILITY without an explicit visibility because developers copy existing code; make those constants explicit (public/protected/private) and update any snippets so the omission stops propagating.
- Controller-level permissions duplicated across classes
- Each controller defines its own REQUIRED_CAPABILITY, duplicating intent and risking inconsistent access scope; centralize capability declarations (base controller constant, permission enum, or auth mapping) so one explicit visibility and semantics apply.
- Generators omit visibility in produced code
- Project scaffolding or templates likely emit class constants without visibility, seeding the same hygiene defect across new files; update generators/snippets to always include explicit visibility so the pattern is broken at source.
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP | Mar 6, 2026 9:42p.m. | Review ↗ | |
| JavaScript | Mar 6, 2026 9:42p.m. | Review ↗ |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnStaticPropertySniff.php`:
- Line 189: The phpdoc return type ordering must be changed to satisfy
phpdoc_types_order: update the return annotation that currently reads
"false|array{accessor: string, property: string}" to "array{accessor: string,
property: string}|false" in the docblock for the method that returns the static
access info in RedundantEmptyOnStaticPropertySniff (the docblock for the method
returning the static access info).
- Line 298: The variable $searchStart in RedundantEmptyOnStaticPropertySniff
(assigned with "$searchStart = $i;") is never used; remove the unused assignment
to eliminate the dead variable—locate the assignment inside the method where $i
is set (the context around the static property scanning logic) and delete the
"$searchStart = $i;" line (or, if the original intent was to use it, replace
other references to use $searchStart consistently), ensuring no other code
depends on that variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f135fbf5-d152-4e8d-bdf1-0afe5407f505
📒 Files selected for processing (9)
classes/controllers/FrmAddonsController.phpclasses/controllers/FrmFormTemplatesController.phpclasses/models/FrmFormState.phpclasses/models/FrmInbox.phpclasses/models/FrmSalesApi.phpphpcs-sniffs/Formidable/Sniffs/CodeAnalysis/PreferNotEmptyTernarySniff.phpphpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnStaticPropertySniff.phpphpcs-sniffs/Formidable/ruleset.xmlstripe/helpers/FrmStrpLiteConnectHelper.php
|
|
||
| // Check if this declaration includes 'static'. | ||
| $hasStatic = false; | ||
| $searchStart = $i; |
There was a problem hiding this comment.
Remove unused variable $searchStart.
Static analysis correctly identifies that $searchStart is assigned but never used.
🧹 Proposed fix
// Check if this declaration includes 'static'.
$hasStatic = false;
- $searchStart = $i;
// Look ahead through modifiers to find 'static' and the variable.📝 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.
| $searchStart = $i; | |
| // Check if this declaration includes 'static'. | |
| $hasStatic = false; | |
| // Look ahead through modifiers to find 'static' and the variable. |
🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 298-298: Avoid unused local variables such as '$searchStart'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnStaticPropertySniff.php`
at line 298, The variable $searchStart in RedundantEmptyOnStaticPropertySniff
(assigned with "$searchStart = $i;") is never used; remove the unused assignment
to eliminate the dead variable—locate the assignment inside the method where $i
is set (the context around the static property scanning logic) and delete the
"$searchStart = $i;" line (or, if the original intent was to use it, replace
other references to use $searchStart consistently), ensuring no other code
depends on that variable.
…undant_uses_of_empty New sniff to remove more redundant uses of empty
Summary by CodeRabbit
Release Notes
Refactor
empty()utility function calls with direct PHP truthiness evaluation for improved code consistency.New Features
Chores