Skip to content

[OptionsResolver] Add prototype definition support for nested options #39913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 1, 2021

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Jan 21, 2021

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

This proposal adds a new method setPrototype(true) to the OptionsResolver component to mark options definition as array prototype:

$this->resolver
    ->setDefault('connections', static function (OptionsResolver $resolver) { // nested option
        $resolver
            ->setPrototype(true) // <- the new method
            ->setRequired('table')
            ->setDefaults(['user' => 'root', 'password' => null]);
    })
;

This feature will allow passing options this way:

$this->resolver->resolve([
    'connections' => [
        'default' => [ // <- the array index "default" is optional and validation free
            'table' => 'default',
        ],
        'custom' => [
            'user' => 'foo',
            'password' => 'pa$$',
            'table' => 'symfony',
        ],
    ],
])

You can add as many items as you want with the advantage of validating each item according to its prototype definition.

The result for this example would be:

[
    'connections' => [
        'default' => [
            'user' => 'root',
            'password' => null,
            'table' => 'default',
        ],
        'custom' => [
            'user' => 'foo',
            'password' => 'pa$$',
            'table' => 'symfony',
        ],
    ],
]

This feature is feasible only for nested options so far and the nested option (e.g. "connections") must be of type array of array.

See the test cases for more details about this feature.

Cheers!

@carsonbot
Copy link

Hey!

I didn't know that was capable of this emotion. I really really like reviewing this PR. Well done.

I think @lmillucci has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@yceruto yceruto force-pushed the options_resolver_prototype branch from 9daae60 to da5ebaf Compare January 28, 2021 20:13
@yceruto
Copy link
Member Author

yceruto commented Jan 28, 2021

There is only one thing left to clarify here, because I'm guessing that prototyping shouldn't be supported as root definition (only for nested options) do we want to throw an exception in case someone tries to do it? I mean, having a prototyped root definition.

@yceruto yceruto force-pushed the options_resolver_prototype branch from da5ebaf to 8b078f4 Compare January 31, 2021 22:07
@azatyan
Copy link
Contributor

azatyan commented Feb 18, 2021

Hi @yceruto hope, you're doing well bro.
In my opinion, yes, adding an exception will help developers to understand faster this feature.

@yceruto yceruto force-pushed the options_resolver_prototype branch 3 times, most recently from 846f42c to 6672a13 Compare February 20, 2021 15:30
@yceruto yceruto added the Ready label Apr 13, 2021
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase on current 5.x?

@yceruto yceruto force-pushed the options_resolver_prototype branch from 6c83da9 to 29d41b1 Compare April 18, 2021 13:38
@yceruto
Copy link
Member Author

yceruto commented Apr 18, 2021

Done.

@Nyholm Nyholm modified the milestones: 5.x, 5.3 Apr 19, 2021
@OskarStark OskarStark requested a review from fabpot April 28, 2021 07:42
@fabpot
Copy link
Member

fabpot commented May 1, 2021

Thank you @yceruto.

@fabpot fabpot merged commit 0489ffc into symfony:5.x May 1, 2021
@fabpot fabpot mentioned this pull request May 1, 2021
@yceruto yceruto deleted the options_resolver_prototype branch May 2, 2021 01:49
javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this pull request May 10, 2021
…method (yceruto)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[OptionsResolver] Documenting new setPrototype() method

PR: symfony/symfony#39913
Fix: symfony#15301

Commits
-------

04316b2 Documenting prototype options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OptionsResolver: validate nested array item structure
6 participants