[Config][DependencyInjection] Add configuration builder for writing PHP config #40600

Nyholm · 2021-03-27T20:28:43Z

Q	A
Branch?	5.x
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets
License	MIT
Doc PR	symfony/symfony-docs#15181

I've spent most part of today to generate this PR. It is far from complete but it is ready for review.

This PR will build classes and store them in the build_dir. The classes will help you write PHP config. It will basically generate an array for you.

Before

// config/packages/security.php
<?php

use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $container) {
  $array = [
        'firewalls' => [
            'main' => [
                'pattern' => '^/*',
                'lazy' => true,
                'anonymous' => [],
            ],
        ],
        'access_control' => [
            [
                'path' => '^/user',
                'roles' => [
                    0 => 'ROLE_USER',
                ],
            ],
            [
                'path' => '^/admin',
                'roles' => 'ROLE_ADMIN',
            ],
        ],
        'role_hierarchy' => [
            'ROLE_ADMIN' => ['ROLE_USER'],
            'ROLE_SUPER_ADMIN' => ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH',
            ],
        ],
    ];

    $container->extension('security', $array);
}

After

// config/packages/security.php
<?php

use Symfony\Config\SecurityConfig;

return static function (SecurityConfig $security) {
    $security
        ->roleHierarchy('ROLE_ADMIN', ['ROLE_USER'])
        ->roleHierarchy('ROLE_SUPER_ADMIN', ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH'])
        ->accessControl()
            ->path('^/user')
            ->role('ROLE_USER');

    $security->accessControl(['path' => '^/admin', 'roles' => 'ROLE_ADMIN']);
    $security->firewall('main')
        ->pattern('^/*')
        ->lazy(true)
        ->anonymous();

};

About autogeneration

This PR is generating the extension's ConfigBuilders when they are first used. Since the PR is already very large, I prefer to follow up with additional PRs to include a cache warmer and command to rebuild the ConfigBuilders.

The generated ConfigBuilder uses a "ucfirst() camelCased" extension alias. If the alias is acme_foo the root ConfigBuilder will be Symfony\Config\AcmeFooConfig.

The recommended way of using this class is:

// config/packages/acme_foo.php
use Symfony\Config\AcmeFooConfig;

return static function (AcmeFooConfig $foo) {
  // ...
  // No need to return 
}

One may also init the class directly, But this will not help you with generation or autoloading

// config/packages/acme_foo.php
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $container) {
  $foo = new \Symfony\Config\AcmeFooConfig();
  // ...
  $container->extension('acme_foo', $foo->toArray());
}

I do think we should only talk about the first way

If a third party bundle like this idea and want to provide their own ConfigBuilder, they have two options:

Create the class Symfony\Config\TheBundleConfig themselves and make sure they configure composer to autoload that file and that the class implements ConfigBuilderInterface. We will never regenerate a file that already exists.
Create any class implementing ConfigBuilderInterface and ask their users to use that class in their config in the same way they would use Symfony\Config\TheBundleConfig.

The first way is obviously the smoothest way of doing things.

BC

There is a great discussion about backwards compatibility for the generated files. We can assure that the class generator don't introduce a BC break with our tests. However, if the bundle changes their configuration definition it may break BC. Things like renaming, changing type or changing a single value to array is obvious BC breaks, however, these can be fixed in the config definition with normalisation.

The generator does not support normalisation. It is way way more complicated to reverse engineer that. I think a future update could fix this in one of two ways:

Add extra definition rules to help the class generator
Allow the bundle to patch part of the generated code

I hate BC breaks as much as the next person, but all the BC breaks in the generated classes will be caught when building the container (not at runtime), so I am fine with not having a 100% complete solution for this issue in the initial PR.

Other limitations

If a bundle is using a custom extension alias, then we cannot guess it.. so a user have to use a cache warmer because we cannot generate the ConfigBuilder on the fly.

TODO

Add tests
Update changelog
Write documentation

The generated code can be found in this example app: https://github.com/Nyholm/sf-issue-40600/tree/main/var/cache/dev/Symfony/Config

Nyholm · 2021-03-27T21:03:28Z

Hm.. I don't understand the psalm error..

https://psalm.dev/r/f2f527484f

jderusse · 2021-03-27T22:44:48Z

I really love this feature that remind me the work from @HeahDude .
I'm super happy and excited about it!!

I worry about edge cases that will not be handled (ie. all our beforeNormalize code)

A small detail, I would remove the set and add prefixes (as we go for router and service configuration)

wouterj · 2021-03-27T23:17:59Z

Hm.. I don't understand the psalm error..

Seems like an issue with psalm: https://psalm.dev/r/c97a88395d ArrayNode::getChildren() (and ArrayNode::$children) have no type information other than array. Seems like Psalm does something weird in the case statements when there is no type information for that variable.

Nyholm · 2021-03-28T10:05:35Z

A small detail, I would remove the set and add prefixes (as we go for router and service configuration)

How about removing the add and set prefixes for all method but not the ones that add a new node. Ie we keep addAccessControl() and addAnonymous()

azjezz · 2021-03-28T11:13:03Z

Seems like Psalm does something weird in the case statements when there is no type information for that variable.

no, the code contains a bug :) you are switching $object, not true.

see: https://psalm.dev/r/92bdc7be90

however, psalm should probably provide a better trace for this scenario.

src/Symfony/Component/Config/Builder/Generator.php

Nyholm · 2021-03-28T11:46:42Z

Just note about the method names. We have a few different scenarios.

Example	Description
`->setFoo(type $value): self`	Set a simple value to "Foo"
`->setFoo($value): self`	Set a simple value to "Foo" with no type (VariableNode)
`->addFoo(type $value): self`	Add a value to the list of "foo". (No `type` when VariableNode)
`->addFoo(string $key, type $value): self`	Add a value to the list of "foo" with a key (No `type` when VariableNode)
`->addFoo(array $value): Foo`	Add a new "Foo" node with some initial values
`->addFoo(array $value): Foo`	Add a new "Foo" node with some initial values (using prototype)
`->addFoo(string $key, array $value): Foo`	Add a new "Foo" node with key and some initial values (using prototype)

EDIT: This is updated, we have dropped all add and set prefixes.

muglug · 2021-03-28T14:40:50Z

however, psalm should probably provide a better trace for this scenario.

It actually cannot when $object's type is wide open – the code is identical to this, but passing takesAll(false) would pass that check:

function takesAll($object): void {
    if ($object == ($object instanceof Foo)) {
        takesFoo($object);
    }
}

ro0NL · 2021-03-28T18:00:55Z

This is brilliant :) IIUC the generated types are problematic for env values (aka string placeholders in Config)

Nyholm · 2021-03-28T18:42:30Z

Using environment variables are fine. Just remember:

Correct

  /** @var \Config\Framework\FrameworkConfig $framework */
    $framework = $container->extensionBuilder('framework');
    $framework->addLock()
        ->addResource('acme', ['%env(LOCK_DSN)%']);

Wrong way

  /** @var \Config\Framework\FrameworkConfig $framework */
    $framework = $container->extensionBuilder('framework');
    $framework->addLock()
         // This shows the wrong way. Dont do this. 
        ->addResource('acme', [getenv('LOCK_DSN')]);

wouterj · 2021-03-28T19:15:33Z

@Nyholm also for BooleanNodes (e.g. setDebug('%env(bool:APP_DEBUG)%'))?

ro0NL · 2021-03-28T19:48:20Z

See ValidateEnvPlaceholdersPassTest, the generated class should be compatible with current tests for EnvConfiguration.

I think it needs eg. bool|Env $value. With #28896 we'd know a a bit more, ie. which nodes support such a type.

nicolas-grekas

I'm sure

return static function (ContainerConfigurator $container) {
    /** @var \Config\Security\SecurityConfig $security */

could be turned into

return static function (SecurityConfig $security) {

by hooking into the FileLoader, that'd be neat :)

I didn't read the code in Generator yet and I won't be able to do so before the feat freeze for 5.3 unfortunately.

But this looks promising!

src/Symfony/Component/Config/Builder/Property.php

src/Symfony/Component/Config/Builder/Generator.php

Nyholm · 2021-04-02T09:50:52Z

I've updated the PR with a lot of tests and docs.
I've also followed Nicolas suggestion.

The PR description is updated. I would like to have a final round of reviews, merge and then follow up with PRs for specific features like cache warmer. This PR is large and complex enough as it is.

jderusse

Thank you very much for this PR, I really like how it brings PHP configuration to the end-user without asking the maintainer to creates these Config classes. ❤️

I played with it on a demo project and faced few issues:

class does not exist (and IDE is not aware of it) until cache generates theme. BUT cache generates the classes only when you create a config that uses it. So you have to know that the class will exist (and have to know the FQDN), to create a file that uses it, let the IDE report error, warm the cache, and then you can use it.
It's didn't experimented a great DX on this part :(
Because of our normalizers, (and because there is no chain/fluent parent/end methods), simple config became complex to setup.
ie. excluded_http_codes in monolog

monolog:
  handlers:
    main:
      type: fingers_crossed
      excluded_http_codes: [404, 405]

    $handler = $monolog->handler('main')
        ->type('fingers_crossed')
    ;
    $handler->excludedHttpCode()->code('404');
    $handler->excludedHttpCode()->code('405');

I don't think this should be implemented in this PR, but we should find a way to provide an "alias" in this new Config

I tried to migrate my monolog.yaml to PHP, but failed

    $handler = $monolog->handler('main')
        ->type('fingers_crossed')
        ->acceptedLevel(['error'])
        ->handler('nested')
        ->bufferSize(50)
    ;
    $handler->excludedHttpCode()->code('404');
    $handler->excludedHttpCode()->code('405');

leads to

Invalid configuration for path "monolog.handlers.main": You can only use excluded_http_codes/excluded_404s with a FingersCrossedHandler definition

I didn't digged into it, but when dumping config->toArray() I well have type=fingers_crossed
But the validate node of the extension (that trigger the exception) see type=stream

Deprecations are not leverage. We should, at least, annotate the methods @deprecated (ie framework.profiler.only_master_requests)

src/Symfony/Component/Config/Builder/ConfigBuilderGenerator.php

jderusse · 2021-04-03T12:29:40Z

src/Symfony/Component/Config/Builder/ConfigBuilderGenerator.php

+        }
+
+        if ($node instanceof EnumNode) {
+            $comment .= sprintf(' * @param %s $value', implode('|', array_map(function ($a) {


I love this!

src/Symfony/Component/Config/Builder/ConfigBuilderGenerator.php

src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php

Nyholm · 2021-04-03T21:18:20Z

Awesome. Thank you.

I'll follow up this PR with a cache warmer so it gets a bit better experience.
That is strange... I get the same error if I copy the config from the docs:

return static function (ContainerBuilder $container) {
    $container->loadFromExtension('monolog', [
        'handlers' => [
            'main' => [
                'type' => 'fingers_crossed',
                'excluded_http_codes' => [404, 404],
            ],
        ],
    ]);
};

Great idea. I missed that

Nyholm · 2021-04-03T21:38:30Z

Thank you for the review. I really appreciate you taking the time to test and experiment with this.

I'll double check the monolog config thing later.
I'll also update the namespace to something a bit simpler. Maybe just Symfony\Config\DoctrineMigrations and Symfony\Config\Monolog.

nicolas-grekas

this looks super nice :)

src/Symfony/Component/Config/Builder/ClassBuilder.php

src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php

src/Symfony/Component/Config/Builder/GeneratorInterface.php

nicolas-grekas · 2021-04-11T21:16:08Z

src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php

+     * @param string $namespace example extension alias ("framework") or FQCN string
+     *                          for a class implementing ConfigBuilderInterface
+     */
+    final public function configBuilder(string $namespace): ConfigBuilderInterface


do we need this method? what about allowing only passing via arguments as resolved by the loader?
using this method doesn't work this autocompletion anyway

You are correct. Thank you.

src/Symfony/Component/Config/Builder/ConfigBuilderGenerator.php

nicolas-grekas

I just realized one more thing: following #40214, the generated builders should have a when() method, for the same purpose.

Nyholm · 2021-04-12T11:02:43Z

I just realized one more thing: following #40214, the generated builders should have a when() method, for the same purpose.

I agree. I will be happy to work on this as soon as this PR is merged. Not adding that feature in this PR would allow to quicker move forward.

I've update the PR. It is ready for a review.

status: need review

nicolas-grekas

All good on my side, thanks for the quick changes!

…HP config

nicolas-grekas · 2021-04-13T16:59:23Z

Thank you @Nyholm.

Nyholm · 2021-04-13T17:00:52Z

Wohoo!

Excellent. Thank you for all the reviews.

This PR was squashed before being merged into the 5.3-dev branch. Discussion ---------- [Config] Add docs for ConfigBuilders This is config for symfony/symfony#40600 We should wait for the code merge Commits ------- 5b427af [Config] Add docs for ConfigBuilders

TomasVotruba · 2021-12-21T21:59:10Z

Hi, I'm trying to find or generate these classes in loca /var/tmp... directory to Rector on Symfony 6, but I can't find them.
Is there some command I have to run on post-composer install?

This is the full composer.json
https://github.com/rectorphp/getrector.org/blob/e759066c7597a09615d7e8d7533625e764dd9877/composer.json#L6-L56

Thanks

Guuzen · 2022-01-08T00:00:02Z

@TomasVotruba
Has same issue. It turned out that on warmup (while this configs are generated) exception about misset env variable was thrown
Try to set breakpoint in Symfony\Bundle\FrameworkBundle\CacheWarmer\ConfigBuilderCacheWarmer::warmUp
Or probably you can find some information in log (in my case logger is null so error just silently skipped)

TomasVotruba · 2022-01-08T07:23:19Z

@Guuzen Thanks, I'll try to remember when I'm debugging again.
I wish there was a simple command, like in other dev operations, like debug:env, debug:router etc.`

bin/console dump:configs

Nyholm added DependencyInjection Config labels Mar 27, 2021

carsonbot changed the title ~~Add configuration builder for writing PHP config~~ [Config][DependencyInjection] Add configuration builder for writing PHP config Mar 27, 2021

carsonbot added Status: Needs Review Feature labels Mar 27, 2021

Nyholm force-pushed the php-config-builder branch 2 times, most recently from d3a8e3b to c2f9a74 Compare March 27, 2021 22:08

chalasr requested a review from weaverryan March 27, 2021 23:28

azjezz reviewed Mar 28, 2021

View reviewed changes

src/Symfony/Component/Config/Builder/Generator.php Outdated Show resolved Hide resolved

nicolas-grekas added this to the 5.x milestone Apr 1, 2021

nicolas-grekas reviewed Apr 1, 2021

View reviewed changes

src/Symfony/Component/Config/Builder/Property.php Outdated Show resolved Hide resolved

src/Symfony/Component/Config/Builder/Generator.php Outdated Show resolved Hide resolved

src/Symfony/Component/Config/Builder/Generator.php Outdated Show resolved Hide resolved

Nyholm force-pushed the php-config-builder branch 2 times, most recently from 707dd68 to e7d5a78 Compare April 2, 2021 08:50

Nyholm force-pushed the php-config-builder branch from e0937fa to 106826c Compare April 2, 2021 10:31

Nyholm mentioned this pull request Apr 2, 2021

[Config] Add docs for ConfigBuilders symfony/symfony-docs#15181

Merged

jderusse reviewed Apr 3, 2021

View reviewed changes

nicolas-grekas mentioned this pull request Apr 11, 2021

[DependencyInjection] Add ContainerConfigurator::compilerPass() to allow specifying compiler passes #40355

Closed

nicolas-grekas reviewed Apr 11, 2021

View reviewed changes

nicolas-grekas requested changes Apr 12, 2021

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Apr 12, 2021

Nyholm force-pushed the php-config-builder branch from f883c6a to e2c9f25 Compare April 12, 2021 11:00

Nyholm force-pushed the php-config-builder branch 2 times, most recently from c80564c to 463940b Compare April 12, 2021 11:58

nicolas-grekas approved these changes Apr 12, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Apr 12, 2021

nicolas-grekas added the Ready label Apr 13, 2021

[Config][DependencyInjection] Add configuration builder for writing P…

460b46f

…HP config

nicolas-grekas force-pushed the php-config-builder branch from c338646 to 460b46f Compare April 13, 2021 16:59

nicolas-grekas merged commit 44bb691 into symfony:5.x Apr 13, 2021

Nyholm deleted the php-config-builder branch April 13, 2021 20:04

fabpot mentioned this pull request Apr 18, 2021

Release v5.3.0-BETA1 #40853

Merged

loic425 mentioned this pull request May 11, 2021

Grid config builders Sylius/SyliusGridBundle#155

Merged

TomasVotruba referenced this pull request in rectorphp/rector-src Jun 5, 2021

[scoper] unprefix container only in our code

d481136

faizanakram99 mentioned this pull request Jul 1, 2021

Support for Symfony\Config\* classes phpstan/phpstan-symfony#182

Closed

HeahDude mentioned this pull request Sep 6, 2023

[Config] Dumped config builders in cache dir when defining a custom kernel.build_dir #51582

Closed

raitocz mentioned this pull request Nov 24, 2023

PHP config builder is using "var/", magic and is preferred over arrays in documentation symfony/symfony-docs#19178

Closed

yceruto mentioned this pull request Feb 12, 2025

[FrameworkBundle] Generate Config class #58771

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Config][DependencyInjection] Add configuration builder for writing PHP config #40600

[Config][DependencyInjection] Add configuration builder for writing PHP config #40600

Nyholm commented Mar 27, 2021 •

edited

Loading

Nyholm commented Mar 27, 2021 •

edited

Loading

jderusse commented Mar 27, 2021

wouterj commented Mar 27, 2021

Nyholm commented Mar 28, 2021

azjezz commented Mar 28, 2021 •

edited

Loading

Nyholm commented Mar 28, 2021 •

edited

Loading

muglug commented Mar 28, 2021

ro0NL commented Mar 28, 2021

Nyholm commented Mar 28, 2021 •

edited

Loading

wouterj commented Mar 28, 2021 •

edited

Loading

ro0NL commented Mar 28, 2021

nicolas-grekas left a comment

Nyholm commented Apr 2, 2021

jderusse left a comment •

edited by Nyholm

Loading

jderusse Apr 3, 2021

Nyholm commented Apr 3, 2021 •

edited by OskarStark

Loading

Nyholm commented Apr 3, 2021

nicolas-grekas left a comment

nicolas-grekas Apr 11, 2021

Nyholm Apr 12, 2021

nicolas-grekas left a comment

Nyholm commented Apr 12, 2021

nicolas-grekas left a comment

nicolas-grekas commented Apr 13, 2021

Nyholm commented Apr 13, 2021

TomasVotruba commented Dec 21, 2021

Guuzen commented Jan 8, 2022

TomasVotruba commented Jan 8, 2022 •

edited

Loading

[Config][DependencyInjection] Add configuration builder for writing PHP config #40600

[Config][DependencyInjection] Add configuration builder for writing PHP config #40600

Conversation

Nyholm commented Mar 27, 2021 • edited Loading

Before

After

About autogeneration

BC

Other limitations

TODO

Nyholm commented Mar 27, 2021 • edited Loading

jderusse commented Mar 27, 2021

wouterj commented Mar 27, 2021

Nyholm commented Mar 28, 2021

azjezz commented Mar 28, 2021 • edited Loading

Nyholm commented Mar 28, 2021 • edited Loading

muglug commented Mar 28, 2021

ro0NL commented Mar 28, 2021

Nyholm commented Mar 28, 2021 • edited Loading

Correct

Wrong way

wouterj commented Mar 28, 2021 • edited Loading

ro0NL commented Mar 28, 2021

nicolas-grekas left a comment

Choose a reason for hiding this comment

Nyholm commented Apr 2, 2021

jderusse left a comment • edited by Nyholm Loading

Choose a reason for hiding this comment

jderusse Apr 3, 2021

Choose a reason for hiding this comment

Nyholm commented Apr 3, 2021 • edited by OskarStark Loading

Nyholm commented Apr 3, 2021

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas Apr 11, 2021

Choose a reason for hiding this comment

Nyholm Apr 12, 2021

Choose a reason for hiding this comment

nicolas-grekas left a comment

Choose a reason for hiding this comment

Nyholm commented Apr 12, 2021

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas commented Apr 13, 2021

Nyholm commented Apr 13, 2021

TomasVotruba commented Dec 21, 2021

Guuzen commented Jan 8, 2022

TomasVotruba commented Jan 8, 2022 • edited Loading

Nyholm commented Mar 27, 2021 •

edited

Loading

Nyholm commented Mar 27, 2021 •

edited

Loading

azjezz commented Mar 28, 2021 •

edited

Loading

Nyholm commented Mar 28, 2021 •

edited

Loading

Nyholm commented Mar 28, 2021 •

edited

Loading

wouterj commented Mar 28, 2021 •

edited

Loading

jderusse left a comment •

edited by Nyholm

Loading

Nyholm commented Apr 3, 2021 •

edited by OskarStark

Loading

TomasVotruba commented Jan 8, 2022 •

edited

Loading