[Form] Support passing EntityManager instances in "em" option

[geoffrey-brier]

Hi,

We are trying to replace a "entity" field by changing only options in an event subscriber.

So basically what we do is:

$form->add(
    $field->getName(),
    $field->getConfig()->getType()->getName(),
    array_merge($field->getConfig()->getOptions(), array('disabled' => true))
);

The problem is that it seems the ->getOptions() returns the normalized options and not the original ones. Thus the em option is normalized and em is not a string anymore but the concrete entity manager.

This is problematic because then the DoctrineType throws an exception because it only deals with strings or null values:

// Symfony\Bridge\Doctrine\Form\Type\DoctrineType;
$emNormalizer = function (Options $options, $em) use ($registry) {
            /* @var ManagerRegistry $registry */
            if (null !== $em) {
                return $registry->getManager($em); // This is where the exception is thrown
            }
            // ...

I think that the code below should probably fix the problem:

// Symfony\Bridge\Doctrine\Form\Type\DoctrineType;
$emNormalizer = function (Options $options, $em) use ($registry) {
            /* @var ManagerRegistry $registry */
            if (null !== $em) {
                if (is_object($em)) {
                    return $em;
                }
                return $registry->getManager($em);
            }
            // ...

What is your opinion?

[egeloen]

I'm the one who detect the issue and ask @geoffrey-brier to report it. After debugging it, IMO, there is two possibilities to fix it.

1. Allow to pass an entity manager instance as em in all doctrine type and then update the normalizer accordingly. (Not sure if the fact it only allow string actually is intentional)
2. Only allow string or null as em an then, update the normalizer with a is_string check.

[webmozart]

Marked as feature request.

[egeloen]

@webmozart What would be your preference about my two propositions (allow an entity manager instance or only allow string)? I would prefer the first one :)

[webmozart]

Both, no? Doing otherwise would break BC.

[egeloen]

Yeah obviously... When I said the entity manager, I meant the old way (string/null/entity manager) :) I will provide a PR this week-end.

[webmozart]

Cool, thanks! :)