Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Dynamically load XML container via PHP #163

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

Closed
ruudk wants to merge 1 commit into phpstan:master from ruudk:xml-container-loader

Conversation

@ruudk
Copy link
Contributor

@ruudk ruudk commented May 17, 2021

This allows to automatically resolve the XML container configuration based on environment variables.

It also has the benefit that it will automatically create the XML container file when it's not present (as it boots the container once).

Fixes #105

LoicBoursin reacted with rocket emoji Kocal reacted with eyes emoji
This allows to automatically resolve the XML container configuration based on environment variables.
It also has the benefit that it will automatically create the XML container file when it's not present (as it boots the container once).
Fixes phpstan#105 
```yaml
parameters:
symfony:
container_xml_path: tests/container-loader.php
Copy link
Member

@ondrejmirtes ondrejmirtes May 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit too magic for my taste. It's called container_xml_path. There should be a different config key for this scenario.

Copy link
Member

@ondrejmirtes ondrejmirtes May 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both config keys are filled with a value, the extension should throw an exception so that the user knows they should provide either XML or PHP, but not both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure if that makes it more clear.

The container_xml_path is just points to a PHP file that returns the XML.

How would you call it?

Copy link
Member

@ondrejmirtes ondrejmirtes May 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should make it more general and instead have a file that returns the whole container (which we can turn into the XML ourselves for our own purposes).

Copy link
Member

@ondrejmirtes ondrejmirtes May 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then it could be called container_loader

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before doing it like this, I tried to create a new ServiceContainerServiceMapFactory and ServiceContainerParameterMapFactory that would actually load the real container via a separate container_loader file.

The problem is, the container is compiled, and does not contain the data we need.

Copy link
Member

Looking at the issue I don't think we really need this complexity in the extension itself. The user can always dump the XML container based on whatever logic they want, to any path they want, and then run PHPStan. So you can do make phpstan which dumps the container with the right env, and then runs vendor/bin/phpstan.

Copy link
Contributor Author

ruudk commented May 17, 2021

I can create a script for sure. But it also means that the integration in PHPStorm will not work out of the box if you have no cache. Personally I rm -rf var/cache often. And I think more Symfony users do.

The console loader automatically boots and loads the application without manual work. In the Doctrine extension, the ObjectManager is automatically loaded without manual work.

Only this Service Container integration relies on a file to be present on disk and does not automatically creates it.

Copy link
Member

We could support container_loader that returns $kernel->getContainer(). The question is - what the user needs to do so that $container->getParameter('debug.container.dump') will always contain what we need?

Copy link

b2p-fred commented Dec 9, 2021

Any news about this PR since May ?

Copy link
Contributor Author

ruudk commented Dec 9, 2021

I lost interest in solving this as this time. Feel free to pick it up from here. Maybe I'll come back to it in the future and we can reopen this PR.

@ruudk ruudk closed this Dec 9, 2021
@ruudk ruudk deleted the xml-container-loader branch December 9, 2021 08:12
Copy link

I also encounter the following issue. The auto detection of XML container based on APP_ENV seems a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support APP_ENV for determining container filename

AltStyle によって変換されたページ (->オリジナル) /