Merge pull request #1747 from nelmio/arrayitemserror

Improve error when the items type of an array is not specified

Exception/UndocumentedArrayItemsException.php

@@ -0,0 +1,42 @@
+<?php
+
+/*
+ * This file is part of the NelmioApiDocBundle package.
+ *
+ * (c) Nelmio
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\ApiDocBundle\Exception;
+
+class UndocumentedArrayItemsException extends \LogicException
+{
+    private $class;
+    private $path;
+
+    public function __construct(string $class = null, string $path = '')
+    {
+        $this->class = $class;
+        $this->path = $path;
+
+        $propertyName = '';
+        if (null !== $class) {
+            $propertyName = $class.'::';
+        }
+        $propertyName .= $path;
+
+        parent::__construct(sprintf('Property "%s" is an array, but its items type isn\'t specified. You can specify that by using the type `string[]` for instance or `@SWG\Property(type="array", @SWG\Items(type="string"))`.', $propertyName));
+    }
+
+    public function getClass()
+    {
+        return $this->class;
+    }
+
+    public function getPath()
+    {
+        return $this->path;
+    }
+}

ModelDescriber/ObjectModelDescriber.php

@@ -15,6 +15,7 @@ use Doctrine\Common\Annotations\Reader;
 use EXSyst\Component\Swagger\Schema;
 use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareInterface;
 use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareTrait;
+use Nelmio\ApiDocBundle\Exception\UndocumentedArrayItemsException;
 use Nelmio\ApiDocBundle\Model\Model;
 use Nelmio\ApiDocBundle\ModelDescriber\Annotations\AnnotationsReader;
 use Nelmio\ApiDocBundle\PropertyDescriber\PropertyDescriberInterface;
@@ -144,7 +145,15 @@ class ObjectModelDescriber implements ModelDescriberInterface, ModelRegistryAwar
                 $propertyDescriber->setModelRegistry($this->modelRegistry);
             }
             if ($propertyDescriber->supports($type)) {
-                $propertyDescriber->describe($type, $property, $model->getGroups());
+                try {
+                    $propertyDescriber->describe($type, $property, $model->getGroups());
+                } catch (UndocumentedArrayItemsException $e) {
+                    if (null !== $e->getClass()) {
+                        throw $e; // This exception is already complete
+                    }
+
+                    throw new UndocumentedArrayItemsException($model->getType()->getClassName(), sprintf('%s%s', $propertyName, $e->getPath()));
+                }
 
                 return;
             }

PropertyDescriber/ArrayPropertyDescriber.php

@@ -14,6 +14,7 @@ namespace Nelmio\ApiDocBundle\PropertyDescriber;
 use EXSyst\Component\Swagger\Schema;
 use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareInterface;
 use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareTrait;
+use Nelmio\ApiDocBundle\Exception\UndocumentedArrayItemsException;
 use Symfony\Component\PropertyInfo\Type;
 
 class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistryAwareInterface
@@ -32,7 +33,7 @@ class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistr
     {
         $type = $type->getCollectionValueType();
         if (null === $type) {
-            throw new \LogicException(sprintf('Property "%s" is an array, but its items type isn\'t specified. You can specify that by using the type `string[]` for instance or `@SWG\Property(type="array", @SWG\Items(type="string"))`.', $property->getTitle()));
+            throw new UndocumentedArrayItemsException();
         }
 
         $property->setType('array');
@@ -43,7 +44,15 @@ class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistr
                 $propertyDescriber->setModelRegistry($this->modelRegistry);
             }
             if ($propertyDescriber->supports($type)) {
-                $propertyDescriber->describe($type, $property, $groups);
+                try {
+                    $propertyDescriber->describe($type, $property, $groups);
+                } catch (UndocumentedArrayItemsException $e) {
+                    if (null !== $e->getClass()) {
+                        throw $e; // This exception is already complete
+                    }
+
+                    throw new UndocumentedArrayItemsException(null, sprintf('%s[]', $e->getPath()));
+                }
 
                 break;
             }

Tests/Functional/ArrayItemsErrorTest.php

@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * This file is part of the NelmioApiDocBundle package.
+ *
+ * (c) Nelmio
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\ApiDocBundle\Tests\Functional;
+
+use Nelmio\ApiDocBundle\Exception\UndocumentedArrayItemsException;
+
+class ArrayItemsErrorTest extends WebTestCase
+{
+    protected function setUp()
+    {
+        parent::setUp();
+
+        static::createClient([], ['HTTP_HOST' => 'api.example.com']);
+    }
+
+    public function testModelPictureDocumentation()
+    {
+        $this->expectException(UndocumentedArrayItemsException::class);
+        $this->expectExceptionMessage('Property "Nelmio\ApiDocBundle\Tests\Functional\Entity\ArrayItemsError\Bar::things[]" is an array, but its items type isn\'t specified.');
+
+        $this->getSwaggerDefinition();
+    }
+
+    protected static function createKernel(array $options = [])
+    {
+        return new TestKernel(TestKernel::ERROR_ARRAY_ITEMS);
+    }
+}

Tests/Functional/BazingaFunctionalTest.php

@@ -122,6 +122,6 @@ class BazingaFunctionalTest extends WebTestCase
 
     protected static function createKernel(array $options = [])
     {
-        return new TestKernel(true, true);
+        return new TestKernel(TestKernel::USE_JMS | TestKernel::USE_BAZINGA);
     }
 }

Tests/Functional/Controller/ArrayItemsErrorController.php

@@ -0,0 +1,35 @@
+<?php
+
+/*
+ * This file is part of the NelmioApiDocBundle package.
+ *
+ * (c) Nelmio
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\ApiDocBundle\Tests\Functional\Controller;
+
+use Nelmio\ApiDocBundle\Annotation\Model;
+use Nelmio\ApiDocBundle\Tests\Functional\Entity\ArrayItemsError\Foo;
+use Swagger\Annotations as SWG;
+use Symfony\Component\Routing\Annotation\Route;
+
+/**
+ * @Route(host="api.example.com")
+ */
+class ArrayItemsErrorController
+{
+    /**
+     * @Route("/api/error", methods={"GET"})
+     * @SWG\Response(
+     *     response=200,
+     *     description="Success",
+     *     @Model(type=Foo::class)
+     * )
+     */
+    public function errorAction()
+    {
+    }
+}

Tests/Functional/Entity/ArrayItemsError/Bar.php

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the NelmioApiDocBundle package.
+ *
+ * (c) Nelmio
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\ApiDocBundle\Tests\Functional\Entity\ArrayItemsError;
+
+/**
+ * @author Guilhem N. <guilhem@gniot.fr>
+ */
+class Bar
+{
+    public $things;
+
+    public function addThing(array $thing) { }
+}

Tests/Functional/Entity/ArrayItemsError/Foo.php

@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of the NelmioApiDocBundle package.
+ *
+ * (c) Nelmio
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\ApiDocBundle\Tests\Functional\Entity\ArrayItemsError;
+
+/**
+ * @author Guilhem N. <guilhem@gniot.fr>
+ */
+class Foo
+{
+    /**
+     * @var string
+     */
+    public $articles;
+
+    /**
+     * @var Bar[]
+     */
+    public $bars;
+}

Tests/Functional/JMSFunctionalTest.php

@@ -304,6 +304,6 @@ class JMSFunctionalTest extends WebTestCase
 
     protected static function createKernel(array $options = [])
     {
-        return new TestKernel(true);
+        return new TestKernel(TestKernel::USE_JMS);
     }
 }

Tests/Functional/TestKernel.php

@@ -33,17 +33,19 @@ use Symfony\Component\Serializer\Annotation\SerializedName;
 
 class TestKernel extends Kernel
 {
+    const USE_JMS = 1;
+    const USE_BAZINGA = 2;
+    const ERROR_ARRAY_ITEMS = 4;
+
     use MicroKernelTrait;
 
-    private $useJMS;
+    private $flags;
-    private $useBazinga;
 
-    public function __construct(bool $useJMS = false, bool $useBazinga = false)
+    public function __construct(int $flags = 0)
     {
-        parent::__construct('test'.(int) $useJMS.(int) $useBazinga, true);
+        parent::__construct('test'.$flags, true);
 
-        $this->useJMS = $useJMS;
+        $this->flags = $flags;
-        $this->useBazinga = $useBazinga;
     }
 
     /**
@@ -61,10 +63,10 @@ class TestKernel extends Kernel
             new FOSRestBundle(),
         ];
 
-        if ($this->useJMS) {
+        if ($this->flags & self::USE_JMS) {
             $bundles[] = new JMSSerializerBundle();
 
-            if ($this->useBazinga) {
+            if ($this->flags & self::USE_BAZINGA) {
                 $bundles[] = new BazingaHateoasBundle();
             }
         }
@@ -91,11 +93,11 @@ class TestKernel extends Kernel
             $routes->import(__DIR__.'/Controller/SerializedNameController.php', '/', 'annotation');
         }
 
-        if ($this->useJMS) {
+        if ($this->flags & self::USE_JMS) {
             $routes->import(__DIR__.'/Controller/JMSController.php', '/', 'annotation');
         }
 
-        if ($this->useBazinga) {
+        if ($this->flags & self::USE_BAZINGA) {
             $routes->import(__DIR__.'/Controller/BazingaController.php', '/', 'annotation');
 
             try {
@@ -104,6 +106,10 @@ class TestKernel extends Kernel
             } catch (\ReflectionException $e) {
             }
         }
+
+        if ($this->flags & self::ERROR_ARRAY_ITEMS) {
+            $routes->import(__DIR__.'/Controller/ArrayItemsErrorController.php', '/', 'annotation');
+        }
     }
 
     /**
@@ -231,7 +237,7 @@ class TestKernel extends Kernel
      */
     public function getCacheDir()
     {
-        return parent::getCacheDir().'/'.(int) $this->useJMS;
+        return parent::getCacheDir().'/'.$this->flags;
     }
 
     /**
@@ -239,7 +245,7 @@ class TestKernel extends Kernel
      */
     public function getLogDir()
     {
-        return parent::getLogDir().'/'.(int) $this->useJMS;
+        return parent::getLogDir().'/'.$this->flags;
     }
 
     public function serialize()