Upgrade to phpstan 2.0 #12
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12 +/- ##
============================================
- Coverage 17.16% 15.38% -1.78%
- Complexity 125 131 +6
============================================
Files 14 14
Lines 268 299 +31
============================================
Hits 46 46
- Misses 222 253 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughThe GitHub Actions workflow job was renamed from Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant PHPStan
participant Composer
Developer->>Composer: Update phpstan and related package versions
Developer->>phpstan.neon: Add bleeding edge rules include
Developer->>GitHub Actions: Rename static analysis job to phpstan
GitHub Actions->>PHPStan: Run static analysis with updated config
PHPStan-->>Developer: Report analysis results
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code Graph Analysis (3)src/Reflection/UserPropertiesClassReflectionExtension.php (1)
src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php (2)
src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php (4)
🪛 GitHub Check: codecov/patchsrc/Reflection/UserPropertiesClassReflectionExtension.php[warning] 36-36: src/Reflection/UserPropertiesClassReflectionExtension.php#L36 src/Reflection/ApplicationPropertiesClassReflectionExtension.php[warning] 52-52: src/Reflection/ApplicationPropertiesClassReflectionExtension.php#L52 src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php[warning] 40-44: src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php#L40-L44 [warning] 66-66: src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php#L66 [warning] 76-80: src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php#L76-L80 [warning] 87-87: src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php#L87 [warning] 99-99: src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php#L99 [warning] 107-107: src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php#L107 src/Type/ActiveRecordDynamicMethodReturnTypeExtension.php[warning] 47-47: src/Type/ActiveRecordDynamicMethodReturnTypeExtension.php#L47 [warning] 53-54: src/Type/ActiveRecordDynamicMethodReturnTypeExtension.php#L53-L54 [warning] 59-60: src/Type/ActiveRecordDynamicMethodReturnTypeExtension.php#L59-L60 [warning] 64-64: src/Type/ActiveRecordDynamicMethodReturnTypeExtension.php#L64 src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php[warning] 27-27: src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php#L27 [warning] 30-30: src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php#L30 [warning] 43-45: src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php#L43-L45 [warning] 48-48: src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php#L48 [warning] 55-60: src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php#L55-L60 [warning] 66-71: src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php#L66-L71 [warning] 75-75: src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php#L75 [warning] 84-88: src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php#L84-L88 [warning] 103-103: src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php#L103 src/Type/ActiveRecordObjectType.php[warning] 21-22: src/Type/ActiveRecordObjectType.php#L21-L22 [warning] 27-27: src/Type/ActiveRecordObjectType.php#L27 [warning] 35-36: src/Type/ActiveRecordObjectType.php#L35-L36 🔇 Additional comments (16)
✨ 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: 1
🔭 Outside diff range comments (1)
src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php (1)
74-82:⚠️ Potential issue
getValue()may throw when argument isn’t aConstantBooleanType
$argTypeis only checked with$argType->isBoolean()->no().
For a variable boolean (e.g.$flag), the type can beBooleanType, which passes the boolean check but has nogetValue()method.
Calling it will trigger a fatal error inside PHPStan’s runtime.-$argType = isset($methodCall->args[0]) && $methodCall->args[0] instanceof Arg - ? $scope->getType($methodCall->args[0]->value) : new ConstantBooleanType(true); -if ($argType->isBoolean()->no()) { +$argType = isset($methodCall->args[0]) && $methodCall->args[0] instanceof Arg + ? $scope->getType($methodCall->args[0]->value) + : new ConstantBooleanType(true); + +// ensure we can safely call getValue() +if ( + !$argType instanceof ConstantBooleanType + || $argType->isBoolean()->no() +) { throw new ShouldNotHappenException( sprintf('Invalid argument provided to asArray method at line %d', $methodCall->getLine()), ); } return new ActiveQueryObjectType( $calledOnType->getModelClass(), $argType->getValue(), );This guarantees
getValue()is invoked only on a compatible type and avoids unexpected crashes during analysis.
🧹 Nitpick comments (2)
src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php (1)
25-31: Consider constructor property promotion & immutabilitySince PHP 8.1 you can shrink the boiler-plate and make the dependency immutable in one go:
- private ReflectionProvider $reflectionProvider; - - public function __construct( - ReflectionProvider $reflectionProvider, - ) { - $this->reflectionProvider = $reflectionProvider; - } + public function __construct( + private readonly ReflectionProvider $reflectionProvider, + ) { + }This reduces noise and clearly communicates that the provider should not change after instantiation.
src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php (1)
41-44: UseselectFromArgs()for argument-aware support checks
isMethodSupported()still relies onselectSingle(), which disregards actual call arguments.
Switching toParametersAcceptorSelector::selectFromArgs()(as done elsewhere) would make the support check consistent with the upgraded return-type logic and prevent false negatives when a method has multiple variants.No functional bug, but worth aligning for completeness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/static.yml(1 hunks)composer.json(1 hunks)phpstan.neon(1 hunks)src/Reflection/ApplicationPropertiesClassReflectionExtension.php(1 hunks)src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php(5 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)tests/ServiceMapTest.php(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/Reflection/ApplicationPropertiesClassReflectionExtension.php (1)
src/ServiceMap.php (1)
ServiceMap(26-141)
src/Type/ActiveRecordObjectType.php (4)
src/Reflection/ApplicationPropertiesClassReflectionExtension.php (1)
hasProperty(27-40)src/Reflection/ResponsePropertiesClassReflectionExtension.php (1)
hasProperty(20-27)src/Reflection/UserPropertiesClassReflectionExtension.php (1)
hasProperty(20-28)src/Reflection/RequestPropertiesClassReflectionExtension.php (1)
hasProperty(20-27)
src/Type/ActiveQueryDynamicMethodReturnTypeExtension.php (2)
src/Type/ActiveRecordObjectType.php (1)
ActiveRecordObjectType(14-40)src/Type/ActiveQueryObjectType.php (2)
getModelClass(18-21)isAsArray(23-26)
🔇 Additional comments (11)
composer.json (1)
18-18: LGTM! Appropriate dependency for PHPStan 2.0 upgrade.The addition of
phpstan-deprecation-rulesis a standard and recommended dependency when upgrading to PHPStan 2.0, as it helps identify deprecated features during static analysis.src/Reflection/ApplicationPropertiesClassReflectionExtension.php (1)
24-24: LGTM! Good code style improvement.The trailing comma addition improves diff readability and is consistent with modern PHP best practices. Since the project requires PHP 8.1+, this syntax is fully supported.
tests/ServiceMapTest.php (1)
48-48: LGTM! Consistent code style improvements.The trailing comma additions across multiple test methods improve code style consistency and diff readability when modifying constructor arguments.
Also applies to: 61-61, 74-74
src/Type/HeaderCollectionDynamicMethodReturnTypeExtension.php (1)
48-48: LGTM! Necessary API adaptation for PHPStan 2.0.The change from direct property access (
parts[0]) to method call (getParts()[0]) is a typical API evolution in PHPStan 2.0, where direct property access is replaced with method calls for better encapsulation while maintaining the same functionality.src/Type/ActiveRecordDynamicMethodReturnTypeExtension.php (2)
37-37: LGTM! Code style improvements.The trailing comma additions improve code consistency and readability, aligning with the broader style improvements across the codebase.
Also applies to: 47-47, 58-59
53-53: LGTM! Correct API adaptation for PHPStan 2.0.The change from
instanceof ConstantStringTypetocount($argType->getConstantStrings()) === 0is a proper adaptation to PHPStan 2.0's API changes. The logic remains correct - checking if there are no constant strings available rather than checking the type instance directly.src/Type/ActiveRecordObjectType.php (2)
21-21: LGTM! Correct PHPStan 2.0 API migration.The change from
instanceof ConstantStringTypetocount($offsetType->getConstantStrings()) === 0correctly implements the PHPStan 2.0 API migration pattern.
34-34: LGTM! Consistent API migration pattern.The change to use
count($offsetType->getConstantStrings()) > 0instead of instanceof checks aligns with the PHPStan 2.0 upgrade and maintains the same logical behavior.phpstan.neon (1)
5-6: LGTM! Enhanced PHPStan 2.0 configuration.The addition of bleeding edge rules and deprecation rules enhances the static analysis capabilities and aligns well with the PHPStan 2.0 upgrade objectives.
.github/workflows/static.yml (1)
25-25: LGTM! Correct job name update.The job name change from
psalmtophpstancorrectly reflects the migration to PHPStan 2.0 for static analysis.src/Type/ContainerDynamicMethodReturnTypeExtension.php (1)
45-49: LGTM! Improved argument-aware type inference.The change from
ParametersAcceptorSelector::selectSingletoselectFromArgswith scope and arguments consideration provides more accurate return type inference by selecting the appropriate variant based on the actual method call context.
src/Type/ActiveRecordDynamicStaticMethodReturnTypeExtension.php
Outdated
Show resolved
Hide resolved
Just starting this by following https://github.com/phpstan/phpstan-src/blob/2.0.x/UPGRADING.md.
Any help is appreciated. Still getting some errors that I don't understand.
Summary by CodeRabbit