From e244d31fb0041eff4b47ac9dbb6db6378f424d52 Mon Sep 17 00:00:00 2001
From: Nicolas Grekas <nicolas.grekas@gmail.com>
Date: Wed, 12 May 2021 12:22:14 +0200
Subject: [PATCH] [FrameworkBundle] improve AbstractController::renderForm()

---
 .../Bundle/FrameworkBundle/CHANGELOG.md       | 10 +++---
 .../Controller/AbstractController.php         | 33 +++++++++++++------
 .../Controller/AbstractControllerTest.php     |  8 ++---
 3 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
index 6e0c4b971b..4d67ff130d 100644
--- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
+++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
@@ -7,11 +7,11 @@ CHANGELOG
  * Deprecate the `session.storage` alias and `session.storage.*` services, use the `session.storage.factory` alias and `session.storage.factory.*` services instead
  * Deprecate the `framework.session.storage_id` configuration option, use the `framework.session.storage_factory_id` configuration option instead
  * Deprecate the `session` service and the `SessionInterface` alias, use the `Request::getSession()` or the new `RequestStack::getSession()` methods instead
- * Added `AbstractController::renderForm()` to render a form and set the appropriate HTTP status code
- * Added support for configuring PHP error level to log levels
- * Added the `dispatcher` option to `debug:event-dispatcher`
- * Added the `event_dispatcher.dispatcher` tag
- * Added `assertResponseFormatSame()` in `BrowserKitAssertionsTrait`
+ * Add `AbstractController::renderForm()` to render a form and set the appropriate HTTP status code
+ * Add support for configuring PHP error level to log levels
+ * Add the `dispatcher` option to `debug:event-dispatcher`
+ * Add the `event_dispatcher.dispatcher` tag
+ * Add `assertResponseFormatSame()` in `BrowserKitAssertionsTrait`
  * Add support for configuring UUID factory services
  * Add tag `assets.package` to register asset packages
  * Add support to use a PSR-6 compatible cache for Doctrine annotations
diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php
index 43682654e7..3ddd8b185c 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php
@@ -20,6 +20,7 @@ use Symfony\Component\Form\Extension\Core\Type\FormType;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\Form\FormFactoryInterface;
 use Symfony\Component\Form\FormInterface;
+use Symfony\Component\Form\FormView;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
 use Symfony\Component\HttpFoundation\Exception\SessionNotFoundException;
 use Symfony\Component\HttpFoundation\JsonResponse;
@@ -267,21 +268,33 @@ abstract class AbstractController implements ServiceSubscriberInterface
     }
 
     /**
-     * Renders a view for a form.
+     * Renders a view and sets the appropriate status code when a form is listed in parameters.
      *
-     * The FormView instance is passed to the template in a variable named
-     * "form" (can be changed via $formVar argument).
-     * If the form is invalid, a 422 status code is returned.
+     * If an invalid form is found in the list of parameters, a 422 status code is returned.
      */
-    protected function renderForm(string $view, FormInterface $form, array $parameters = [], Response $response = null, string $formVar = 'form'): Response
+    protected function renderForm(string $view, array $parameters = [], Response $response = null): Response
     {
-        $response = $this->render($view, [$formVar => $form->createView()] + $parameters, $response);
-
-        if ($form->isSubmitted() && !$form->isValid()) {
-            $response->setStatusCode(422);
+        if (null === $response) {
+            $response = new Response();
         }
 
-        return $response;
+        foreach ($parameters as $k => $v) {
+            if ($v instanceof FormView) {
+                throw new \LogicException(sprintf('Passing a FormView to "%s::renderForm()" is not supported, pass directly the form instead for parameter "%s".', get_debug_type($this), $k));
+            }
+
+            if (!$v instanceof FormInterface) {
+                continue;
+            }
+
+            $parameters[$k] = $v->createView();
+
+            if (200 === $response->getStatusCode() && $v->isSubmitted() && !$v->isValid()) {
+                $response->setStatusCode(422);
+            }
+        }
+
+        return $this->render($view, $parameters, $response);
     }
 
     /**
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php
index 97b31bf28b..46fab2bcda 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php
@@ -420,7 +420,7 @@ class AbstractControllerTest extends TestCase
         $form->expects($this->once())->method('createView')->willReturn($formView);
 
         $twig = $this->getMockBuilder(Environment::class)->disableOriginalConstructor()->getMock();
-        $twig->expects($this->once())->method('render')->with('foo', ['form' => $formView, 'bar' => 'bar'])->willReturn('bar');
+        $twig->expects($this->once())->method('render')->with('foo', ['bar' => $formView])->willReturn('bar');
 
         $container = new Container();
         $container->set('twig', $twig);
@@ -428,7 +428,7 @@ class AbstractControllerTest extends TestCase
         $controller = $this->createController();
         $controller->setContainer($container);
 
-        $response = $controller->renderForm('foo', $form, ['bar' => 'bar']);
+        $response = $controller->renderForm('foo', ['bar' => $form]);
 
         $this->assertTrue($response->isSuccessful());
         $this->assertSame('bar', $response->getContent());
@@ -444,7 +444,7 @@ class AbstractControllerTest extends TestCase
         $form->expects($this->once())->method('isValid')->willReturn(false);
 
         $twig = $this->getMockBuilder(Environment::class)->disableOriginalConstructor()->getMock();
-        $twig->expects($this->once())->method('render')->with('foo', ['myForm' => $formView, 'bar' => 'bar'])->willReturn('bar');
+        $twig->expects($this->once())->method('render')->with('foo', ['bar' => $formView])->willReturn('bar');
 
         $container = new Container();
         $container->set('twig', $twig);
@@ -452,7 +452,7 @@ class AbstractControllerTest extends TestCase
         $controller = $this->createController();
         $controller->setContainer($container);
 
-        $response = $controller->renderForm('foo', $form, ['bar' => 'bar'], null, 'myForm');
+        $response = $controller->renderForm('foo', ['bar' => $form]);
 
         $this->assertSame(422, $response->getStatusCode());
         $this->assertSame('bar', $response->getContent());