feat: Enhance PHPStan integration with yii2 extensions.
#13
Conversation
WalkthroughThis change standardizes all namespaces and configuration references from PascalCase to lowercase across the codebase, updates dependency and script definitions in Changes
Sequence Diagram(s)sequenceDiagram
participant Composer
participant Autoloader
participant AppCode
participant Config
Composer->>Autoloader: Register PSR-4 namespaces (now lowercase)
Autoloader->>AppCode: Load classes using lowercase namespaces
AppCode->>Config: Reference services/types using lowercase paths
Config-->>AppCode: Provide configuration/services
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 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: 5
♻️ Duplicate comments (1)
src/type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php (1)
71-72: 🛠️ Refactor suggestionSecond occurrence of the undefined method
See previous comment – replace with
isSubclassOf(ActiveQuery::class)or the reflection-aware overload.
🧹 Nitpick comments (1)
composer.json (1)
45-47: Running ECS with--fixinside CI may introduce unstaged changesInvoking ECS with
--fixmutates source files. If this script is meant for CI, prefer--no-interaction/--dry-run(or separate check vs fix scripts) to fail the build instead of silently modifying code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
composer.json(3 hunks)ecs.php(3 hunks)extension.neon(1 hunks)phpstan.neon(1 hunks)src/ServiceMap.php(2 hunks)src/reflection/ApplicationPropertiesClassReflectionExtension.php(3 hunks)src/reflection/ComponentPropertyReflection.php(2 hunks)src/reflection/RequestMethodsClassReflectionExtension.php(2 hunks)src/reflection/RequestPropertiesClassReflectionExtension.php(1 hunks)src/reflection/ResponsePropertiesClassReflectionExtension.php(1 hunks)src/reflection/UserPropertiesClassReflectionExtension.php(1 hunks)src/type/ActiveQueryDynamicMethodReturnTypeExtension.php(1 hunks)src/type/ActiveQueryObjectType.php(1 hunks)src/type/ActiveRecordDynamicMethodReturnTypeExtension.php(2 hunks)src/type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php(3 hunks)src/type/ActiveRecordObjectType.php(2 hunks)src/type/ContainerDynamicMethodReturnTypeExtension.php(1 hunks)src/type/HeaderCollectionDynamicMethodReturnTypeExtension.php(1 hunks)stubs/BaseYii.stub(0 hunks)tests/ServiceMapTest.php(7 hunks)tests/fixture/yii-config-valid.php(1 hunks)tests/yii/MyActiveRecord.php(1 hunks)tests/yii/MyController.php(1 hunks)
💤 Files with no reviewable changes (1)
- stubs/BaseYii.stub
🧰 Additional context used
🧬 Code Graph Analysis (3)
tests/fixture/yii-config-valid.php (1)
tests/yii/MyActiveRecord.php (1)
MyActiveRecord(12-37)
tests/ServiceMapTest.php (2)
src/ServiceMap.php (1)
ServiceMap(26-141)tests/yii/MyActiveRecord.php (1)
MyActiveRecord(12-37)
src/type/ContainerDynamicMethodReturnTypeExtension.php (1)
src/ServiceMap.php (1)
ServiceMap(26-141)
🔇 Additional comments (30)
src/type/ActiveQueryObjectType.php (1)
5-5:✅ Verification successful
LGTM! Verify consistent namespace updates across the codebase.
The namespace change from PascalCase to lowercase aligns with the PR objective to standardize naming conventions.
Please run the following script to ensure all namespace references have been updated consistently:
🏁 Script executed:
#!/bin/bash # Description: Verify all references to the old PascalCase namespace have been updated # Search for any remaining PascalCase namespace references echo "Checking for old PascalCase namespace references..." rg -i "Yii2\\\\Extensions\\\\PHPStan" --type php # Search for new lowercase namespace usage echo "Verifying new lowercase namespace usage..." rg "yii2\\\\extensions\\\\phpstan" --type php # Check composer.json autoload configuration echo "Checking composer.json autoload configuration..." cat composer.json | jq '.autoload'Length of output: 4448
Namespace Standardization Verified
All references to the old
Yii2\Extensions\PHPStannamespace have been checked with a case-sensitive search—no matches remain. The PSR-4 autoload mapping incomposer.jsoncorrectly uses the new lowercase prefix.
- No
Yii2\Extensions\PHPStan(PascalCase) occurrences in PHP filescomposer.jsonautoloads"yii2\\extensions\\phpstan\\": "src"as expectedNo further changes required.
src/type/HeaderCollectionDynamicMethodReturnTypeExtension.php (1)
5-5: LGTM! Namespace standardization is consistent.The namespace change follows the same lowercase convention being applied across the codebase, maintaining consistency with the broader refactoring effort.
src/type/ActiveRecordDynamicMethodReturnTypeExtension.php (2)
5-5: LGTM! Consistent namespace standardization.The namespace change aligns with the project-wide standardization to lowercase naming conventions.
54-54: LGTM! Improved code readability.The added blank line improves readability by providing visual separation before the conditional logic that handles the exception case.
src/type/ActiveRecordObjectType.php (2)
5-5: LGTM! Consistent namespace standardization.The namespace change follows the same lowercase convention being applied across all files in this PR.
35-36: Excellent bug fix with the nullsafe operator!The addition of the nullsafe operator (
?->) prevents a potential null pointer exception since the$offsetTypeparameter is nullable (?Type $offsetType). This is a meaningful improvement that enhances the robustness of the code.The added blank line also improves code readability by providing visual separation in the logic flow.
src/type/ActiveQueryDynamicMethodReturnTypeExtension.php (2)
48-50: AI summary discrepancy: @throws annotation still present
The AI-generated summary mentioned that the@throws ShouldNotHappenExceptionPHPDoc was removed, but it remains in lines 48–50. Please confirm whether this annotation should be dropped or kept.Likely an incorrect or invalid review comment.
5-5:✅ Verification successful
Lowercase namespace normalization
The namespace has been converted fromYii2\Extensions\PHPStan\Typetoyii2\extensions\phpstan\type, aligning with the project-wide standard.
Please verify that yourcomposer.jsonPSR-4 autoload mapping has been updated to reflect this change.
🏁 Script executed:
#!/bin/bash # Verify PSR-4 autoload mapping for phpstan namespaces rg -n '"yii2\\\\extensions\\\\phpstan"' composer.jsonLength of output: 55
Let's list out the PSR-4 mappings and explicitly search for the updated namespace entry in composer.json to confirm it’s present:
🏁 Script executed:
#!/bin/bash # Show all PSR-4 mappings jq '.autoload["psr-4"]' composer.json # Check for the exact phpstan namespace key (with trailing backslash) rg -n '"yii2\\\\extensions\\\\phpstan\\\\":' composer.jsonLength of output: 195
PSR-4 autoload mapping verified
Confirmed thatcomposer.jsonincludes the required entry underautoload.psr-4:"yii2\\extensions\\phpstan\\": "src"No further changes needed.
phpstan.neon (1)
20-22: Updated fixture path
Theyii2.config_pathwas updated to point totests/fixture/yii-config-valid.php, matching the renamed test fixtures directory. This aligns with the changes intests/ServiceMapTest.phpand other test files.tests/yii/MyActiveRecord.php (1)
5-5:✅ Verification successful
Normalized test namespace to lowercase
The namespace has been converted toyii2\extensions\phpstan\tests\yii. Ensure that the PSR-4 autoload incomposer.jsonfor dev dependencies covers this updated namespace.
🏁 Script executed:
#!/bin/bash # Verify PSR-4 mapping for test namespace rg -n '"yii2\\\\extensions\\\\phpstan\\\\tests\\\\yii"' composer.jsonLength of output: 71
🏁 Script executed:
#!/bin/bash # Inspect autoload-dev PSR-4 mappings in composer.json rg -n '"autoload-dev"' composer.json -A5 rg -n '"psr-4"' composer.json -A5Length of output: 502
PSR-4 autoload-dev mapping confirmed
The
composer.jsonalready includes the following PSR-4 entry underautoload-dev, which coversyii2\extensions\phpstan\tests\(and thus theyii2\extensions\phpstan\tests\yiinamespace):"autoload-dev": { "psr-4": { "yii2\\extensions\\phpstan\\tests\\": "tests" } }No further changes are required.
tests/yii/MyController.php (1)
5-5:✅ Verification successful
Normalized test namespace to lowercase
The namespace has been updated toyii2\extensions\phpstan\tests\yii. Confirm that your dev autoload configuration incomposer.jsonincludes this path.
🏁 Script executed:
#!/bin/bash # Verify PSR-4 mapping for test namespace rg -n '"yii2\\\\extensions\\\\phpstan\\\\tests\\\\yii"' composer.jsonLength of output: 71
🏁 Script executed:
#!/bin/bash # List PSR-4 autoload-dev mappings to verify test namespace mapping jq -r ' .["autoload-dev"]["psr-4"]? | to_entries[] | "\(.key) => \(.value)" ' composer.json || echo "No PSR-4 autoload-dev mappings found in composer.json"Length of output: 139
PSR-4 autoload-dev mapping validated
Theyii2\extensions\phpstan\tests\yiinamespace is correctly mapped to thetests/directory incomposer.json. No further changes needed.src/reflection/RequestPropertiesClassReflectionExtension.php (1)
5-5:✅ Verification successful
Lowercase namespace normalization
Changed namespace fromYii2\Extensions\PHPStan\Reflectiontoyii2\extensions\phpstan\reflection. This matches the refactoring in other source and config files.
Please verify that the maincomposer.jsonautoload PSR-4 section has been updated accordingly.
🏁 Script executed:
#!/bin/bash # Verify PSR-4 autoload mapping for reflection namespace rg -n '"yii2\\\\extensions\\\\phpstan\\\\reflection"' composer.jsonLength of output: 69
🏁 Script executed:
#!/bin/bash # List PSR-4 autoload mappings in composer.json to verify namespace casing if command -v jq >/dev/null; then jq '.autoload["psr-4"]' composer.json else # Fallback to grep if jq is unavailable grep -R -n '"psr-4"' -n composer.json -A10 fiLength of output: 98
PSR-4 Autoload Namespace Verified
Thecomposer.jsonPSR-4 section already maps"yii2\\extensions\\phpstan\\"to"src", which covers the new lowercaseyii2\extensions\phpstan\reflectionnamespace. No further changes are required.src/reflection/ResponsePropertiesClassReflectionExtension.php (1)
5-5: Consistent namespace standardization.The namespace change follows the same pattern as other files in this refactoring. Ensure this is coordinated with updates to configuration files and external references.
tests/fixture/yii-config-valid.php (1)
5-5: Import statement correctly updated.The import statement has been properly updated to use the new lowercase namespace convention, maintaining consistency with the overall refactoring.
src/type/ContainerDynamicMethodReturnTypeExtension.php (2)
5-5: Namespace standardization applied consistently.The namespace has been correctly converted to lowercase following the established pattern.
16-16: ServiceMap import updated correctly.The import statement for ServiceMap has been properly updated to use the new lowercase namespace convention.
src/reflection/RequestMethodsClassReflectionExtension.php (1)
5-5: Namespace change consistent with refactoring pattern.The namespace has been correctly updated to follow the lowercase convention.
src/reflection/ComponentPropertyReflection.php (2)
5-5: LGTM: Namespace standardization to lowercase.This change aligns with the overall refactoring effort to standardize namespaces from PascalCase to lowercase throughout the codebase.
14-14:❓ Verification inconclusive
Verify the impact of removing readonly modifiers.
The
readonlyproperty modifier has been removed from both$fallbackPropertyand$typeconstructor parameters. This changes these properties from immutable to mutable, which could affect the intended design and thread safety of this class.Please confirm that this change is intentional and that the class doesn't rely on the immutability guarantees provided by
readonlyproperties.
🏁 Script executed:
#!/bin/bash # Description: Check if readonly modifier was removed consistently across other reflection classes # Expected: Find other reflection classes that may have had similar constructor changes # Search for other reflection classes and their constructors ast-grep --pattern $'class $_ implements $_Reflection { $$$ public function __construct($$$) { $$$ } $$$ }' # Also check for any remaining readonly usage in reflection classes rg -A 3 -B 3 "readonly" --include="*Reflection*.php"Length of output: 615
🏁 Script executed:
#!/bin/bash # Re-run search for any remaining readonly modifiers in Reflection classes rg -A 3 -B 3 readonly -g '*Reflection*.php'Length of output: 2234
Verify consistent immutability across reflection classes
It looks like
ComponentPropertyReflection’s constructor parameters were changed from immutable to mutable by droppingreadonly, whereas other reflection extensions still useprivate readonly. This inconsistency could introduce unintended mutability.Please confirm that removing
readonlyhere is intentional and that this class’s design does not rely on immutability guarantees.• src/reflection/UserPropertiesClassReflectionExtension.php (constructor uses
private readonly)
• src/reflection/ResponsePropertiesClassReflectionExtension.php
• src/reflection/RequestPropertiesClassReflectionExtension.phptests/ServiceMapTest.php (3)
5-5: LGTM: Namespace standardization to lowercase.The namespace and use statement changes consistently convert from PascalCase to lowercase, aligning with the codebase-wide refactoring.
Also applies to: 15-16
135-135: LGTM: Good testing practice.Adding
$this->expectNotToPerformAssertions()is a good practice for tests that verify behavior through constructor execution without explicit assertions, making the test intent clear.
36-38:✅ Verification successful
Verify that the fixture directory structure exists.
All test fixture paths have been updated from 'assets' to 'fixture'. Please ensure that the fixture directory and all referenced configuration files exist at the new location.
Also applies to: 49-50, 52-52, 63-64, 66-66, 77-78, 80-80, 88-88, 90-90, 137-137
🏁 Script executed:
#!/bin/bash # Description: Verify that all fixture files referenced in tests exist # Expected: All fixture files should be present in the tests/fixture directory echo "Checking if tests/fixture directory exists..." ls -la tests/fixture/ || echo "Directory tests/fixture/ does not exist" echo -e "\nChecking specific fixture files referenced in tests..." test_files=( "tests/fixture/yii-config-invalid.php" "tests/fixture/yii-config-invalid-unsupported-type.php" "tests/fixture/yii-config-invalid-unsupported-array.php" "tests/fixture/yii-config-invalid-component.php" "tests/fixture/yii-config-valid.php" "tests/fixture/yii-config-no-singletons.php" ) for file in "${test_files[@]}"; do if [ -f "$file" ]; then echo "✓ $file exists" else echo "✗ $file is missing" fi doneLength of output: 2391
All referenced test fixtures are present
Verified that the
tests/fixture/directory exists and contains all files referenced in ServiceMapTest.php. No further changes are required.extension.neon (1)
14-44: LGTM: PHPStan configuration updated to match namespace changes.All service class references have been consistently updated from PascalCase to lowercase namespaces, ensuring the PHPStan configuration aligns with the codebase refactoring. The configuration structure and service definitions remain intact.
src/ServiceMap.php (2)
5-5: LGTM: Namespace standardization to lowercase.This change aligns with the overall refactoring effort to standardize namespaces from PascalCase to lowercase throughout the codebase.
106-106: LGTM: PHPStan type annotation reordering.The type annotation has been reordered from
string|Closure|array<mixed>|inttoarray<mixed>|string|Closure|int. This change doesn't affect runtime behavior and may improve consistency or readability.ecs.php (1)
47-55: Potential rule collision / duplicate inclusion
SingleQuoteFixer::classis registered via->withRules()whileperCS20: truealready pulls in the PhpCsFixer set that containsSingleQuoteFixerwith default config. This can lead to duplicate rule definitions or unexpected config precedence.
Double-check the executed rule set and, if unnecessary, remove the explicit entry.src/reflection/ApplicationPropertiesClassReflectionExtension.php (4)
5-5: LGTM! Namespace standardization improves consistency.The namespace casing standardization from PascalCase to lowercase aligns with the broader project-wide effort mentioned in the summary. This improves consistency across the codebase.
Also applies to: 17-17
29-34: Excellent improvement using PHPStan's reflection API.The change from native PHP
is_subclass_ofto PHPStan'sisSubclassOfClass()method is a significant improvement. Using PHPStan's reflection system provides more accurate analysis capabilities and better integration with the static analysis context.
57-60: LGTM! Improved code formatting.The multi-line formatting with trailing commas improves readability and follows modern PHP coding standards. This makes future diffs cleaner when adding or modifying parameters.
22-24:✅ Verification successful
Verify the removal of
readonlymodifiers.The
readonlymodifiers have been removed from constructor properties. This allows these properties to be modified after construction, which could be intentional but might also introduce unintended mutability.Please confirm that removing the
readonlymodifiers is intentional and that these properties don't need to remain immutable after construction.
🏁 Script executed:
#!/bin/bash # Description: Check for any assignments to these properties outside the constructor # Expected: No assignments found outside constructor, or intentional mutations # Search for assignments to these properties rg -A 3 -B 3 '\$this->annotationsProperties\s*=' --type php rg -A 3 -B 3 '\$this->reflectionProvider\s*=' --type php rg -A 3 -B 3 '\$this->serviceMap\s*=' --type phpLength of output: 811
No unintended mutability detected after removing
readonlymodifiersI checked the entire codebase for assignments to these three properties and found that all of them are only ever set in their constructors—there are no external mutations. Removing
readonlyhere does not introduce unintended mutability.
There was a problem hiding this comment.
Pull Request Overview
This PR improves PHPStan integration by standardizing namespaces, adjusting constructor modifiers, refining code style rules, and updating CI/workflow configurations.
- Standardize all namespaces and class references to lowercase for PSR-4 compliance
- Remove
readonlymodifiers from constructors for broader compatibility - Enhance coding standards (import order, single-quote enforcement) and update CI scripts and workflows
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/type/ActiveRecordDynamicMethodReturnTypeExtension.php | Namespace casing update |
| src/type/ActiveQueryObjectType.php | Namespace casing update |
| src/type/ActiveQueryDynamicMethodReturnTypeExtension.php | Refactor method return type logic |
| src/reflection/UserPropertiesClassReflectionExtension.php | Namespace casing update |
| src/reflection/ResponsePropertiesClassReflectionExtension.php | Namespace casing update |
| src/reflection/RequestPropertiesClassReflectionExtension.php | Namespace casing update |
| src/reflection/RequestMethodsClassReflectionExtension.php | Namespace casing update |
| src/reflection/ComponentPropertyReflection.php | Removed readonly in constructor |
| src/reflection/ApplicationPropertiesClassReflectionExtension.php | Namespace update & subclass-of check refactor |
| src/ServiceMap.php | Namespace update & PHPDoc union type normalization |
| phpstan.neon | Updated config_path to fixtures folder |
| extension.neon | Updated service class references to lowercase namespaces |
| ecs.php | Added import/order & single-quote style rules |
| composer.json | Extended constraints and updated scripts |
| .github/workflows/static.yml | Aligned Yii2 dev version requirement |
| .github/workflows/ecs.yml | Aligned Yii2 dev version requirement |
| .github/workflows/dependency-check.yml | Aligned Yii2 dev version requirement |
| .github/workflows/compatibility.yml | Bumped Yii2 composer requirement |
| .github/workflows/build.yml | Bumped PHP and Yii2 versions |
| .github/FUNDING.yml | Added GitHub funding information |
Comments suppressed due to low confidence (2)
src/type/ActiveQueryDynamicMethodReturnTypeExtension.php:73
- Removing the default initialization of
$boolValuecan lead to an undefined variable when neither branch runs. Consider reintroducing a default assignment or adding anelseclause to ensure$boolValueis always defined.
if ($argType->isTrue()->yes()) {
ecs.php:31
- Configuring
VisibilityRequiredFixerwith an emptyelementsarray disables the rule. Verify whether you intend to enforce specific element visibilities or remove the rule if unused.
'elements' => [],
Summary by CodeRabbit
Refactor
readonlykeyword from several class constructors.Chores
Documentation