I would name this $expectedAttr and move it below the create() call to make the intent of the test clear. Usually, tests should be structured into the following sections (which may in part be missing):

1. Fixture setup
2. Execution of the test
3. Test verification
4. Cleanup

The create() statement belongs to 2, while $attr ($expectedAttr) is already part of the test verification. Hence I'd move it below.

Understood, thanks, I'm fixing it.

this is broken in case the callable is an array (something like array($this, 'buildEntryOptions'))

@stof, thanks I'll fix it and add a test for this.

btw, please add a test using such a callable to prevent regressions in its support

ok looks like I got your tother comments with delay :) woking on it.

same issue here

I just realize that there is a big issue with this solution. Above, in preSetData(), the method getOptionsForEntry() receives the data in the model format. For example, if we have a collection of Tag instances, a Tag instance would be passed.

Here, however, we pass the submitted data - a string (the tag name) or an array (multiple tag properties) - which are yet to be transformed back into a Tag instance. Consequently, the callback needs to be aware of either format and dynamically handle both. That won't be very intuitive nor easy to use in practice.

Right, I did not see that. So I need to add a test on submit and I think I can handle this.

How would you handle this?

It's funny because I was thinking about it last night and I wondered how was the prototype_data option handled though, and I realised while remembering your comment that you can only define the view data of a prototype. The CollectionType doc reference should be clear about it, especially if you need to transform value.

And just this morning, I see this ticket #17992...

Since there is no problem as long as the underlying data is a string value, it may not be working for an object underlying data.

I see an easy solution but it's not "pretty", it should be mentioned in the docs that the entry_options option as callable must test the string type :

'entry_options' => function ($entry) {
    $options = array(
        // set default
    );

    if (is_string($entry) {
        // $entry <=> $viewData
        // or $entry <=> $modelData <=> $viewData
        $options['global_dynamic'] = // use $entry as string
    }

    // Else entry is an instanceof `data_class` or null
    $options['model_dynamic'] = // use $entry as model data or null

    return $options;
},

But it would be better to solve this in a feature way, by adding for example an add_entry option (which may have its counterpart in a future allow_add'able ChoiceType as add_choice).

This option would be a callable getting passed the new view data as only argument and should return a new model data. Almost like empty_data but this last one is only called for an empty view data and takes the form as first argument.

Then we would be able to use add_entry call on the new view data before passing it to getOptionsForEntry.

Another solution is to try to fix this while refactoring CollectionType more globally thanks to a CollectionList support, since I'm already investigating this way.
I think this would allow to solve many old issues with this type, with at first dynamic indexing, and polymorphism.

What do you think ?

Of courses this is more a DX concern as one could use DataMapperInterface for that, but I'm not sure it's working while the new fields are added on PRE_SUBMIT event.

I'm going to add a test for it anyway, as for the behavior of prototype_data and empty_data on submit to avoid regressions such as #17992.

I keep investigating, I think this feature can be useful and doable even if it needs a more global refactoring of CollectionType.

It may not be finished for 3.1 though, but I'll try to work on that this week-end.

@HeahDude even with a refactoring, we will still need to add elements based on 2 different formats, as one case wants to add fields when knowing the model data and the other case when knowing the view data (and this cannot change with any refactoring, as it is part of the basic about how a collection type should work).

Btw, one of your previous comment assumes that the view data is a string. this assumption is wrong. you can perfectly have collections of objects (and then a compound form type as the entry type)

I agree with don't misunderstood me, I would assume default view data is string when not null, or when compound an array or object.

In fact, I mentioned that ChoiceType is particular in this sense since it is always compound (whether it can be a select input with no children) and will always have a string view data.
That what I would like to ensure decoupling the data transformers from the list mapper in #18180.

In fact I think expanded ChoiceType is not far from the CollectionType and I was thinking about decoupling the form list creation with a factory interface as #14050 did for ChoiceType to allow dealing with polymorphic and custom indexed/named collection.

and dynamic entry options.

[...] gets passed [...] as the only [...]