refactor: Move UserPropertiesClassReflectionExtension to property directory and add testing.
#44
Conversation
… directory and add testing.
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 16 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (4)
""" WalkthroughThe changes reorganize the Changes
Sequence Diagram(s)sequenceDiagram
participant TestCase as UserPropertiesClassReflectionExtensionTest
participant PHPStan as PHPStan Type Inference Engine
participant Fixture as UserPropertiesClassReflectionType
participant User as User (stub)
TestCase->>PHPStan: Run type inference assertions from Fixture
PHPStan->>User: Analyze properties/methods (e.g., identity, id, getAuthKey)
User-->>PHPStan: Return types per IdentityInterface implementation
PHPStan-->>TestCase: Report inferred types for assertions
Possibly related PRs
Poem
✨ 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. 🪧 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #44 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 62 62
===========================================
Files 1 1
Lines 107 107
===========================================
Hits 107 107 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
src/property/UserPropertiesClassReflectionExtension.php (2)
142-145:⚠️ Potential issue
isSubclassOfClass()is not part of PHPStan’s API — will trigger a fatal error.
ClassReflectionexposesisSubclassOf(string $className): bool, notisSubclassOfClass(). Passing aClassReflectioninstance is also wrong.- $classReflection->isSubclassOfClass($this->reflectionProvider->getClass(User::class)) === false + $classReflection->isSubclassOf(User::class) === falseWithout this fix the extension crashes during analysis.
104-110: 🛠️ Refactor suggestion
isGuestshould beBooleanType, not the constanttrue.
$user->isGuestcan be eithertrueorfalse. UsingConstantBooleanType(true)falsely narrows the type and hides bugs where the value isfalse.- new ConstantBooleanType(true), + new \PHPStan\Type BooleanType(),Also, in
hasProperty()the existence ofisGuest(andid) is tied toidentityClasswhich is unnecessary; those properties always exist.
🧹 Nitpick comments (2)
CHANGELOG.md (1)
14-14: Capitalization inconsistent with existing entries.All prior bullets start with
Bug,Enh, etc. Line 14 uses lowercasebug, breaking the pattern and changelog automation that might rely on the prefix.- - bug #44: Move `UserPropertiesClassReflectionExtension` to `property` directory and add testing (@terabytesoftw) + - Bug #44: Move `UserPropertiesClassReflectionExtension` to `property` directory and add testing (@terabytesoftw)tests/fixture/data/property/UserPropertiesClassReflectionType.php (1)
46-54: Static call via a potentially-null object risks fatal error.
Yii::$app->user->identitymay benull; invoking::findIdentity*()on it will crash at runtime, even though the snippet is used only for static analysis.
Consider asserting on the class directly or guarding with null-safe operator to reflect real-world usage:assertType('yii\web\IdentityInterface|null', Yii::$app->user->identity?::findIdentityByAccessToken('123abc'));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)extension.neon(1 hunks)src/property/UserPropertiesClassReflectionExtension.php(5 hunks)tests/fixture/data/property/UserPropertiesClassReflectionType.php(1 hunks)tests/property/UserPropertiesClassReflectionExtensionTest.php(1 hunks)tests/stub/User.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/property/UserPropertiesClassReflectionExtension.php (2)
src/reflection/ComponentPropertyReflection.php (1)
ComponentPropertyReflection(37-271)src/ServiceMap.php (1)
ServiceMap(60-537)
tests/fixture/data/property/UserPropertiesClassReflectionType.php (1)
tests/stub/User.php (5)
validateAuthKey(48-51)findIdentityByAccessToken(33-36)findIdentity(28-31)getAuthKey(43-46)getId(38-41)
🔇 Additional comments (1)
extension.neon (1)
29-31: Confirm removal of the oldreflection\UserPropertiesClassReflectionExtensionservice.The service has been re-registered under
property\…. Please double-check that the oldreflection\UserPropertiesClassReflectionExtensionclass file and its service entry were deleted to avoid PHPStan loading the same logic twice.
tests/fixture/data/property/UserPropertiesClassReflectionType.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the UserPropertiesClassReflectionExtension by relocating it to a dedicated property directory, updating its namespace and configuration, and adds comprehensive tests and a stub user class for PHPStan type inference.
- Moved
UserPropertiesClassReflectionExtensionfromreflectiontopropertydirectory and updated namespaces. - Updated
extension.neonto register the relocated extension and removed the old registration. - Added a test stub (
tests/stub/User.php), PHPStan property tests, fixtures, and updated the changelog.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/stub/User.php | Implemented IdentityInterface methods in the user stub. |
| tests/property/UserPropertiesClassReflectionExtensionTest.php | Added a test class for property reflection extension. |
| tests/fixture/data/property/UserPropertiesClassReflectionType.php | Created fixture tests to assert inferred property and method types. |
| src/property/UserPropertiesClassReflectionExtension.php | Updated namespace and documentation for the property extension. |
| extension.neon | Registered the moved property extension and removed the old one. |
| CHANGELOG.md | Documented the move of the extension and added the testing notes. |
Comments suppressed due to low confidence (1)
CHANGELOG.md:14
- The changelog entry uses a lowercase 'bug' and lacks a version header; please capitalize 'Bug #44' to match the existing pattern and add a new version section (e.g., '## 0.2.4') above this entry.
- bug #44: Move `UserPropertiesClassReflectionExtension` to `property` directory and add testing (@terabytesoftw)
…opertiesClassReflectionExtension`.
…nExtension` move.
Summary by CodeRabbit
Bug Fixes
Documentation
Tests