[RFC] Deprecate end() in config fluent interface #26351

ro0NL · 2018-03-01T12:33:03Z

Q	A
Bug report?	no
Feature request?	yes
BC Break report?	no
RFC?	yes
Symfony version	4.x

I believe this was already proposed once when building the DI fluent config format. Moreover, i think it's possible in a way.. relying on something like what's currently also being proposed for DI config: #25650

However for Config component it would be required to do it that way, as without end() we dont know the scope of the node definition (meaning based on indentation things could either apply to a child- or parent node).

Overall, i think it could be a cool step. Aiming for a bit more simplicity as well as better support for custom node types in the long run.

Im thinking something along the lines of:

<?php
$rootNode
    ->children(function (YourNodeBuilder $builder): void {
        $builder
            ->scalarNode('foo')
                ->isRequired()
            ->booleanNode('bar')
                ->cannotBeEmpty();
    })
    ->validate()->always(function ($value) {
        // ...
    });

Thoughts?

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

ro0NL · 2018-03-01T12:46:44Z

Some more suggestion from my side:

consider allowEmpty() over cannotBeEmpty() (the no. of configs i saw forgetting this is too high).
default features handled in VariableNodeDefinition/ArrayNodeDefinition::createNode() should be shared by NodeDefinition::create/prepareNode(), ideally :)
[Config] Introduce BuilderAwareInterface #26308 :)

stof · 2018-03-01T12:53:44Z

I'm not sure the usage of nested closures to configure nested hierarchies is actually that much easier to use.

ro0NL · 2018-03-01T13:03:42Z

Yes, it looks a bit more cluttered. Then again, it makes the current node scope also look more explicit IMHO 🤔

Eventually what convinced me to do the RFC is it could solve autocomplete/static analysis issues, considering custom node types (which is what im getting into currently :)). Depending on the use case i really started to like having those.

Currently if you do a custom NodeBuilder (to provide myNode(...) you need to override all end() definitions in order to let config know about the builder and always providing autocompletion.

But now thinking about it.. im not sure closures / magic __call or so solve that 🤔

ro0NL · 2018-03-01T13:14:31Z

So basically this is about:

@return NodeParentInterface|NodeBuilder|NodeDefinition|ArrayNodeDefinition|VariableNodeDefinition|null

on end(); mother of mixed API :)

null is a pain to deal with in terms of static analysis
my custom node builder is missing from the list, as well as my custom node type
technically, all the calls work =/

stof · 2018-03-01T13:15:42Z

well, normal nodes would still not have a defining myNode method, so it would either need to rely on the generic node method (which cannot tell the IDE that passing my as second argument returns a MyNodeDefinition object) or using __call to allow using myNode(), which is even worse as NodeBuilder would not even be able to tell your IDE that it has a magic myNode method (as it does not have it until you register your custom node type).

So your proposal would not solve anything for custom node types (as all methods in Symfony would still document NodeBuilder, not your custom child class)

stof · 2018-03-01T13:16:53Z

btw, your example is wrong. The argument passed to the children callback would not be an ArrayNodeDefinition

ro0NL · 2018-03-01T13:30:48Z

^ correct. You'd get a builder :) just wrote that quickly.

So your proposal would not solve anything for custom node types (as all methods in Symfony would still document NodeBuilder, not your custom child class)

I think you're right. It bugs me a bit though :) as technically it's working as expected/designed. But today that's not enough without proper autocompletion/static analysis IMHO. Defeating a useful thing...

So ... put different; can we think of something clever?

fabpot · 2018-03-02T13:23:57Z

Yes, I can think of something clever: let's not change anything. I mean, I don't see why we should change the way it works today. I've never seem anybody complaining about the current syntax. And changing it to something means breaking all configs out there (at the minimum, people would have to remove all their end() calls at some point).

ro0NL · 2018-03-02T14:19:40Z

symfony/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

Lines 369 to 374 in 1f7b9f0

would be solved with closures. Today i'd prefer that.. but i understand making the change now is not a top prio.

if something could solve SA/autocompletion that be nice. With either end() or closures... but it's not possible AFAIK.

Sorry for the noise :)

javiereguiluz added Config RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Mar 1, 2018

ro0NL closed this as completed Mar 2, 2018

ragboyjr mentioned this issue Dec 27, 2019

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Deprecate end() in config fluent interface #26351

[RFC] Deprecate end() in config fluent interface #26351