add attribute targets #156
Conversation
Signed-off-by: Robert Landers <landers.robert@gmail.com>
WalkthroughThe attribute declarations for several PHP classes were updated to expand their applicability. Specifically, the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/State/Attributes/AllowCreateAll.php (1)
29-33: Align attribute flags with the rest of the AccessControl suiteMost sibling attributes (
AllowAnyOperation,DenyAnyOperation,AllowCreateFor*, etc.) are declared as repeatable. Unless there is a specific reason forAllowCreateAllto be single-use only, marking it repeatable keeps the API surface symmetric and avoids a compile-time error if someone accidentally stacks the attribute.-#[Attribute(Attribute::TARGET_ALL)] +#[Attribute(Attribute::IS_REPEATABLE | Attribute::TARGET_ALL)]If single-use is intentional, consider adding an explanatory doc-block to make that constraint explicit.
src/State/Attributes/AllowCreateForUser.php (1)
30-32: Align with sibling attributes by declaring the classreadonlyOther access-control attributes in this namespace (
AllowCreateForRole,AllowCreateAll, …) are markedreadonly, guaranteeing their public data remains immutable once instantiated.
For consistency and defensive coding, consider the same here:-class AllowCreateForUser implements AccessControl +readonly class AllowCreateForUser implements AccessControlsrc/State/Attributes/AllowAnyOperation.php (1)
10-17: Consider declaring the classreadonlyfor immutability parity
Several sibling attributes (AllowCreateForRole,AllowCreateForAuth,AllowCreateFrom) are declaredreadonly, signalling that their constructor-promoted properties should never mutate.AllowAnyOperationfollows the same “bag of params” pattern, so marking itreadonlyimproves consistency and prevents accidental state changes.- class AllowAnyOperation implements AccessControl + readonly class AllowAnyOperation implements AccessControl
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/State/Attributes/AllowAnyOperation.php(1 hunks)src/State/Attributes/AllowCreateAll.php(1 hunks)src/State/Attributes/AllowCreateForAuth.php(1 hunks)src/State/Attributes/AllowCreateForRole.php(1 hunks)src/State/Attributes/AllowCreateForUser.php(1 hunks)src/State/Attributes/AllowCreateFrom.php(1 hunks)src/State/Attributes/DenyAnyOperation.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/State/Attributes/AllowCreateFrom.php (6)
src/State/Attributes/AllowAnyOperation.php (1)
Attribute(9-18)src/State/Attributes/AllowCreateForAuth.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateAll.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForUser.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForRole.php (1)
Attribute(29-33)src/State/Attributes/DenyAnyOperation.php (1)
Attribute(31-40)
src/State/Attributes/AllowCreateForRole.php (4)
src/State/Attributes/AllowCreateForAuth.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateAll.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForUser.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateFrom.php (1)
Attribute(11-28)
src/State/Attributes/DenyAnyOperation.php (6)
src/State/Attributes/AllowAnyOperation.php (1)
Attribute(9-18)src/State/Attributes/AllowCreateForAuth.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateAll.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForUser.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForRole.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateFrom.php (1)
Attribute(11-28)
src/State/Attributes/AllowCreateForUser.php (4)
src/State/Attributes/AllowCreateForAuth.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateAll.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForRole.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateFrom.php (1)
Attribute(11-28)
src/State/Attributes/AllowCreateAll.php (6)
src/State/Attributes/AllowAnyOperation.php (1)
Attribute(9-18)src/State/Attributes/AllowCreateForAuth.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForUser.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForRole.php (1)
Attribute(29-33)src/State/Attributes/DenyAnyOperation.php (1)
Attribute(31-40)src/State/Attributes/AllowCreateFrom.php (1)
Attribute(11-28)
src/State/Attributes/AllowAnyOperation.php (6)
src/State/Attributes/AllowCreateForAuth.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateAll.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForUser.php (1)
Attribute(29-33)src/State/Attributes/AllowCreateForRole.php (1)
Attribute(29-33)src/State/Attributes/DenyAnyOperation.php (1)
Attribute(31-40)src/State/Attributes/AllowCreateFrom.php (1)
Attribute(11-28)
🔇 Additional comments (6)
src/State/Attributes/AllowCreateForAuth.php (1)
29-29: LGTM! Attribute target expansion correctly implemented.The addition of
Attribute::TARGET_ALLproperly expands the scope of this attribute to allow usage on any target type (classes, methods, properties, parameters, etc.). This change aligns with the PR objective and follows PHP attribute conventions.src/State/Attributes/DenyAnyOperation.php (1)
31-31: LGTM! Consistent attribute target expansion.The addition of
Attribute::TARGET_ALLexpands the attribute's applicability to all target types while maintaining its repeatable nature. This change is consistent with similar modifications made to other access control attributes in the same directory (AllowAnyOperation, AllowCreateForRole, AllowCreateForUser, etc.).src/State/Attributes/AllowCreateForRole.php (1)
29-29: LGTM! Consistent attribute target expansion.The addition of
Attribute::TARGET_ALLto the attribute declaration aligns with the systematic changes made across other access control attributes in this directory, providing greater flexibility for attribute placement while maintaining the repeatable nature.src/State/Attributes/AllowCreateForUser.php (1)
29-29: Target broadened toAttribute::TARGET_ALL– please confirm scope is intentionalExpanding the target to all language elements increases flexibility but also raises the chance the attribute is placed on irrelevant constructs (e.g. parameters, properties) where the runtime logic may ignore or mis-interpret it.
Double-check that your attribute-processing codepaths gracefully handle every possible target or, if not, limit the mask to only the supported ones (e.g.CLASS | METHOD).src/State/Attributes/AllowAnyOperation.php (1)
9-9: Consistent use ofTARGET_ALL– looks good
AddingAttribute::TARGET_ALLaligns this attribute with the others in the folder and broadens applicability without affecting behaviour.src/State/Attributes/AllowCreateFrom.php (1)
11-11: AllowCreateFrom attribute target expansion is safe.I’ve verified that
AllowCreateFromis only instantiated explicitly (viacreateAttribute) and isn’t auto-discovered on methods, properties, etc. No reflection code would suddenly pick it up on other targets, so addingTARGET_ALLis backward-compatible and aligns with the other attributes in this PR.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v2 #156 +/- ##
============================================
- Coverage 46.83% 46.77% -0.06%
Complexity 1313 1313
============================================
Files 103 103
Lines 3677 3677
============================================
- Hits 1722 1720 -2
- Misses 1955 1957 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Performance Metrics
|
Summary by CodeRabbit