[Config] Allow validating values using callables in ExprBuilder #19578

chalasr · 2016-08-09T12:42:55Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	yes
Deprecations?	no
Tests pass?	yes
Fixed tickets	n/a
License	MIT
Doc PR	n/a

This PR is a proposal for adding the ability to pass any callable for validating values using the ExprBuilder, which accepts only closures ATM.

The problem with closures is they often involve to write redundant code such as useless assignments or intermediate methods or lots of arguments in the use() part of the closure, as soon as the validation logic is decoupled from the configuration for instance.

Before

public function getConfigTreeBuilder()
{
    // ...
    $validator = $this->validator;
    $rootNode
        ->children()
            ->scalarNode('foo')
                ->validate()
                ->ifString()
                    ->then(function ($v) use ($validator) { return $validator->validate($v); })
                ->end()
            ->end()
        ->end()
    ->end()
}

After

    // ...
    $rootNode
        ->children()
            ->scalarNode('foo')
                ->validate()
                ->ifString()
                    ->then([$this->validator, 'validate'])
                ->end()
            ->end()
        ->end()
    ->end()

It is a BC break but IMHO it worths it, ATM I don't see any use case where this would have a negative impact, except when overriding the changed methods (never saw such case).

nicolas-grekas · 2016-08-09T12:58:42Z

As you spotted, it's a BC break, forbidden by our written policy.
Btw, on PHP 7.1, you could write:
->then(\Closure::fromCallable([$this->validator, 'validate'])) :)
and on PHP 5.4 you can already write:
->then(function ($v) {return $this->validator->validate($v);})

👎

ro0NL · 2016-08-09T13:58:17Z

Somewhere it is a shame this cant target 4.x :( the change itself is good imho.

Allow validating values using callable

773b4df

carsonbot added Status: Needs Review Config Feature BC Break labels Aug 9, 2016

chalasr closed this Aug 9, 2016

chalasr deleted the expr_builder_callable branch August 9, 2016 13:50

ro0NL mentioned this pull request Aug 24, 2016

[HttpFoundation] Add support for DateTimeImmutable in setters #19727

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Config] Allow validating values using callables in ExprBuilder #19578

[Config] Allow validating values using callables in ExprBuilder #19578

chalasr commented Aug 9, 2016

nicolas-grekas commented Aug 9, 2016 •

edited

Loading

ro0NL commented Aug 9, 2016

[Config] Allow validating values using callables in ExprBuilder #19578

[Config] Allow validating values using callables in ExprBuilder #19578

Conversation

chalasr commented Aug 9, 2016

nicolas-grekas commented Aug 9, 2016 • edited Loading

ro0NL commented Aug 9, 2016

nicolas-grekas commented Aug 9, 2016 •

edited

Loading