-
Notifications
You must be signed in to change notification settings - Fork 90
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
make creation of ServiceMap and ParameterMap lazy #390
base: 1.3.x
Are you sure you want to change the base?
Conversation
Currently one can not use PHPStans `bootstrapFiles` to create the container (dump) which could then be ready by `symfony.containerXmlPath`. This as the order of initialization results in the `XmlServiceMapFactory` / `XmlParameterMapFactory` `create` methods being called before the `bootstrapFiles` are executed. By making the `ServiceMap` / `ParameterMap` lazy these will only be created when actually used, which is after the `bootstrapFiles` have been executed.
@@ -18,6 +18,6 @@ public function getParameter(string $key): ?ParameterDefinition; | |||
/** | |||
* @return array<string> | |||
*/ | |||
public static function getParameterKeysFromNode(Expr $node, Scope $scope): array; | |||
public function getParameterKeysFromNode(Expr $node, Scope $scope): array; |
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 might be considered a breaking change and thus require a major version bump?
@@ -15,6 +15,6 @@ public function getServices(): array; | |||
|
|||
public function getService(string $id): ?ServiceDefinition; | |||
|
|||
public static function getServiceIdFromNode(Expr $node, Scope $scope): ?string; | |||
public function getServiceIdFromNode(Expr $node, Scope $scope): ?string; |
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.
Same as above for ParameterMap
change, this might be considered a breaking change and thus require a major version bump?
Whoops, changes are PHP 7.2 and 7.3 incompatible. I will fix them if considered mergable etc. But if the changes to the interface are decided to be a breaking change and require a major version bump it might be considered to bump the minimum versions as well? |
this is okay. I’m not willing to merge anything that would make the code more complex, because you can generate the container dump before running PHPStan. It’s been done like that for many years and people don’t have a problem with that. For example you can use make or Composer scripts to still invoke a single command. |
Currently one can not use PHPStans
bootstrapFiles
to create the container (dump) which could then be ready bysymfony.containerXmlPath
. This as the order of initialization results in theXmlServiceMapFactory
/XmlParameterMapFactory
create
methods being called before thebootstrapFiles
are executed. By making theServiceMap
/ParameterMap
lazy these will only be created when actually used, which is after thebootstrapFiles
have been executed.