[8.x] Give a more meaningul message when route parameters are missing (…

…#35706) * Give a more meaningul message when route parameters are missing The message for `UrlGenerationException` when parameters are missing is a nice start, but knowing which parameters are missing can be a huge help in figuring out where something is broken. Especially if a route has multiple possible parameters. Which parameter is actually missing? Are more than one missing? Now we'll know! * Fix small typo Co-authored-by: Michel Bardelmeijer <m.bardelmeijer@gmail.com> Co-authored-by: Michel Bardelmeijer <m.bardelmeijer@gmail.com>
laravel · Jan 1, 2021 · 4da7a2b · 4da7a2b
1 parent 2c9c12c
commit 4da7a2b
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 5 deletions.
diff --git a/src/Illuminate/Routing/Exceptions/UrlGenerationException.php b/src/Illuminate/Routing/Exceptions/UrlGenerationException.php
@@ -3,17 +3,35 @@
 namespace Illuminate\Routing\Exceptions;
 
 use Exception;
+use Illuminate\Routing\Route;
+use Illuminate\Support\Str;
 
 class UrlGenerationException extends Exception
 {
     /**
      * Create a new exception for missing route parameters.
      *
      * @param  \Illuminate\Routing\Route  $route
+     * @param array $parameters
      * @return static
      */
-    public static function forMissingParameters($route)
+    public static function forMissingParameters(Route $route, array $parameters = [])
     {
-        return new static("Missing required parameters for [Route: {$route->getName()}] [URI: {$route->uri()}].");
+        $parameterLabel = Str::plural('parameter', count($parameters));
+
+        $message = sprintf(
+            'Missing required %s for [Route: %s] [URI: %s]',
+            $parameterLabel,
+            $route->getName(),
+            $route->uri()
+        );
+
+        if (count($parameters) > 0) {
+            $message .= sprintf(' [Missing %s: %s]', $parameterLabel, implode(', ', $parameters));
+        }
+
+        $message .= '.';
+
+        return new static($message);
     }
 }
diff --git a/src/Illuminate/Routing/RouteUrlGenerator.php b/src/Illuminate/Routing/RouteUrlGenerator.php
@@ -87,8 +87,8 @@ public function to($route, $parameters = [], $absolute = false)
             $route
         ), $parameters);
 
-        if (preg_match('/\{.*?\}/', $uri)) {
-            throw UrlGenerationException::forMissingParameters($route);
+        if (preg_match_all('/{(.*?)}/', $uri, $matchedMissingParameters)) {
+            throw UrlGenerationException::forMissingParameters($route, $matchedMissingParameters[1]);
         }
 
         // Once we have ensured that there are no missing parameters in the URI we will encode

diff --git a/tests/Routing/RoutingRouteTest.php b/tests/Routing/RoutingRouteTest.php
@@ -1831,7 +1831,7 @@ public function testRouteRedirectStripsMissingStartingForwardSlash()
     public function testRouteRedirectExceptionWhenMissingExpectedParameters()
     {
         $this->expectException(UrlGenerationException::class);
-        $this->expectExceptionMessage('Missing required parameters for [Route: laravel_route_redirect_destination] [URI: users/{user}].');
+        $this->expectExceptionMessage('Missing required parameter for [Route: laravel_route_redirect_destination] [URI: users/{user}] [Missing parameter: user].');
 
         $container = new Container;
         $router = new Router(new Dispatcher, $container);

diff --git a/tests/Routing/RoutingUrlGeneratorTest.php b/tests/Routing/RoutingUrlGeneratorTest.php
@@ -550,6 +550,61 @@ public function testUrlGenerationForControllersRequiresPassingOfRequiredParamete
         $this->assertSame('http://www.foo.com:8080/foo?test=123', $url->route('foo', $parameters));
     }
 
+    public function provideParametersAndExpectedMeaningfulExceptionMessages()
+    {
+        return [
+            'Missing parameters "one", "two" and "three"' => [
+                [],
+                'Missing required parameters for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameters: one, two, three].',
+            ],
+            'Missing parameters "two" and "three"' => [
+                ['one' => '123'],
+                'Missing required parameters for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameters: two, three].',
+            ],
+            'Missing parameters "one" and "three"' => [
+                ['two' => '123'],
+                'Missing required parameters for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameters: one, three].',
+            ],
+            'Missing parameters "one" and "two"' => [
+                ['three' => '123'],
+                'Missing required parameters for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameters: one, two].',
+            ],
+            'Missing parameter "three"' => [
+                ['one' => '123', 'two' => '123'],
+                'Missing required parameter for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameter: three].',
+            ],
+            'Missing parameter "two"' => [
+                ['one' => '123', 'three' => '123'],
+                'Missing required parameter for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameter: two].',
+            ],
+            'Missing parameter "one"' => [
+                ['two' => '123', 'three' => '123'],
+                'Missing required parameter for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameter: one].',
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideParametersAndExpectedMeaningfulExceptionMessages
+     */
+    public function testUrlGenerationThrowsExceptionForMissingParametersWithMeaningfulMessage($parameters, $expectedMeaningfulExceptionMessage)
+    {
+        $this->expectException(UrlGenerationException::class);
+        $this->expectExceptionMessage($expectedMeaningfulExceptionMessage);
+
+        $url = new UrlGenerator(
+            $routes = new RouteCollection,
+            Request::create('http://www.foo.com:8080/')
+        );
+
+        $route = new Route(['GET'], 'foo/{one}/{two}/{three}/{four?}', ['as' => 'foo', function () {
+            //
+        }]);
+        $routes->add($route);
+
+        $url->route('foo', $parameters);
+    }
+
     public function testForceRootUrl()
     {
         $url = new UrlGenerator(