-
-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Add ServiceLocatorDynamicMethodReturnTypeExtension
to provide precise type inference for get()
method.
#56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… precise type inference for `get()` method.
WalkthroughThis change introduces a dynamic return type extension for Yii's Changes
Sequence Diagram(s)sequenceDiagram
participant StaticAnalyzer as PHPStan
participant Extension as ServiceLocatorDynamicMethodReturnTypeExtension
participant ServiceLocator as ServiceLocator
participant ServiceMap as ServiceMap
participant Reflection as ReflectionProvider
StaticAnalyzer->>Extension: Analyze ServiceLocator::get($id)
Extension->>ServiceMap: Resolve $id to class/service
alt Known component/service
ServiceMap-->>Extension: Return class name
Extension->>Reflection: Verify class exists
Reflection-->>Extension: Class exists
Extension-->>StaticAnalyzer: Return ObjectType(class)
else Unknown or dynamic $id
Extension-->>StaticAnalyzer: Return MixedType
end
Possibly related PRs
Suggested labels
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 #56 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 71 71
===========================================
Files 2 2
Lines 129 129
===========================================
Hits 129 129 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/type/ServiceLocatorDynamicMethodReturnTypeExtension.php (1)
97-97
: Consider usinginstanceof
for more robust type checking.The current type check
$methodCall->args[0]::class !== Arg::class
is unusual and only checks for the exact class. Usinginstanceof
would be more conventional and handle subclasses properly:-if (isset($methodCall->args[0]) === false || $methodCall->args[0]::class !== Arg::class) { +if (isset($methodCall->args[0]) === false || !$methodCall->args[0] instanceof Arg) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md
(1 hunks)README.md
(2 hunks)docs/examples.md
(5 hunks)extension.neon
(1 hunks)src/type/ServiceLocatorDynamicMethodReturnTypeExtension.php
(1 hunks)tests/data/type/ServiceLocatorDynamicMethodReturnType.php
(1 hunks)tests/type/ServiceLocatorDynamicMethodReturnTypeExtensionTest.php
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.4-ubuntu-latest
- GitHub Check: composer-require-checker / PHP 8.4-ubuntu-latest
🔇 Additional comments (15)
CHANGELOG.md (1)
26-26
: LGTM: Changelog entry is properly formatted.The enhancement entry follows the established format and accurately describes the new ServiceLocator dynamic return type extension feature.
extension.neon (1)
50-52
: LGTM: Service registration is properly configured.The ServiceLocatorDynamicMethodReturnTypeExtension is correctly registered with the appropriate PHPStan tag and follows the established pattern of other dynamic return type extensions.
docs/examples.md (2)
89-89
: LGTM: Minor grammar correction.The comment correction from "knows this returns" to "knows this return" improves grammatical consistency.
457-499
: LGTM: Comprehensive service locator examples.The new "Service locator in custom classes" section provides excellent practical examples demonstrating:
- Custom ServiceLocator extension
- Type-safe service resolution by ID and class name
- Proper error handling with logging
- Multiple service resolution patterns
The examples effectively showcase the enhanced type inference capabilities.
tests/type/ServiceLocatorDynamicMethodReturnTypeExtensionTest.php (1)
31-55
: LGTM: Well-structured PHPStan test class.The test class properly follows PHPStan's testing patterns:
- Correct inheritance from
TypeInferenceTestCase
- Proper use of data provider for fixture-based testing
- Additional config file specification for extension testing
- Comprehensive documentation explaining the test purpose and features
The implementation ensures robust testing of the ServiceLocator dynamic return type extension.
src/type/ServiceLocatorDynamicMethodReturnTypeExtension.php (1)
38-148
: LGTM: Solid implementation of dynamic return type extension.The ServiceLocatorDynamicMethodReturnTypeExtension is well-implemented with:
Strengths:
- Proper implementation of
DynamicMethodReturnTypeExtension
interface- Sound type resolution logic with appropriate fallback chain
- Comprehensive error handling and edge case management
- Excellent documentation with detailed PHPDoc comments
- Proper dependency injection pattern
- Focuses on the correct method (
get
) and class (ServiceLocator
)Type Resolution Logic:
- Component class resolution via ServiceMap
- Service class resolution via ServiceMap
- Direct class name verification via ReflectionProvider
- Fallback to MixedType for unknown/dynamic cases
This approach provides precise type inference while maintaining compatibility with PHPStan's analysis framework.
README.md (2)
60-65
: Well-documented feature with clear resolution priority.The Service Locator Component Resolution section effectively explains the new dynamic return type extension with clear priority ordering and comprehensive coverage of supported scenarios.
169-185
: Comprehensive examples demonstrating the new functionality.The service locator examples clearly illustrate the enhanced type inference capabilities, showing both class-based and string-based component resolution with appropriate type annotations.
tests/data/type/ServiceLocatorDynamicMethodReturnType.php (7)
1-34
: Well-structured test file with comprehensive documentation.The file header, imports, and class documentation clearly explain the purpose and scope of the ServiceLocator type inference tests. The PHPDoc is particularly thorough in explaining the test scenarios and key features.
40-45
: Correct test for class name constant resolution.This test validates that
ServiceLocator::get()
with a class name constant returns the expected type. The assertion correctly uses the fully qualified class name string.
50-57
: Effective test for string variable class name resolution.This test demonstrates that the extension can infer types even when the class name is passed as a string variable, which is important for real-world usage patterns.
62-67
: Proper fallback behavior validation.This test correctly validates that unknown component identifiers fall back to mixed type, which aligns with the documented behavior in the README.
72-80
: Comprehensive testing of built-in Yii2 components.The test covers multiple built-in Yii2 classes to ensure the extension works correctly with framework components.
85-103
: Thorough testing of component ID resolution and parameters.These tests validate both component ID string resolution and the optional
$throwException
parameter, ensuring the extension handles all method signatures correctly.
108-125
: Proper testing of ServiceLocator subclasses.The tests correctly validate that the extension works with
Application
andModule
classes, which inherit from ServiceLocator, ensuring comprehensive framework integration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces the ServiceLocatorDynamicMethodReturnTypeExtension to improve type inference for ServiceLocator::get() calls in Yii2 applications. It adds dynamic return type extension logic, updates tests and fixtures, and refines documentation and examples accordingly.
- Implements dynamic type resolution for service locator calls.
- Adds comprehensive tests and fixtures for the new feature.
- Updates documentation and changelog to reflect the precise type inference support.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/type/ServiceLocatorDynamicMethodReturnTypeExtensionTest.php | Adds test coverage for the new dynamic return type extension. |
tests/data/type/ServiceLocatorDynamicMethodReturnType.php | Introduces type assertion tests for various component resolution scenarios. |
src/type/ServiceLocatorDynamicMethodReturnTypeExtension.php | Implements dynamic return type logic based on ServiceMap and ReflectionProvider. |
extension.neon | Registers the new extension with PHPStan. |
docs/examples.md | Updates examples to reflect the improved type inference behavior. |
README.md | Documents the new capabilities for ServiceLocator component resolution. |
CHANGELOG.md | Records the new feature in the changelog. |
Summary by CodeRabbit
New Features
ServiceLocator::get()
method, providing more precise static analysis and autocompletion support in Yii2 applications.Documentation
Tests