From c43171895161c8eb342bc5fc5eb21760dd91b646 Mon Sep 17 00:00:00 2001
From: Djordy Koert <djordy.koert@gmail.com>
Date: Fri, 8 Nov 2024 16:00:51 +0100
Subject: [PATCH] refactor: deprecate `options` possibly being null (#2387)

## Description

Deprecates `options` nullable type for `Model` classes. This reduces
complexity and streamlines it with the `$serializationContext`.

## What type of PR is this? (check all applicable)
- [ ] Bug Fix
- [ ] Feature
- [x] Refactor
- [x] Deprecation
- [ ] Breaking Change
- [ ] Documentation Update
- [ ] CI

## Checklist
- [ ] I have made corresponding changes to the documentation (`docs/`)
- [x] I have made corresponding changes to the changelog
(`CHANGELOG.md`)
---
 CHANGELOG.md                                  | 19 ++++
 phpunit-ignore.txt                            |  6 +-
 phpunit.xml.dist                              |  1 +
 src/Attribute/Model.php                       | 11 ++-
 src/Model/Model.php                           | 15 ++--
 src/Model/ModelRegistry.php                   |  2 +-
 src/ModelDescriber/FormModelDescriber.php     |  2 +-
 src/ModelDescriber/JMSModelDescriber.php      |  2 +-
 .../ObjectPropertyDescriber.php               |  4 +-
 .../Controller/DeprecationController.php      | 31 +++++++
 tests/Functional/ControllerTest.php           |  7 ++
 .../Fixtures/DeprecationController.json       | 86 +++++++++++++++++++
 tests/Model/ModelRegistryTest.php             |  8 +-
 13 files changed, 176 insertions(+), 18 deletions(-)
 create mode 100644 tests/Functional/Controller/DeprecationController.php
 create mode 100644 tests/Functional/Fixtures/DeprecationController.json

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 70dc95021..f23be4a92 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,24 @@
 # CHANGELOG
 
+## 4.33.4
+* Deprecated `null` type from `$options` in `Nelmio\ApiDocBundle\Attribute\Model::__construct()`. Pass an empty array (`[]`) instead.
+* Deprecated `null` type from `$options` in `NNelmio\ApiDocBundle\Attribute\Model::__construct()`. Pass an empty array (`[]`) instead.
+
+## 4.33.3
+* Bumped swagger-ui files from `5.18.1` to `5.18.2`
+* Bumped redoc files to `2.2.0`
+
+## 4.33.2
+* Fixed incorrect directory updated for swagger-ui files from version `4.33.2`
+
+## 4.33.1
+* Bumped swagger-ui files to `5.18.1`
+* Fixed explicitly set default values defined in `#[OA\Property]` being overwritten
+
+## 4.33.0
+* Fixed custom JMS enum type handling
+* Added support for name based serialisation of JMS enums
+
 ## 4.32.3
 
 * Deprecated `Nelmio\ApiDocBundle\Annotation` namespace in favor of `Nelmio\ApiDocBundle\Attribute` namespace in preparation for 5.x. Consider upgrading to the new attribute syntax.
diff --git a/phpunit-ignore.txt b/phpunit-ignore.txt
index c62fbbb5f..0f455835a 100644
--- a/phpunit-ignore.txt
+++ b/phpunit-ignore.txt
@@ -2,4 +2,8 @@
 /^Since nelmio\/api-doc-bundle 4\.17\.0: Using the \$groups parameter of "Nelmio\\ApiDocBundle\\PropertyDescriber\\(PropertyDescriber|IntegerPropertyDescriber|NullablePropertyDescriber|StringPropertyDescriber|ObjectPropertyDescriber)::describe\(\)" is deprecated/
 
 # Ignoring deprecations from Nelmio\ApiDocBundle 4.32.3
-/^Since nelmio\/api-doc-bundle 4\.32\.3: The "Nelmio\\ApiDocBundle\\Annotation\\(Operation|Security|Areas|Model)" class is deprecated and will be removed in 5\.0\. Use the "\\Nelmio\\ApiDocBundle\\Attribute\\(Operation|Security|Areas|Model)" attribute instead\./
\ No newline at end of file
+/^Since nelmio\/api-doc-bundle 4\.32\.3: The "Nelmio\\ApiDocBundle\\Annotation\\(Operation|Security|Areas|Model)" class is deprecated and will be removed in 5\.0\. Use the "\\Nelmio\\ApiDocBundle\\Attribute\\(Operation|Security|Areas|Model)" attribute instead\./
+
+# Ignoring deprecations from Nelmio\ApiDocBundle 4.33.4
+/^Since nelmio\/api-doc-bundle 4\.33\.4: Passing null to the "\$options" argument of "Nelmio\\ApiDocBundle\\Model\\Model\:\:\_\_construct\(\)" is deprecated\, pass an empty array instead\./
+/^Since nelmio\/api-doc-bundle 4\.33\.4: Passing null to the "\$options" argument of "Nelmio\\ApiDocBundle\\Attribute\\Model\:\:\_\_construct\(\)" is deprecated\, pass an empty array instead\./
\ No newline at end of file
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
index bd5c8def7..910850797 100644
--- a/phpunit.xml.dist
+++ b/phpunit.xml.dist
@@ -9,6 +9,7 @@
          failOnRisky="true"
          stopOnFailure="false"
          bootstrap="vendor/autoload.php"
+         displayDetailsOnTestsThatTriggerWarnings="true"
 >
     <php>
         <env name="SHELL_VERBOSITY" value="-1" />
diff --git a/src/Attribute/Model.php b/src/Attribute/Model.php
index 67be0c493..143834d47 100644
--- a/src/Attribute/Model.php
+++ b/src/Attribute/Model.php
@@ -41,9 +41,9 @@ class Model extends Attachable
     public ?array $groups;
 
     /**
-     * @var mixed[]|null
+     * @var mixed[]
      */
-    public ?array $options;
+    public array $options;
 
     /**
      * @var array<string, mixed>
@@ -60,9 +60,14 @@ public function __construct(
         array $properties = [],
         string $type = Generator::UNDEFINED,
         ?array $groups = null,
-        ?array $options = null,
+        ?array $options = [],
         array $serializationContext = []
     ) {
+        if (null === $options) {
+            trigger_deprecation('nelmio/api-doc-bundle', '4.33.4', 'Passing null to the "$options" argument of "%s()" is deprecated, pass an empty array instead.', __METHOD__);
+            $options = [];
+        }
+
         parent::__construct($properties + [
             'type' => $type,
             'groups' => $groups,
diff --git a/src/Model/Model.php b/src/Model/Model.php
index 59504e5d5..1a911d156 100644
--- a/src/Model/Model.php
+++ b/src/Model/Model.php
@@ -18,9 +18,9 @@ final class Model
     private Type $type;
 
     /**
-     * @var mixed[]|null
+     * @var mixed[]
      */
-    private ?array $options;
+    private array $options;
 
     /**
      * @var mixed[]
@@ -32,8 +32,13 @@ final class Model
      * @param mixed[]|null  $options
      * @param mixed[]       $serializationContext
      */
-    public function __construct(Type $type, ?array $groups = null, ?array $options = null, array $serializationContext = [])
+    public function __construct(Type $type, ?array $groups = null, ?array $options = [], array $serializationContext = [])
     {
+        if (null === $options) {
+            trigger_deprecation('nelmio/api-doc-bundle', '4.33.4', 'Passing null to the "$options" argument of "%s()" is deprecated, pass an empty array instead.', __METHOD__);
+            $options = [];
+        }
+
         $this->type = $type;
         $this->options = $options;
         $this->serializationContext = $serializationContext;
@@ -69,9 +74,9 @@ public function getHash(): string
     }
 
     /**
-     * @return mixed[]|null
+     * @return mixed[]
      */
-    public function getOptions(): ?array
+    public function getOptions(): array
     {
         return $this->options;
     }
diff --git a/src/Model/ModelRegistry.php b/src/Model/ModelRegistry.php
index d258d3f14..544b3037e 100644
--- a/src/Model/ModelRegistry.php
+++ b/src/Model/ModelRegistry.php
@@ -70,7 +70,7 @@ public function __construct($modelDescribers, OA\OpenApi $api, array $alternativ
             $this->alternativeNames[] = $model = new Model(
                 new Type('object', false, $criteria['type']),
                 $criteria['groups'],
-                $criteria['options'] ?? null,
+                $criteria['options'] ?? [],
                 $criteria['serializationContext'] ?? [],
             );
             $this->names[$model->getHash()] = $alternativeName;
diff --git a/src/ModelDescriber/FormModelDescriber.php b/src/ModelDescriber/FormModelDescriber.php
index 9c19611c4..ed9f06f9d 100644
--- a/src/ModelDescriber/FormModelDescriber.php
+++ b/src/ModelDescriber/FormModelDescriber.php
@@ -85,7 +85,7 @@ public function describe(Model $model, OA\Schema $schema): void
 
         $this->setContextFromReflection($schema->_context, new \ReflectionClass($class));
 
-        $form = $this->formFactory->create($class, null, $model->getOptions() ?? []);
+        $form = $this->formFactory->create($class, null, $model->getOptions());
         $this->parseForm($schema, $form);
 
         $this->setContext(null);
diff --git a/src/ModelDescriber/JMSModelDescriber.php b/src/ModelDescriber/JMSModelDescriber.php
index 91006dd16..9520bc3a7 100644
--- a/src/ModelDescriber/JMSModelDescriber.php
+++ b/src/ModelDescriber/JMSModelDescriber.php
@@ -350,7 +350,7 @@ public function describeItem(array $type, OA\Schema $property, Context $context,
             $groups = $this->computeGroups($context, $type);
             unset($serializationContext['groups']);
 
-            $model = new Model(new Type(Type::BUILTIN_TYPE_OBJECT, false, $type['name']), $groups, null, $serializationContext);
+            $model = new Model(new Type(Type::BUILTIN_TYPE_OBJECT, false, $type['name']), $groups, [], $serializationContext);
             $modelRef = $this->modelRegistry->register($model);
 
             $customFields = (array) $property->jsonSerialize();
diff --git a/src/PropertyDescriber/ObjectPropertyDescriber.php b/src/PropertyDescriber/ObjectPropertyDescriber.php
index fddd38970..09385fff9 100644
--- a/src/PropertyDescriber/ObjectPropertyDescriber.php
+++ b/src/PropertyDescriber/ObjectPropertyDescriber.php
@@ -56,13 +56,13 @@ public function describe(array $types, OA\Schema $property, ?array $groups = nul
 
         if ($types[0]->isNullable()) {
             $weakContext = Util::createWeakContext($property->_context);
-            $schemas = [new OA\Schema(['ref' => $this->modelRegistry->register(new Model($type, $groups, null, $context)), '_context' => $weakContext])];
+            $schemas = [new OA\Schema(['ref' => $this->modelRegistry->register(new Model($type, $groups, [], $context)), '_context' => $weakContext])];
             $property->oneOf = $schemas;
 
             return;
         }
 
-        $property->ref = $this->modelRegistry->register(new Model($type, $groups, null, $context));
+        $property->ref = $this->modelRegistry->register(new Model($type, $groups, [], $context));
     }
 
     public function supports(array $types): bool
diff --git a/tests/Functional/Controller/DeprecationController.php b/tests/Functional/Controller/DeprecationController.php
new file mode 100644
index 000000000..eec971a02
--- /dev/null
+++ b/tests/Functional/Controller/DeprecationController.php
@@ -0,0 +1,31 @@
+<?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\Attribute\Model;
+use Nelmio\ApiDocBundle\Tests\Functional\Entity\Article81;
+use OpenApi\Attributes as OA;
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\Routing\Annotation\Route;
+
+class DeprecationController
+{
+    #[Route(path: '/legacy/null_options', name: 'legacy_null_options', methods: 'POST')]
+    #[OA\Response(
+        response: 200,
+        description: 'Legacy null options',
+        content: new Model(type: Article81::class, options: null),
+    )]
+    public function __invoke(Article81 $requestDTO): JsonResponse
+    {
+    }
+}
diff --git a/tests/Functional/ControllerTest.php b/tests/Functional/ControllerTest.php
index 76957178e..1bb8dbc3e 100644
--- a/tests/Functional/ControllerTest.php
+++ b/tests/Functional/ControllerTest.php
@@ -105,6 +105,13 @@ public static function provideAttributeTestCases(): \Generator
             [__DIR__.'/Configs/JMS.yaml'],
         ];
 
+        yield 'Deprecation null model options' => [
+            [
+                'name' => 'DeprecationController',
+                'type' => $type,
+            ],
+        ];
+
         if (version_compare(Kernel::VERSION, '6.3.0', '>=')) {
             yield 'https://github.com/nelmio/NelmioApiDocBundle/issues/2209' => [
                 [
diff --git a/tests/Functional/Fixtures/DeprecationController.json b/tests/Functional/Fixtures/DeprecationController.json
new file mode 100644
index 000000000..03c2b3fbb
--- /dev/null
+++ b/tests/Functional/Fixtures/DeprecationController.json
@@ -0,0 +1,86 @@
+{
+    "openapi": "3.0.0",
+    "info": {
+        "title": "",
+        "version": "0.0.0"
+    },
+    "paths": {
+        "/legacy/null_options": {
+            "post": {
+                "operationId": "post_legacy_null_options",
+                "responses": {
+                    "200": {
+                        "description": "Legacy null options",
+                        "content": {
+                            "application/json": {
+                                "schema": {
+                                    "$ref": "#/components/schemas/Article81"
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    },
+    "components": {
+        "schemas": {
+            "Article81": {
+                "required": [
+                    "id",
+                    "type",
+                    "intBackedType",
+                    "notBackedType"
+                ],
+                "properties": {
+                    "id": {
+                        "type": "integer"
+                    },
+                    "type": {
+                        "$ref": "#/components/schemas/ArticleType81"
+                    },
+                    "intBackedType": {
+                        "$ref": "#/components/schemas/ArticleType81IntBacked"
+                    },
+                    "notBackedType": {
+                        "$ref": "#/components/schemas/ArticleType81NotBacked"
+                    },
+                    "nullableType": {
+                        "nullable": true,
+                        "oneOf": [
+                            {
+                                "$ref": "#/components/schemas/ArticleType81"
+                            }
+                        ]
+                    }
+                },
+                "type": "object"
+            },
+            "ArticleType81": {
+                "type": "string",
+                "enum": [
+                    "draft",
+                    "final"
+                ]
+            },
+            "ArticleType81IntBacked": {
+                "type": "integer",
+                "enum": [
+                    0,
+                    1
+                ]
+            },
+            "ArticleType81NotBacked": {
+                "required": [
+                    "name"
+                ],
+                "properties": {
+                    "name": {
+                        "type": "string"
+                    }
+                },
+                "type": "object"
+            }
+        }
+    }
+}
\ No newline at end of file
diff --git a/tests/Model/ModelRegistryTest.php b/tests/Model/ModelRegistryTest.php
index c6bb4fba5..304053091 100644
--- a/tests/Model/ModelRegistryTest.php
+++ b/tests/Model/ModelRegistryTest.php
@@ -50,7 +50,7 @@ public function testNameCollisionsAreLogged(Type $type, array $arrayType): void
                 'Can not assign a name for the model, the name "ModelRegistryTest" has already been taken.', [
                     'model' => [
                         'type' => $arrayType,
-                        'options' => null,
+                        'options' => [],
                         'groups' => ['group2'],
                         'serialization_context' => [
                             'groups' => ['group2'],
@@ -58,7 +58,7 @@ public function testNameCollisionsAreLogged(Type $type, array $arrayType): void
                     ],
                     'taken_by' => [
                         'type' => $arrayType,
-                        'options' => null,
+                        'options' => [],
                         'groups' => ['group1'],
                         'serialization_context' => [
                             'groups' => ['group1'],
@@ -134,7 +134,7 @@ public function testNameCollisionsAreLoggedWithAlternativeNames(): void
                             'collection_key_types' => null,
                             'collection_value_types' => null,
                         ],
-                        'options' => null,
+                        'options' => [],
                         'groups' => ['group2'],
                         'serialization_context' => ['groups' => ['group2']],
                     ],
@@ -147,7 +147,7 @@ public function testNameCollisionsAreLoggedWithAlternativeNames(): void
                             'collection_key_types' => null,
                             'collection_value_types' => null,
                         ],
-                        'options' => null,
+                        'options' => [],
                         'groups' => ['group1'],
                         'serialization_context' => ['groups' => ['group1']],
                     ],