bug symfony#34223 [DI] Suggest typed argument when binding fails with untyped argument (gudfar)

nicolas-grekas · nicolas-grekas · commit 477e843790b2 · 2020-01-14T17:43:06.000+01:00
This PR was merged into the 4.3 branch. Discussion ---------- [DI] Suggest typed argument when binding fails with untyped argument | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | symfony#33470 | License | MIT I've added a condition that looks for arguments and if the typehint doesn’t match, throws an `InvalidArgumentException` Commits ------- 0e92399 [DI] Suggest typed argument when binding fails with untyped argument
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php
@@ -112,6 +112,8 @@ protected function processValue($value, $isRoot = false)
             return parent::processValue($value, $isRoot);
         }
 
+        $bindingNames = [];
+
         foreach ($bindings as $key => $binding) {
             list($bindingValue, $bindingId, $used, $bindingType, $file) = $binding->getValues();
             if ($used) {
@@ -121,7 +123,11 @@ protected function processValue($value, $isRoot = false)
                 $this->unusedBindings[$bindingId] = [$key, $this->currentId, $bindingType, $file];
             }
 
-            if (preg_match('/^(?:(?:array|bool|float|int|string) )?\$/', $key)) {
+            if (preg_match('/^(?:(?:array|bool|float|int|string|([^ $]++)) )\$/', $key, $m)) {
+                $bindingNames[substr($key, \strlen($m[0]))] = $binding;
+            }
+
+            if (!isset($m[1])) {
                 continue;
             }
 
@@ -182,11 +188,17 @@ protected function processValue($value, $isRoot = false)
                     continue;
                 }
 
-                if (!$typeHint || '\\' !== $typeHint[0] || !isset($bindings[$typeHint = substr($typeHint, 1)])) {
+                if ($typeHint && '\\' === $typeHint[0] && isset($bindings[$typeHint = substr($typeHint, 1)])) {
+                    $arguments[$key] = $this->getBindingValue($bindings[$typeHint]);
+
                     continue;
                 }
 
-                $arguments[$key] = $this->getBindingValue($bindings[$typeHint]);
+                if (isset($bindingNames[$parameter->name])) {
+                    $bindingKey = array_search($binding, $bindings, true);
+                    $argumentType = substr($bindingKey, 0, strpos($bindingKey, ' '));
+                    $this->errorMessages[] = sprintf('Did you forget to add the type "%s" to argument "$%s" of method "%s::%s()"?', $argumentType, $parameter->name, $reflectionMethod->class, $reflectionMethod->name);
+                }
             }
 
             if ($arguments !== $call[1]) {
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php
@@ -18,6 +18,7 @@
 use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy;
@@ -157,4 +158,19 @@ public function testSyntheticServiceWithBind()
 
         $this->assertSame([1 => 'bar'], $container->getDefinition(NamedArgumentsDummy::class)->getArguments());
     }
+
+    public function testEmptyBindingTypehint()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Did you forget to add the type "string" to argument "$apiKey" of method "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy::__construct()"?');
+
+        $container = new ContainerBuilder();
+        $bindings = [
+            'string $apiKey' => new BoundArgument('foo'),
+        ];
+        $definition = $container->register(NamedArgumentsDummy::class, NamedArgumentsDummy::class);
+        $definition->setBindings($bindings);
+        $pass = new ResolveBindingsPass();
+        $pass->process($container);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -112,6 +112,8 @@ protected function processValue($value, $isRoot = false)`
`112`	`112`	`return parent::processValue($value, $isRoot);`
`113`	`113`	`}`
`114`	`114`
	`115`	`+ $bindingNames = [];`
	`116`	`+`
`115`	`117`	`foreach ($bindings as $key => $binding) {`
`116`	`118`	`list($bindingValue, $bindingId, $used, $bindingType, $file) = $binding->getValues();`
`117`	`119`	`if ($used) {`
`@@ -121,7 +123,11 @@ protected function processValue($value, $isRoot = false)`
`121`	`123`	`$this->unusedBindings[$bindingId] = [$key, $this->currentId, $bindingType, $file];`
`122`	`124`	`}`
`123`	`125`
`124`		`- if (preg_match('/^(?:(?:array\|bool\|float\|int\|string) )?\$/', $key)) {`
	`126`	`+ if (preg_match('/^(?:(?:array\|bool\|float\|int\|string\|([^ $]++)) )\$/', $key, $m)) {`
	`127`	`+ $bindingNames[substr($key, \strlen($m[0]))] = $binding;`
	`128`	`+ }`
	`129`	`+`
	`130`	`+ if (!isset($m[1])) {`
`125`	`131`	`continue;`
`126`	`132`	`}`
`127`	`133`
`@@ -182,11 +188,17 @@ protected function processValue($value, $isRoot = false)`
`182`	`188`	`continue;`
`183`	`189`	`}`
`184`	`190`
`185`		`- if (!$typeHint \|\| '\\' !== $typeHint[0] \|\| !isset($bindings[$typeHint = substr($typeHint, 1)])) {`
	`191`	`+ if ($typeHint && '\\' === $typeHint[0] && isset($bindings[$typeHint = substr($typeHint, 1)])) {`
	`192`	`+ $arguments[$key] = $this->getBindingValue($bindings[$typeHint]);`
	`193`	`+`
`186`	`194`	`continue;`
`187`	`195`	`}`
`188`	`196`
`189`		`- $arguments[$key] = $this->getBindingValue($bindings[$typeHint]);`
	`197`	`+ if (isset($bindingNames[$parameter->name])) {`
	`198`	`+ $bindingKey = array_search($binding, $bindings, true);`
	`199`	`+ $argumentType = substr($bindingKey, 0, strpos($bindingKey, ' '));`
	`200`	`+ $this->errorMessages[] = sprintf('Did you forget to add the type "%s" to argument "$%s" of method "%s::%s()"?', $argumentType, $parameter->name, $reflectionMethod->class, $reflectionMethod->name);`
	`201`	`+ }`
`190`	`202`	`}`
`191`	`203`
`192`	`204`	`if ($arguments !== $call[1]) {`