[5.4] Bring back an old behaviour in resolving controller method dependencies (#18646)

*           bring back the old behaviour while fixing the issue

*    fix style

*        fix test to make more sense

*        fix test to make more sense

*       cover the scenarios with tests

*    fix style

*        add test to cover model bindings

*    fix style

*    clean tests

Author: themsaid

src/Illuminate/Routing/RouteDependencyResolverTrait.php

@@ -37,8 +37,6 @@ protected function resolveClassMethodDependencies(array $parameters, $instance,
      */
     public function resolveMethodDependencies(array $parameters, ReflectionFunctionAbstract $reflector)
     {
-        $results = [];
-
         $instanceCount = 0;
 
         $values = array_values($parameters);
@@ -51,14 +49,14 @@ public function resolveMethodDependencies(array $parameters, ReflectionFunctionA
             if (! is_null($instance)) {
                 $instanceCount++;
 
-                $results[$parameter->getName()] = $instance;
-            } else {
-                $results[$parameter->getName()] = isset($values[$key - $instanceCount])
-                    ? $values[$key - $instanceCount] : $parameter->getDefaultValue();
+                $this->spliceIntoParameters($parameters, $key, $instance);
+            } elseif (! isset($values[$key - $instanceCount]) &&
+                      $parameter->isDefaultValueAvailable()) {
+                $this->spliceIntoParameters($parameters, $key, $parameter->getDefaultValue());
             }
         }
 
-        return $results;
+        return $parameters;
     }
 
     /**
@@ -93,4 +91,19 @@ protected function alreadyInParameters($class, array $parameters)
             return $value instanceof $class;
         }));
     }
+
+    /**
+     * Splice the given value into the parameter list.
+     *
+     * @param  array  $parameters
+     * @param  string  $offset
+     * @param  mixed  $value
+     * @return void
+     */
+    protected function spliceIntoParameters(array &$parameters, $offset, $value)
+    {
+        array_splice(
+            $parameters, $offset, 0, [$value]
+        );
+    }
 }

tests/Routing/RoutingRouteTest.php

@@ -309,24 +309,6 @@ public function testClassesCanBeInjectedIntoRoutes()
         unset($_SERVER['__test.route_inject']);
     }
 
-    public function testClassesAndVariablesCanBeInjectedIntoRoutes()
-    {
-        unset($_SERVER['__test.route_inject']);
-        $router = $this->getRouter();
-        $router->get('foo/{var}/{bar?}/{baz?}', function (stdClass $foo, $var, $bar = 'test', stdClass $baz = null) {
-            $_SERVER['__test.route_inject'] = func_get_args();
-
-            return 'hello';
-        });
-        $this->assertEquals('hello', $router->dispatch(Request::create('foo/bar', 'GET'))->getContent());
-        $this->assertInstanceOf('stdClass', $_SERVER['__test.route_inject'][0]);
-        $this->assertEquals('bar', $_SERVER['__test.route_inject'][1]);
-        $this->assertEquals('test', $_SERVER['__test.route_inject'][2]);
-        $this->assertInstanceOf('stdClass', $_SERVER['__test.route_inject'][3]);
-        $this->assertArrayHasKey(3, $_SERVER['__test.route_inject']);
-        unset($_SERVER['__test.route_inject']);
-    }
-
     public function testOptionsResponsesAreGeneratedByDefault()
     {
         $router = $this->getRouter();
@@ -420,6 +402,56 @@ public function testRouteParametersDefaultValue()
         $this->assertEquals('foo', $router->dispatch(Request::create('foo', 'GET'))->getContent());
     }
 
+    public function testControllerCallActionMethodParameters()
+    {
+        $router = $this->getRouter();
+
+        // Has one argument but receives two
+        unset($_SERVER['__test.controller_callAction_parameters']);
+        $router->get(($str = str_random()).'/{one}/{two}', 'Illuminate\Tests\Routing\RouteTestAnotherControllerWithParameterStub@oneArgument');
+        $router->dispatch(Request::create($str.'/one/two', 'GET'));
+        $this->assertEquals(['one' => 'one', 'two' => 'two'], $_SERVER['__test.controller_callAction_parameters']);
+
+        // Has two arguments and receives two
+        unset($_SERVER['__test.controller_callAction_parameters']);
+        $router->get(($str = str_random()).'/{one}/{two}', 'Illuminate\Tests\Routing\RouteTestAnotherControllerWithParameterStub@twoArguments');
+        $router->dispatch(Request::create($str.'/one/two', 'GET'));
+        $this->assertEquals(['one' => 'one', 'two' => 'two'], $_SERVER['__test.controller_callAction_parameters']);
+
+        // Has two arguments but with different names from the ones passed from the route
+        unset($_SERVER['__test.controller_callAction_parameters']);
+        $router->get(($str = str_random()).'/{one}/{two}', 'Illuminate\Tests\Routing\RouteTestAnotherControllerWithParameterStub@differentArgumentNames');
+        $router->dispatch(Request::create($str.'/one/two', 'GET'));
+        $this->assertEquals(['one' => 'one', 'two' => 'two'], $_SERVER['__test.controller_callAction_parameters']);
+
+        // Has two arguments with same name but argument order is reversed
+        unset($_SERVER['__test.controller_callAction_parameters']);
+        $router->get(($str = str_random()).'/{one}/{two}', 'Illuminate\Tests\Routing\RouteTestAnotherControllerWithParameterStub@reversedArguments');
+        $router->dispatch(Request::create($str.'/one/two', 'GET'));
+        $this->assertEquals(['one' => 'one', 'two' => 'two'], $_SERVER['__test.controller_callAction_parameters']);
+
+        // No route parameters while method has parameters
+        unset($_SERVER['__test.controller_callAction_parameters']);
+        $router->get(($str = str_random()).'', 'Illuminate\Tests\Routing\RouteTestAnotherControllerWithParameterStub@oneArgument');
+        $router->dispatch(Request::create($str, 'GET'));
+        $this->assertEquals([], $_SERVER['__test.controller_callAction_parameters']);
+
+        // With model bindings
+        unset($_SERVER['__test.controller_callAction_parameters']);
+        $router->get(($str = str_random()).'/{user}/{defaultNull?}/{team?}', [
+            'middleware' => SubstituteBindings::class,
+            'uses' => 'Illuminate\Tests\Routing\RouteTestAnotherControllerWithParameterStub@withModels',
+        ]);
+        $router->dispatch(Request::create($str.'/1', 'GET'));
+
+        $values = array_values($_SERVER['__test.controller_callAction_parameters']);
+
+        $this->assertInstanceOf('Illuminate\Http\Request', $values[0]);
+        $this->assertEquals(1, $values[1]->value);
+        $this->assertNull($values[2]);
+        $this->assertInstanceOf('Illuminate\Tests\Routing\RoutingTestTeamModel', $values[3]);
+    }
+
     /**
      * @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
      */
@@ -1293,6 +1325,34 @@ public function returnParameter($bar = '')
     }
 }
 
+class RouteTestAnotherControllerWithParameterStub extends Controller
+{
+    public function callAction($method, $parameters)
+    {
+        $_SERVER['__test.controller_callAction_parameters'] = $parameters;
+    }
+
+    public function oneArgument($one)
+    {
+    }
+
+    public function twoArguments($one, $two)
+    {
+    }
+
+    public function differentArgumentNames($bar, $baz)
+    {
+    }
+
+    public function reversedArguments($two, $one)
+    {
+    }
+
+    public function withModels(Request $request, RoutingTestUserModel $user, $defaultNull = null, RoutingTestTeamModel $team = null)
+    {
+    }
+}
+
 class RouteTestResourceControllerWithModelParameter extends Controller
 {
     public function show(RoutingTestUserModel $fooBar)
@@ -1471,6 +1531,31 @@ public function firstOrFail()
     }
 }
 
+class RoutingTestTeamModel extends Model
+{
+    public function getRouteKeyName()
+    {
+        return 'id';
+    }
+
+    public function where($key, $value)
+    {
+        $this->value = $value;
+
+        return $this;
+    }
+
+    public function first()
+    {
+        return $this;
+    }
+
+    public function firstOrFail()
+    {
+        return $this;
+    }
+}
+
 class RoutingTestExtendedUserModel extends RoutingTestUserModel
 {
 }