[FrameworkBundle] Generate Config class
#58771
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
280475c to
1f9c0b0
Compare
src/Symfony/Component/Config/Builder/ArrayShapeGener
8000
ator.php
Outdated
Show resolved
Hide resolved
|
Thank you for your review! I'm having a look at it and will soon address your comments |
1f9c0b0 to
1d3af0c
Compare
|
Prototyped arrays are now supported, here's a screenshot from the newly generated config (snippet): /* @param array{
* access_denied_url?: string|int|float|bool,
* session_fixation_strategy?: "none"|"migrate"|"invalidate",
* hide_user_not_found?: bool,
* erase_credentials?: bool,
* access_decision_manager?: array{
* strategy?: "affirmative"|"consensus"|"unanimous"|"priority",
* service?: string|int|float|bool,
* strategy_service?: string|int|float|bool,
* allow_if_all_abstain?: bool,
* allow_if_equal_granted_denied?: bool,
* },
* password_hashers?: array<array{
* algorithm?: string|int|float|bool,
* migrate_from?: array<string|int|float|bool>,
* hash_algorithm?: string|int|float|bool,
* key_length?: string|int|float|bool,
* ignore_case?: bool,
* encode_as_base64?: bool,
* iterations?: string|int|float|bool,
* cost?: int<4, 31>,
* memory_cost?: string|int|float|bool,
* time_cost?: string|int|float|bool,
* id?: string|int|float|bool,
* }>,
* providers?: array<array{
* id?: string|int|float|bool,
* chain?: array{
* providers?: array<string|int|float|bool>,
* },
* memory?: array{
* users?: array<array{
* password?: string|int|float|bool,
* roles?: array<string|int|float|bool>,
* }>,
* },
* ldap?: array{
* service: string|int|float|bool,
* base_dn: string|int|float|bool,
* search_dn?: string|int|float|bool,
* search_password?: string|int|float|bool,
* extra_fields?: array<string|int|float|bool>,
* default_roles?: array<string|int|float|bool>,
* uid_key?: string|int|float|bool,
* filter?: string|int|float|bool,
* password_attribute?: string|int|float|bool,
* },
* }>,
* ...
*/Providers, password hashers, etc. are all prototypes. |
1d3af0c to
81124f2
Compare
src/Symfony/Component/Config/Builder/ConfigBuilderGenerator.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Config/Builder/ConfigFunctionGenerator.php
Outdated
Show resolved
Hide resolved
81124f2 to
0cd82fd
Compare
0cd82fd to
e84099f
Compare
|
To anyone willing to see improved support for this updated PHP config format, please upvote this phpstorm issue ;) |
e84099f to
35abd04
Compare
src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ConfigBuilderCacheWarmer.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
This is missing pre-filled shapes for "services", "imports" and "parameters" tree, which are not defined by a bundle, so don't have a configuration definition.
Also, can you investigate how an IDE like vscode could add support for auto-completing shapes? who's in charge of the most used vscode extensions so that we can ping them?
|
I'm also wondering if using named arguments for the 1st level provides the best DX: the IDE cannot know we want to type the name of an argument quickly enough I fear. Whereas if we start typing |
|
I like the named argument way of doing because it is a bit more convenient to Cmd/Ctrl+Click on the named argument to have the full doc at a glance, whereas the Cmd/Ctrl+Click isn't available for array indexes. If there are a lot of bundles enabled, being able to go to the "definition" of the named argument could really help |
35abd04 to
4a674ab
Compare
|
You can now enjoy comments in PHPDoc types as part of PHPStan 2.1.6 and phpstan/phpdoc-parser 2.1.0 🥳 |
|
Great news, thank you @ondrejmirtes for being that responsive on this request! |
edf409a to
91086d0
Compare
src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ConfigBuilderCacheWarmer.php
Show resolved
Hide resolved
214e266 to
4a8be2c
Compare
4a8be2c to
1e0f160
Compare
|
Comments in array shapes are live in the latest PhpStorm 2025.1 Beta:
|
|
I'm shaking again this config format to find the best syntax. I'm wondering about how one could define several arrays in the same file. We do so currently when some settings depend on the env - with What about using a config object with as many methods as needed, and no return value? <?php
/** @var \Symfony\Config\Config $config */;
$config->framework([...]);
if ('dev' === $config->env) {
$config->framework([...]);
}If we want to keep splitting shapes in separate files (to ease scrolling the shape?) we might generate traits and use them in the generated Config class. WDYT? |
|
Suits me fine, I particularly like the trait part. I definitely think it would be way easier to scroll through shapes if we have
I agree, just a suggestion on the last bit. What about actually returning $config
->framework([...])
->webProfiler([...])
->security([...]); |
…bois) This PR was merged into the 7.3 branch. Discussion ---------- [Config] Add `NodeDefinition::docUrl()` | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT Adding such information would allow extensions and bundles to provide even more info with a documentation "one click away". The primary goal is to use this feature in conjunction with #58771, allowing to dump a ``@see` https://symfony.com/doc/...` right next to the configuration array shape. Commits ------- 3c7fce2 [Config] Add `NodeDefinition::docUrl()`
…bois) This PR was merged into the 7.3 branch. Discussion ---------- [Config] Add `NodeDefinition::docUrl()` | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT Adding such information would allow extensions and bundles to provide even more info with a documentation "one click away". The primary goal is to use this feature in conjunction with symfony/symfony#58771, allowing to dump a ``@see` https://symfony.com/doc/...` right next to the configuration array shape. Commits ------- 3c7fce2e326 [Config] Add `NodeDefinition::docUrl()`
…bois) This PR was merged into the 7.3 branch. Discussion ---------- [Config] Add `NodeDefinition::docUrl()` | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT Adding such information would allow extensions and bundles to provide even more info with a documentation "one click away". The primary goal is to use this feature in conjunction with symfony/symfony#58771, allowing to dump a ``@see` https://symfony.com/doc/...` right next to the configuration array shape. Commits ------- 3c7fce2e326 [Config] Add `NodeDefinition::docUrl()`
…bois) This PR was merged into the 7.3 branch. Discussion ---------- [Config] Add `NodeDefinition::docUrl()` | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT Adding such information would allow extensions and bundles to provide even more info with a documentation "one click away". The primary goal is to use this feature in conjunction with symfony/symfony#58771, allowing to dump a ``@see` https://symfony.com/doc/...` right next to the configuration array shape. Commits ------- 3c7fce2e326 [Config] Add `NodeDefinition::docUrl()`
|
JetBrains did their part, thanks to them! Coming to PhpStorm 2025.1
Why not indeed. Up to resume work on this topic? |
|
It's a bit of a rush at the moment, but as soon as I have a bit of time to get on with the job, this PR is my priority. 🙂 |
43b5e13 to
e394d68
Compare
Config class
e394d68 to
bf2754c
Compare
|
I just pushed the revamped version. PR description updated with the new syntax. After playing with, navigating through the shapes is much simpler now that traits are generated. |
bf2754c to
a371101
Compare
|
|
||
| private static function generateInlinePhpDocForNode(BaseNode $node): string | ||
| { | ||
| $hasContent = false; |
There was a problem hiding this comment.
needless var: $comment should start empty and be prefixed only at the end, if not empty anymore
also, CR/LF in $comment should be replaced by a space so that we're sure it remains a oneliner
There was a problem hiding this comment.
It currently outputs something like this for framework:
* strict_requirements?: string|int|float|bool, // set to true to throw an exception when a parameter does not match the requirements set to false to disable exceptions when a parameter does not match the requirements (and return null instead) set to null to disable parameter checks against requirements 'true' is the preferred configuration in development mode, while 'false' or 'null' might be preferred in production Default: true
Maybe we could instead replace \r and \n with ; ?
There was a problem hiding this comment.
Actually maybe not, it seems it's a particular case. Other places end the info with a ., so maybe we should adjust the info of this particular node.
| // the closure forbids access to the private scope in the included file | ||
| $load = \Closure::bind(function ($path, $env) use ($container, $loader, $resource, $type) { | ||
| $config = class_exists(Config::class) ? new Config() : null; |
There was a problem hiding this comment.
Why does the generated Config class have constructor arguments when those are never used?
Also, it'd be nice to add public readonly vars to the $config object to get access to the other tools one could use, especially $config->env (possibly $config->path, container, loader, resource, type, although I'm not sure what they could be used for there - for thoughts.)
There was a problem hiding this comment.
Updated, it now looks like this:
<?php
namespace Symfony\Config;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
use Symfony\Config\FrameworkConfig;
use Symfony\Config\SecurityConfig;
use Symfony\Config\TwigConfig;
use Symfony\Config\WebProfilerConfig;
use Symfony\Config\MonologConfig;
use Symfony\Config\DebugConfig;
/**
* This class is automatically generated to help in creating a config.
*/
class Config
{
use \Symfony\Component\DependencyInjection\Resource\ImportsTrait;
use \Symfony\Component\DependencyInjection\Resource\ParametersTrait;
use \Symfony\Component\DependencyInjection\Resource\ServicesTrait;
use \Symfony\Config\FrameworkTrait;
use \Symfony\Config\SecurityTrait;
use \Symfony\Config\TwigTrait;
use \Symfony\Config\WebProfilerTrait;
use \Symfony\Config\MonologTrait;
use \Symfony\Config\DebugTrait;
private $builders;
private $frameworkConfig;
private $securityConfig;
private $twigConfig;
private $webProfilerConfig;
private $monologConfig;
private $debugConfig;
public function __construct(public readonly ?string $env)
{
$this->frameworkConfig = new FrameworkConfig();
$this->securityConfig = new SecurityConfig();
$this->twigConfig = new TwigConfig();
$this->webProfilerConfig = new WebProfilerConfig();
$this->monologConfig = new MonologConfig();
$this->debugConfig = new DebugConfig();
$this->builders = [$this->frameworkConfig, $this->securityConfig, $this->twigConfig, $this->webProfilerConfig, $this->monologConfig, $this->debugConfig];
}
public function getBuilders(): array
{
return $this->builders;
}
}$env is injected. It's always possible to add other ones if needed, $env being the most important to me as well
| private array $parameters = []; | ||
|
|
||
| /** | ||
| * @var array<string, mixed> $parameters |
There was a problem hiding this comment.
Narrowed the type a bit to array<string, array|bool|string|int|float|\UnitEnum|null>
| * @var array<array{ | ||
| * id: string, | ||
| * class?: string|int|float|bool, | ||
| * }>|array<string, class-string|null> $services |
There was a problem hiding this comment.
This is minimalistic. In reality, we could be way more descriptive here, see what YamlFileLoader describes for services. Maybe you preferred postponing this to a follow up?
There was a problem hiding this comment.
I started implementing what's left (lazy, abstract, args, synthetic, etc.) but there's a bit more work than I expected. I propose to validate the PR as-is, then comeback to this bit when the rest is OK?
a371101 to
2990a68
Compare
This PR adds a new
Configclass to configure a Symfony application:One method exists for each extension, as well as
services(),imports()andparameters().The class is generated at cache warmup, allowing to generate the corresponding phpdoc. Here a sneak peek of what the functions looks like:
This will bring nice IDE autocomplete as well as strong static analysis. Also, you're always one click away from the complete documentation.
Credits to Nicolas, Kevin and Ryan for the idea 🙂