-
Notifications
You must be signed in to change notification settings - Fork 516
More details php version information in diagnose #3609
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
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.
This shows me we need to do a bit of a refactoring.
Current state: ComposerPhpVersionFactory either reads phpVersion
from phpstan.neon (if it has min+max), or from composer.json
. And ConstantResolver uses ComposerPhpVersionFactory for this logic.
Wanted state: ComposerPhpVersionFactory only reads PHP version from composer.json
, as its name says. And then ConstantResolver decides if it should read phpVersion from phpstan.neon with min/max provided. And if it's not there then it should read it from ComposerPhpVersionFactory.
Feel free to do this in a separate PR before this one. Thanks.
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.
I'd say the composer.json PHP version is irrelevant if there's phpVersion.min+max version in phpstan.neon. The diagnose command should reflect that and print composer.json version only if phpVersion in phpstan.neon is int or null.
Looking at ComposerPhpVersionFactory usages, I found a bug: VersionCompareFunctionDynamicReturnTypeExtension only looks at composer version, not phpVersion.min+max version. |
I will send a separate PR |
Awesome, now it makes sense to me :) |
@@ -2108,6 +2108,7 @@ services: | |||
arguments: | |||
composerAutoloaderProjectPaths: %composerAutoloaderProjectPaths% | |||
allConfigFiles: %allConfigFiles% | |||
configPhpVersion: %phpVersion% | |||
autowired: false | |||
|
|||
# Error formatters |
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.
Fix_Resolve_Complete
as requested in #3584 (comment)