feature #22527 [DI] Throw useful exception on bad XML argument tags (nicolas-grekas)

fabpot · fabpot · commit 8872833c5d6a · 2017-04-29T09:33:57.000-07:00
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Throw useful exception on bad XML argument tags | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22525 | License | MIT | Doc PR | - I still think that the feature request in #22525 would make things better. But at least, let's make thing fail loudly, instead of silently today, with the associated usual wtfs :) Commits ------- 91828ec [DI] Throw useful exception on bad XML argument tags
diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
@@ -50,7 +50,7 @@ public function load($resource, $type = null)
         $this->parseImports($xml, $path);
 
         // parameters
-        $this->parseParameters($xml);
+        $this->parseParameters($xml, $path);
 
         // extensions
         $this->loadFromExtensions($xml);
@@ -83,11 +83,12 @@ public function supports($resource, $type = null)
      * Parses parameters.
      *
      * @param \DOMDocument $xml
+     * @param string       $file
      */
-    private function parseParameters(\DOMDocument $xml)
+    private function parseParameters(\DOMDocument $xml, $file)
     {
         if ($parameters = $this->getChildren($xml->documentElement, 'parameters')) {
-            $this->container->getParameterBag()->add($this->getArgumentsAsPhp($parameters[0], 'parameter'));
+            $this->container->getParameterBag()->add($this->getArgumentsAsPhp($parameters[0], 'parameter', $file));
         }
     }
 
@@ -278,8 +279,8 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
             $definition->setDeprecated(true, $deprecated[0]->nodeValue ?: null);
         }
 
-        $definition->setArguments($this->getArgumentsAsPhp($service, 'argument', false, $definition instanceof ChildDefinition));
-        $definition->setProperties($this->getArgumentsAsPhp($service, 'property'));
+        $definition->setArguments($this->getArgumentsAsPhp($service, 'argument', $file, false, $definition instanceof ChildDefinition));
+        $definition->setProperties($this->getArgumentsAsPhp($service, 'property', $file));
 
         if ($factories = $this->getChildren($service, 'factory')) {
             $factory = $factories[0];
@@ -312,7 +313,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
         }
 
         foreach ($this->getChildren($service, 'call') as $call) {
-            $definition->addMethodCall($call->getAttribute('method'), $this->getArgumentsAsPhp($call, 'argument'));
+            $definition->addMethodCall($call->getAttribute('method'), $this->getArgumentsAsPhp($call, 'argument', $file));
         }
 
         $tags = $this->getChildren($service, 'tag');
@@ -446,11 +447,12 @@ private function processAnonymousServices(\DOMDocument $xml, $file)
      *
      * @param \DOMElement $node
      * @param string      $name
+     * @param string      $file
      * @param bool        $lowercase
      *
      * @return mixed
      */
-    private function getArgumentsAsPhp(\DOMElement $node, $name, $lowercase = true, $isChildDefinition = false)
+    private function getArgumentsAsPhp(\DOMElement $node, $name, $file, $lowercase = true, $isChildDefinition = false)
     {
         $arguments = array();
         foreach ($this->getChildren($node, $name) as $arg) {
@@ -486,6 +488,9 @@ private function getArgumentsAsPhp(\DOMElement $node, $name, $lowercase = true,
 
             switch ($arg->getAttribute('type')) {
                 case 'service':
+                    if (!$arg->getAttribute('id')) {
+                        throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="service" has no or empty "id" attribute in "%s".', $name, $file));
+                    }
                     if ($arg->hasAttribute('strict')) {
                         @trigger_error(sprintf('The "strict" attribute used when referencing the "%s" service is deprecated since version 3.3 and will be removed in 4.0.', $arg->getAttribute('id')), E_USER_DEPRECATED);
                     }
@@ -496,17 +501,23 @@ private function getArgumentsAsPhp(\DOMElement $node, $name, $lowercase = true,
                     $arguments[$key] = new Expression($arg->nodeValue);
                     break;
                 case 'closure-proxy':
+                    if (!$arg->getAttribute('id')) {
+                        throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="closure-proxy" has no or empty "id" attribute in "%s".', $name, $file));
+                    }
+                    if (!$arg->getAttribute('method')) {
+                        throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="closure-proxy" has no or empty "method" attribute in "%s".', $name, $file));
+                    }
                     $arguments[$key] = new ClosureProxyArgument($arg->getAttribute('id'), $arg->getAttribute('method'), $invalidBehavior);
                     break;
                 case 'collection':
-                    $arguments[$key] = $this->getArgumentsAsPhp($arg, $name, false);
+                    $arguments[$key] = $this->getArgumentsAsPhp($arg, $name, $file, false);
                     break;
                 case 'iterator':
-                    $arg = $this->getArgumentsAsPhp($arg, $name, false);
+                    $arg = $this->getArgumentsAsPhp($arg, $name, $file, false);
                     try {
                         $arguments[$key] = new IteratorArgument($arg);
                     } catch (InvalidArgumentException $e) {
-                        throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="iterator" only accepts collections of type="service" references.', $name));
+                        throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="iterator" only accepts collections of type="service" references in "%s".', $name, $file));
                     }
                     break;
                 case 'string':
