Upgrade phpdocumentor/reflection-docblock to v6#79
Merged
Conversation
Member
Author
|
@coderabbitai review |
Contributor
Reviewer's GuideUpgrades phpDocumentor dependencies to v6, injects a shared DocBlockFactory into ReturnEntity via DI, and updates return-type parsing logic to support both Array_ and Generic types while wiring the factory in the MediaQuery module and tests. Sequence diagram for ReturnEntity docblock parsing with injected DocBlockFactorysequenceDiagram
actor Caller
participant DIContainer
participant DocBlockFactory as DocBlockFactoryInterface
participant ReturnEntity
participant Method as ReflectionMethod
participant ContextFactory
participant DocBlock
participant ReturnTag as Return_
participant Type
participant Array_
participant Generic
participant Object_
DIContainer->>DocBlockFactory: createInstance()
DIContainer->>ReturnEntity: __construct(DocBlockFactory)
Caller->>ReturnEntity: __invoke(Method)
ReturnEntity->>Method: getReturnType()
ReturnEntity-->>Caller: string or null (if ReflectionType is class-string)
Caller-->>ReturnEntity: (fallback when array-like)
ReturnEntity->>ReturnEntity: docblock(Method)
ReturnEntity->>ContextFactory: createFromReflector(Method)
ContextFactory-->>ReturnEntity: Context
ReturnEntity->>Method: getDocComment()
Method-->>ReturnEntity: docComment
ReturnEntity->>DocBlockFactory: create(docComment, Context)
DocBlockFactory-->>ReturnEntity: DocBlock
ReturnEntity->>DocBlock: getTagsByName(return)
DocBlock-->>ReturnEntity: ReturnTag[]
ReturnEntity->>ReturnTag: getType()
ReturnTag-->>ReturnEntity: Type
ReturnEntity->>ReturnEntity: extractValueType(Type)
alt Type is Array_
ReturnEntity->>Array_: getValueType()
Array_-->>ReturnEntity: Type
else Type is Generic
ReturnEntity->>Generic: getTypes()
Generic-->>ReturnEntity: Type[]
ReturnEntity->>ReturnEntity: select first Type
else other Type
ReturnEntity-->>ReturnEntity: null
end
ReturnEntity->>Object_: (check instance)
alt valueType is Object_
ReturnEntity-->>Caller: class-string
else
ReturnEntity-->>Caller: null
end
Class diagram for ReturnEntity with injected DocBlockFactory and updated type handlingclassDiagram
class ReturnEntityInterface
class ReturnEntity {
- DocBlockFactoryInterface docBlockFactory
+ __construct(DocBlockFactoryInterface docBlockFactory)
+ __invoke(ReflectionMethod method) string
- getReturnTypeName(ReflectionType reflectionType) string
- extractValueType(Type type) Type
- docblock(ReflectionMethod method) string
}
class MediaQueryDbModule {
+ configure() void
}
class DocBlockFactoryInterface
class DocBlockFactory {
+ createInstance() DocBlockFactory
}
class ContextFactory {
+ createFromReflector(ReflectionMethod method) Context
}
class Return_ {
+ getType() Type
}
class Type
class Array_ {
+ getValueType() Type
}
class Generic {
+ getTypes() Type[]
}
class Object_
ReturnEntity ..|> ReturnEntityInterface
DocBlockFactory ..|> DocBlockFactoryInterface
MediaQueryDbModule ..> DocBlockFactoryInterface : binds
MediaQueryDbModule ..> ReturnEntityInterface : binds
ReturnEntity ..> DocBlockFactoryInterface : uses
ReturnEntity ..> ContextFactory : uses
ReturnEntity ..> Return_ : uses
ReturnEntity ..> Type : uses
ReturnEntity ..> Array_ : uses
ReturnEntity ..> Generic : uses
ReturnEntity ..> Object_ : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
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. 📝 WalkthroughWalkthroughComposer upgrades for phpDocumentor packages; DocBlockFactory is bound in the DI module and injected into ReturnEntity, which uses a new extractValueType helper for docblock type handling; tests updated to use the injected DocBlockFactory instance. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
extractValueType, using$types[0]forGenericassumes the first type is always the value type; if you expect multi-parameter generics (e.g., key/value), consider constraining to known shapes or adding a more explicit selection/validation of the relevant type argument. - The
ReturnEntity::__invokemethod is documented as returning?class-stringbut is typed asstring|null; aligning the return type declaration with the docblock (e.g.,?string→?class-stringif your PHP version and tooling allow) would improve static analysis and consistency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `extractValueType`, using `$types[0]` for `Generic` assumes the first type is always the value type; if you expect multi-parameter generics (e.g., key/value), consider constraining to known shapes or adding a more explicit selection/validation of the relevant type argument.
- The `ReturnEntity::__invoke` method is documented as returning `?class-string` but is typed as `string|null`; aligning the return type declaration with the docblock (e.g., `?string` → `?class-string` if your PHP version and tooling allow) would improve static analysis and consistency.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/ReturnEntity.php`:
- Around line 58-68: The extractValueType(Type|null $type) helper currently
returns the first generic parameter for Generic instances, which picks the key
for multi-parameter generics; update the logic in extractValueType to return the
last type from Generic::getTypes() (use array_key_last($types) or equivalent) so
it returns the value type for both single-parameter (array<V>) and
multi-parameter (array<K, V>) generics; keep existing handling for Array_ and
return null when no types exist.
- Update phpdocumentor/reflection-docblock from ^5.3 to ^6.0 - Update phpdocumentor/type-resolver from ^1.6.1 to ^2.0 - Inject DocBlockFactoryInterface via constructor in ReturnEntity - Add DocBlockFactoryInterface binding in MediaQueryDbModule - Handle Generic type (v6) in addition to Array_ type for return types
Return the last type from Generic::getTypes() instead of the first, so array<K, V> correctly returns V (the value type) instead of K.
koriym
force-pushed
the
upgrade-phpdocumentor-v6
branch
from
7a64091 to
0e4158a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
phpdocumentor/reflection-docblockfrom ^5.3 to ^6.0phpdocumentor/type-resolverfrom ^1.6.1 to ^2.0DocBlockFactoryInterfacevia constructor inReturnEntityGenerictype (v6) in addition toArray_type for return typesPerformance
Test plan
composer tests- 83 tests, 124 assertions)Summary by Sourcery
Upgrade docblock parsing dependencies and adapt return type resolution to support new Generic types while integrating the docblock factory via dependency injection.
New Features:
Enhancements:
Build:
Tests:
Summary by CodeRabbit
Chores
Refactor
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.