From 7c696827752b0003728686a705495824a1dee18a Mon Sep 17 00:00:00 2001
From: Nicolas Grekas <nicolas.grekas@gmail.com>
Date: Wed, 12 May 2021 07:26:19 +0200
Subject: [PATCH] [FrameworkBundle] improve AbstractController::handleForm()

---
 .../Controller/AbstractController.php         | 23 +++++++++-----
 .../Controller/AbstractControllerTest.php     | 31 +++++++++++++++----
 2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php
index b0c4f16acd..ff20fbeb65 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php
@@ -297,23 +297,30 @@ abstract class AbstractController implements ServiceSubscriberInterface
      * * if the form is submitted but invalid, $render is called and a 422 HTTP status code is set if the current status hasn't been customized
      * * if the form is submitted and valid, $onSuccess is called, usually this method saves the data and returns a 303 HTTP redirection
      *
-     * @param callable(FormInterface, mixed): Response $onSuccess
-     * @param callable(FormInterface, mixed): Response $render
+     * For both callables, instead of "mixed", you can use your form's data class as a type-hint for argument #2.
+     *
+     * @param callable(FormInterface, mixed, Request): Response $onSuccess
+     * @param callable(FormInterface, mixed, Request): Response $render
      */
     public function handleForm(FormInterface $form, Request $request, callable $onSuccess, callable $render): Response
     {
         $form->handleRequest($request);
 
         $submitted = $form->isSubmitted();
-
         $data = $form->getData();
-        if ($submitted && $form->isValid()) {
-            return $onSuccess($form, $data);
+
+        if ($isValid = $submitted && $form->isValid()) {
+            $response = $onSuccess($form, $data, $request);
+        } else {
+            $response = $render($form, $data, $request);
+
+            if ($submitted && 200 === $response->getStatusCode()) {
+                $response->setStatusCode(Response::HTTP_UNPROCESSABLE_ENTITY);
+            }
         }
 
-        $response = $render($form, $data);
-        if ($submitted && 200 === $response->getStatusCode()) {
-            $response->setStatusCode(Response::HTTP_UNPROCESSABLE_ENTITY);
+        if (!$response instanceof Response) {
+            throw new \TypeError(sprintf('The "%s" callable passed to "%s::handleForm()" must return a Response, "%s" returned.', $isValid ? '$onSuccess' : '$render', get_debug_type($this), get_debug_type($response)));
         }
 
         return $response;
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php
index ca5c029ea0..c659f7e35e 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php
@@ -433,10 +433,10 @@ class AbstractControllerTest extends TestCase
         $response = $controller->handleForm(
             $form,
             Request::create('https://example.com'),
-            function (FormInterface $form, $data): Response {
+            function (FormInterface $form, $data, Request $request): Response {
                 return new RedirectResponse('https://example.com/redir', Response::HTTP_SEE_OTHER);
             },
-            function (FormInterface $form, $data): Response {
+            function (FormInterface $form, $data, Request $request): Response {
                 return new Response('rendered');
             }
         );
@@ -455,10 +455,10 @@ class AbstractControllerTest extends TestCase
         $response = $controller->handleForm(
             $form,
             Request::create('https://example.com'),
-            function (FormInterface $form): Response {
+            function (FormInterface $form, $data, Request $request): Response {
                 return new RedirectResponse('https://example.com/redir', Response::HTTP_SEE_OTHER);
             },
-            function (FormInterface $form): Response {
+            function (FormInterface $form, $data, Request $request): Response {
                 return new Response('rendered');
             }
         );
@@ -477,10 +477,10 @@ class AbstractControllerTest extends TestCase
         $response = $controller->handleForm(
             $form,
             Request::create('https://example.com'),
-            function (FormInterface $form): Response {
+            function (FormInterface $form, $data, Request $request): Response {
                 return new RedirectResponse('https://example.com/redir', Response::HTTP_SEE_OTHER);
             },
-            function (FormInterface $form): Response {
+            function (FormInterface $form, $data, Request $request): Response {
                 return new Response('rendered');
             }
         );
@@ -490,6 +490,25 @@ class AbstractControllerTest extends TestCase
         $this->assertSame('https://example.com/redir', $response->getTargetUrl());
     }
 
+    public function testHandleFormTypeError()
+    {
+        $form = $this->createMock(FormInterface::class);
+        $form->expects($this->once())->method('isSubmitted')->willReturn(true);
+        $form->expects($this->once())->method('isValid')->willReturn(true);
+
+        $controller = $this->createController();
+
+        $this->expectException(\TypeError::class);
+        $this->expectExceptionMessage('The "$onSuccess" callable passed to "Symfony\Bundle\FrameworkBundle\Tests\Controller\TestAbstractController::handleForm()" must return a Response, "string" returned.');
+
+        $response = $controller->handleForm(
+            $form,
+            Request::create('https://example.com'),
+            function () { return 'abc'; },
+            function () { return 'abc'; }
+        );
+    }
+
     public function testRedirectToRoute()
     {
         $router = $this->createMock(RouterInterface::class);