[RFC][Config] Declarative Syntax for Config Tree Builder #35127

ragboyjr · 2019-12-27T21:41:37Z

Description

Defining configuration trees can be a bit cumbersome and syntactically verbose, and in some non-intuitive for some combinations of configuration.

Let's look a seemingly simple configuration file:

my_package:
  string_key: 'abc'
  int_key: 1
  dict_key: 
    a: 1
    b: 2
  list_key: [1, 2, 3]
  list_of_dict_key:
    - a: 1
      b: 2
  dict_of_list:
    a: ['', '']
    b: [0, 0]

To define this with the current tree builder syntax, it would look like:

Current Syntax

$treeBuilder = new TreeBuilder();
$treeBuilder->root('my_package')
    ->children()
        ->scalarNode('string_key')->end()
        ->integerNode('int_key')->end()
        ->arrayNode('dict_key')
            ->children()
                ->scalarNode('a')->end()
                ->integerNode('b')->end()
            ->end()
        ->end()
        ->arrayNode('list_key')
            ->integerPrototype()->end()
        ->end()
        ->arrayNode('list_of_dict_key')
            ->arrayPrototype()
                ->children()
                    ->integerNode('a')->end()
                    ->integerNode('b')->end()
                ->end()
            ->end()
        ->end()
        ->arrayNode('dict_of_list_key')
            ->children()
                ->arrayNode('a')
                    ->scalarPrototype()->end()
                ->end()
                ->arrayNode('b')
                    ->integerPrototype()->end()
                ->end()
            ->end()
        ->end()
    ->end();
return $treeBuilder;

Syntax aside, I still find myself struggling to write the proper code for validation (even with an IDE) because it's very easy to miss an end() call which then messes up downstream definitions, and this can cascade if you have large config trees.

I think this is a great example where using a more declarative composition-friendly interface would make declaring configuration schemas easier to maintain, learn, and understand.

Declarative Syntax

return configTree('my_package', dict([
    'string_key' => string(),
    'int_key' => int(),
    'dict_key' => dict([
        'a' => int(),
        'b' => int(),
    ]),
    'list_key' => listOf(int()),
    'list_of_dict_key' => listOf(dict([
        'a' => int(),
        'b' => int(),
    ])),
    'dict_of_list_key' => dict([
        'a' => listOf(string()),
        'b' => listOf(int()),
    ])
]));

IMHO, this syntax is much easier to read and understand the structure of what we are configuring and is less error prone to missing an end statement.

It also is pretty easy to allow access to the actual node definitions via the configure method like:

dict([])->configure(fn(ArrayNodeDefinition $node) => $node->canBeEnabled()->ignoreExtraKeys(false));

Rough Implementation

The following code is a working implementation for this declarative syntax, the formatting is rough, but if this RFC is received well, I could clean this up into a pull request.

abstract class ConfigNode {
    protected $attributes;

    public function __construct(array $attributes = []) {
        $this->attributes = $attributes;
    }

    public function attribute(string $key) {
        return $this->attributes[$key] ?? null;
    }

    public function attributes(): array {
        return $this->attributes;
    }

    public function withAttributes(array $attributes): self {
        $self = clone $this;
        $self->attributes = $attributes;
        return $self;
    }

    public function withAddedAttributes(array $addedAttributes): self {
        return $this->withAttributes(array_merge($this->attributes, $addedAttributes));
    }

    public function name(): ?string {
        return $this->attribute('name');
    }

    public function configure(callable $configure): self {
        return $this->withAddedAttributes(['configure' => $configure]);
    }

    public function optionallyConfigure($node): void {
        $configure = $this->attribute('configure');
        if ($configure) {
            $configure($node);
        }
    }
}
final class DictNode extends ConfigNode {
    /** @return ConfigNode[] */
    public function nodesByName(): array {
        return $this->attributes['nodesByName'] ?? [];
    }
}
final class ListOfNode extends ConfigNode {
    public function node(): ?ConfigNode {
        return $this->attribute('node');
    }
}
final class StringNode extends ConfigNode {}
final class IntNode extends ConfigNode {}
final class FloatNode extends ConfigNode {}
final class ScalarNode extends ConfigNode {}
final class BoolNode extends ConfigNode {}

function configTree(string $rootName, DictNode $dictNode): TreeBuilder {
    $treeBuilder = new TreeBuilder();
    configureNode($treeBuilder->root($rootName), $dictNode);
    return $treeBuilder;
}

function configureNode($node, ConfigNode $configNode): void {
    if ($configNode instanceof DictNode) {
        if ($node instanceof ArrayNodeDefinition) {
            $resNode = $node;
        } else if ($node instanceof NodeBuilder) {
            $resNode = $node->arrayNode($configNode->name());
        } else {
            throw new \RuntimeException('Unexpected dict node type.');
        }

        /** @var ArrayNodeDefinition $node */
        foreach ($configNode->nodesByName() as $name => $childConfigNode) {
            configureNode($resNode->children(), $childConfigNode->withAddedAttributes(['name' => $name]));
        }
    } else if ($configNode instanceof StringNode || $configNode instanceof ScalarNode) {
        /** @var NodeBuilder $node */
        $resNode = $node->scalarNode($configNode->name());
    } else if ($configNode instanceof IntNode) {
        /** @var NodeBuilder $node */
        $resNode = $node->integerNode($configNode->name());
    } else if ($configNode instanceof FloatNode) {
        /** @var NodeBuilder $node */
        $resNode = $node->floatNode($configNode->name());
    } else if ($configNode instanceof BoolNode) {
        /** @var NodeBuilder $node */
        $resNode = $node->booleanNode($configNode->name());
    } else if ($configNode instanceof ListOfNode) {
        if ($node instanceof ArrayNodeDefinition) {
            $resNode = $node;
        } else if ($node instanceof NodeBuilder) {
            $resNode = $node->arrayNode($configNode->name());
        } else {
            throw new \RuntimeException('Unexpected dict node type.');
        }
        configureArrayNode($resNode, $configNode->node());
    } else {
        throw new \RuntimeException('Unhandled node types.');
    }

    $configNode->optionallyConfigure($resNode);
}

function configureArrayNode(ArrayNodeDefinition $arrayNode, ConfigNode $configNode): void {
    if ($configNode instanceof DictNode) {
        configureNode($arrayNode->arrayPrototype(), $configNode);
    } else if ($configNode instanceof StringNode || $configNode instanceof ScalarNode) {
        $arrayNode->scalarPrototype();
    } else if ($configNode instanceof IntNode) {
        $arrayNode->integerPrototype();
    } else if ($configNode instanceof FloatNode) {
        $arrayNode->floatPrototype();
    } else if ($configNode instanceof BoolNode) {
        $arrayNode->booleanPrototype();
    } else if ($configNode instanceof ListOfNode) {
        configureNode($arrayNode->arrayPrototype(), $configNode);
    } else {
        throw new \RuntimeException('Unhandled node types.');
    }
}

/** @param ConfigNode[] $nodesByName */
function dict(array $nodesByName): DictNode { return new DictNode(['nodesByName' => $nodesByName]); }
function tuple(ConfigNode ...$nodes): DictNode { return dict($nodes); }
function listOf(ConfigNode $node): ListOfNode { return new ListOfNode(['node' => $node]); }
function string(): StringNode { return new StringNode(); }
function int(): IntNode { return new IntNode(); }
function float(): FloatNode { return new FloatNode(); }
function scalar(): ScalarNode { return new ScalarNode(); }
function bool(): BoolNode { return new BoolNode(); }

Related Merge Requests

[RFC] Deprecate end() in config fluent interface #26351 It seems this was an attempt to reduce some of the syntactic noise with defining configuration trees.

The text was updated successfully, but these errors were encountered:

sstok · 2019-12-29T10:01:41Z

->scalarNode('string_key')->end() This indeed very verbose, and technically it's not really a node 😋

What about:

->scalarLeave('string_key', function (ScalarNodeDefinition $node)) { } ) ?

But keeping the end() call for configuration that is actual Node, like an Array.

jkufner · 2019-12-29T11:37:03Z

The use of array in "Declarative Syntax" is a tempting but very dangerous way. IDE cannot help you and a larger configuration tree is difficult to maintain.

The use of the configure method with lambda parameter is much better approach if proper typehints are provided. IDE can assist you, it is easy to split the definition into multiple methods to reduce nesting levels in the source code, and PHP checks the syntax on compilation.

TomasVotruba · 2019-12-29T22:53:23Z

Some inspiration from @nette https://github.com/nette/schema#defining-schema

Used in neon (~= YAML) like this: https://github.com/phpstan/phpstan-src/blob/35047b54c1d2435fe2e16488bf5a5164f313e123/conf/config.neon#L106-L192

nicolas-grekas · 2019-12-30T09:37:27Z

I'm pretty sure this can be done independently from the component, by implementing LoaderInterface.
That's what we do in DI and Routing to provide their PHP-DSL.
I'd suggest experimenting this as a standalone package, that could be considered for merging later on, once "proven".

jkufner · 2019-12-30T12:17:23Z

Another potential inspiration is JSON Schema. Just like XML Schema, it is a JSON document that defines allowed structure of JSON documents.

jkufner · 2019-12-30T12:36:35Z

Some inspiration from @nette https://github.com/nette/schema#defining-schema

@TomasVotruba The static builder seems like a good idea. However, I wonder what are the benefits in comparison with an instantiated builder, i.e., like $sb = new SchemaBuilder(); $sb->something()->add($sb->scalar()); instead of Expect::something()->add(Expect::scalar());?

TomasVotruba · 2019-12-30T13:43:31Z

It's rather about idea, not static/normal.

< 8000 form class="js-comment-update" id="issuecomment-569682363-edit-form" data-turbo="false" action="/symfony/symfony/issue_comments/569682363" accept-charset="UTF-8" method="post">

ragboyjr · 2019-12-30T16:15:24Z

@nicolas-grekas I definitely agree this can be a standalone package. I'm not fully sure where the LoaderInterface comes into play because this is for configuring a TreeBuilder in the Configuration. But your input is very appreciated.

@sstok

->scalarLeave('string_key', function (ScalarNodeDefinition $node)) { } )

This still feels fairly verbose when compared to the declarative syntax. But removing the end for sure would help with usability, but I believe it could make the implementation much more difficult.

@TomasVotruba Ah, that nette package looks very neat, and definitely the same vibes that I'm going for. My gut tells me that if this has any chance of ever making it into the core one day, then I'd likely need to ensure that i'm not using any external packages.

@jkufner There still is decent IDE support with the declarative syntax. It's not like Laravel validations where everything is string based. The only piece that is string based are the dictionary keys, but those are currently also defined as strings in the current tree builder as well.

Also, when it comes to static method calls vs global methods vs a builder object, I'm not sure any of those have any real benefits over the others, so whichever is least verbose and maximized discovery is what I think we should choose.

If you think about it, this isn't much different than symfony's validation api. You compose validations that are apart of the Validations namespace and then validate data against the one validation. The only real difference is that I'm creating global helpers to make the code a bit more terse.

TomasVotruba · 2019-12-30T16:21:31Z

then I'd likely need to ensure that i'm not using any external package

Agreed. It's just for inspiration, aka copy-paste, not to use it as a dependency

ragboyjr · 2020-03-22T16:32:10Z

Oomph, this took a bit for me to get around to this, but just built out this library to address the above: https://github.com/krakphp/schema

Would appreciate some feedback, it's in a very early state.

stof · 2020-03-23T10:32:31Z

@ragboyjr note that in all your examples, your usage of dict is really confusing, because you use it to configure a struct, not a dictionary (which is also called a map in many languages), as you have known keys.

ragboyjr · 2020-03-23T12:51:49Z

hmm, it’s interesting though because php arrays really can only be a list or dict. I’m not sure known keys is the only difference of strict vs dict. I feel like fixed keys is more the deciding difference.

Im not validating objects with fixed keys but rather arrays that act as dicts (or maps). But either way, I feel dicts and structs are also fairly similar in their nature of key value pairs, so using dict or struct would have been equally understandable from my pov.

Honest question: what initially were you thinking dict was doing before you made the connection that it was like struct?

stof · 2020-03-23T13:45:02Z

for the processing of the config, that makes a big difference. That's why the config component can configure an array node either with ->children() (struct mode, with pre-defined keys with each their type) or prototype() (where keys are user-defined, with all the same type)

ragboyjr · 2020-03-23T14:17:08Z

ah, ok, this makes more sense where you're coming from. I took children vs prototype to be dict vs list, but the reality of it is that it's really struct vs dict because the config module doesn't care if you use int based keys or string based keys.

Very helpful, will update, thanks for the feedback. I'll probably just introduce the struct keyword, but leave dict where struct = heterogenous values +fixed keys, dict = homogenous values, + unfixed keys, list = homogenous values, index based keys.

stof · 2020-03-23T14:21:40Z

prototype() can be either a list or a map, depending on how you configure it (you need to call useAttributeAsKey to make it a map rather than a list, the name of the method coming from how this appears in XML files where the key of the map is actually in an attribute because XSD does not like unknown tag names)

stof · 2020-03-23T14:22:48Z

And the Config component cares about lists vs maps when merging config from several files (forgetting useAttributeAsKey will make the string keys being lost as soon as merging is necessary).

carsonbot · 2021-02-19T13:02:16Z

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

carsonbot · 2021-03-05T13:34:29Z

Friendly ping? Should this still be open? I will close if I don't hear anything.

carsonbot · 2021-03-19T13:39:28Z

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

chalasr added Config RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Dec 28, 2019

carsonbot added the Stalled label Feb 19, 2021

carsonbot closed this as completed Mar 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[RFC][Config] Declarative Syntax for Config Tree Builder #35127

[RFC][Config] Declarative Syntax for Config Tree Builder #35127

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!