feature symfony#59618 [OptionsResolver] Deprecate defining nested options via setDefault() use setOptions() instead (yceruto)

This PR was merged into the 7.3 branch.

Discussion
----------

[OptionsResolver] Deprecate defining nested options via `setDefault()` use `setOptions()` instead

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | yes
| Issues        | -
| License       | MIT

this removes unnecessary limitations that I hadn't considered when introducing nested options feature in symfony#27291.

#### 1. Allow defining default values for nested options

Imagine you want to define the following nested option in a generic form type:

```php
class GenericType extends AbstractType
{
    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefault('foo', function (OptionsResolver $foo) {
            $foo->define('bar')->allowedTypes('int');
        });
    }
}
```

then, I'd like to define a concrete type with a default value for it.

```php
class ConcreteType extends AbstractType
{
    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefault('foo', ['bar' => 23]);
    }

    public function getParent(): string
    {
        return GenericType::class;
    }
}
```

this might seem unexpected, but the fact is that the nested definition for `foo` option in `ConcreteType` is gone. As a result, when resolved, the `foo` option will have a default value (`['bar' => 23]`) but without any constraints, allowing end users any value/type to be passed through this option for `ConcreteType` instances

For example, passing `['foo' => false]` as options for `ConcreteType` would be allowed, instead of requiring an array where `bar` expects an integer value.

#### 2. Allow defining lazy default for nested options

the same problem would occur with a lazy default for a nested definition:

```php
class ConcreteType extends AbstractType
{
    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setRequired(['baz'])
        $resolver->setDefault('foo', function (Options $options) {
            return ['bar' => $options['baz']];
        });
    }

    public function getParent(): string
    {
        return GenericType::class;
    }
}
```

the issue here is the same as in the previous example, meaning this new default essentially overrides/removes the original nested definition

---

the two features mentioned earlier are now supported by introducing a new method `setOptions()`, which separates the nested definition from its default value (whether direct or lazy). Additionally this PR deprecates the practice of defining nested options using `setDefault()` method

this also enables the ability to set default values for prototyped options, which wasn't possible before. For example:
```php
class NavigatorType extends AbstractType
{
    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->define('buttons')
            ->options(function (OptionsResolver $buttons) {
                $buttons->setPrototype(true);
                $buttons->define('name')->required()->allowedTypes('string');
                $buttons->define('type')->default(SubmitType::class)->allowedTypes('string');
                $buttons->define('options')->default([])->allowedTypes('array');
            })
            ->default([
                'back' => ['name' => 'back', 'options' => ['validate' => false, 'validation_groups' => false]],
                'next' => ['name' => 'next'],
                'submit' => ['name' => 'submit'],
            ]);
    }
}
```

cheers!

Commits
-------

b0bb9a1 Add setOptions method

Author: fabpot

UPGRADE-7.3.md

@@ -94,6 +94,25 @@ FrameworkBundle
  * Deprecate the `--show-arguments` option of the `container:debug` command, as arguments are now always shown
  * Deprecate the `framework.validation.cache` config option
 
+OptionsResolver
+---------------
+
+* Deprecate defining nested options via `setDefault()`, use `setOptions()` instead
+
+  *Before*
+  ```php
+  $resolver->setDefault('option', function (OptionsResolver $resolver) {
+      // ...
+  });
+  ```
+
+  *After*
+  ```php
+  $resolver->setOptions('option', function (OptionsResolver $resolver) {
+      // ...
+  });
+  ```
+
 SecurityBundle
 --------------
 

src/Symfony/Component/Ldap/Adapter/ExtLdap/Connection.php

@@ -170,7 +170,7 @@ protected function configureOptions(OptionsResolver $resolver): void
         $resolver->setAllowedTypes('debug', 'bool');
         $resolver->setDefault('referrals', false);
         $resolver->setAllowedTypes('referrals', 'bool');
-        $resolver->setDefault('options', function (OptionsResolver $options, Options $parent) {
+        $resolver->setOptions('options', function (OptionsResolver $options, Options $parent) {
             $options->setDefined(array_map('strtolower', array_keys((new \ReflectionClass(ConnectionOptions::class))->getConstants())));
 
             if (true === $parent['debug']) {

src/Symfony/Component/Ldap/composer.json

@@ -18,14 +18,13 @@
     "require": {
         "php": ">=8.2",
         "ext-ldap": "*",
-        "symfony/options-resolver": "^6.4|^7.0"
+        "symfony/options-resolver": "^7.3"
     },
     "require-dev": {
         "symfony/security-core": "^6.4|^7.0",
         "symfony/security-http": "^6.4|^7.0"
     },
     "conflict": {
-        "symfony/options-resolver": "<6.4",
         "symfony/security-core": "<6.4"
     },
     "autoload": {

src/Symfony/Component/OptionsResolver/CHANGELOG.md

@@ -5,6 +5,8 @@ CHANGELOG
 ---
 
  * Support union type in `OptionResolver::setAllowedTypes()` method
+ * Add `OptionsResolver::setOptions()` and `OptionConfigurator::options()` methods
+ * Deprecate defining nested options via `setDefault()`, use `setOptions()` instead
 
 6.4
 ---

src/Symfony/Component/OptionsResolver/Debug/OptionsResolverIntrospector.php

@@ -101,4 +101,14 @@ public function getDeprecation(string $option): array
     {
         return ($this->get)('deprecated', $option, \sprintf('No deprecation was set for the "%s" option.', $option));
     }
+
+    /**
+     * @return \Closure[]
+     *
+     * @throws NoConfigurationException when no nested option is configured
+     */
+    public function getNestedOptions(string $option): array
+    {
+        return ($this->get)('nested', $option, \sprintf('No nested option was set for the "%s" option.', $option));
+    }
 }

src/Symfony/Component/OptionsResolver/OptionConfigurator.php

@@ -143,4 +143,18 @@ public function ignoreUndefined(bool $ignore = true): static
 
         return $this;
     }
+
+    /**
+     * Defines nested options.
+     *
+     * @param \Closure(OptionsResolver $resolver, Options $parent): void $nested
+     *
+     * @return $this
+     */
+    public function options(\Closure $nested): static
+    {
+        $this->resolver->setOptions($this->name, $nested);
+
+        return $this;
+    }
 }

src/Symfony/Component/OptionsResolver/OptionsResolver.php

@@ -64,6 +64,13 @@ class OptionsResolver implements Options
      */
     private array $nested = [];
 
+    /**
+     * BC layer. Remove in Symfony 8.0.
+     *
+     * @var array<string, true>
+     */
+    private array $deprecatedNestedOptions = [];
+
     /**
      * The names of required options.
      */
@@ -178,20 +185,6 @@ class OptionsResolver implements Options
      * is spread across different locations of your code, such as base and
      * sub-classes.
      *
-     * If you want to define nested options, you can pass a closure with the
-     * following signature:
-     *
-     *     $options->setDefault('database', function (OptionsResolver $resolver) {
-     *         $resolver->setDefined(['dbname', 'host', 'port', 'user', 'pass']);
-     *     }
-     *
-     * To get access to the parent options, add a second argument to the closure's
-     * signature:
-     *
-     *     function (OptionsResolver $resolver, Options $parent) {
-     *         // 'default' === $parent['connection']
-     *     }
-     *
      * @return $this
      *
      * @throws AccessException If called from a lazy option or normalizer
@@ -226,13 +219,22 @@ public function setDefault(string $option, mixed $value): static
                 $this->lazy[$option][] = $value;
                 $this->defined[$option] = true;
 
-                // Make sure the option is processed and is not nested anymore
-                unset($this->resolved[$option], $this->nested[$option]);
+                // Make sure the option is processed
+                unset($this->resolved[$option]);
+
+                // BC layer. Remove in Symfony 8.0.
+                if (isset($this->deprecatedNestedOptions[$option])) {
+                    unset($this->nested[$option]);
+                }
 
                 return $this;
             }
 
+            // Remove in Symfony 8.0.
             if (isset($params[0]) && ($type = $params[0]->getType()) instanceof \ReflectionNamedType && self::class === $type->getName() && (!isset($params[1]) || (($type = $params[1]->getType()) instanceof \ReflectionNamedType && Options::class === $type->getName()))) {
+                trigger_deprecation('symfony/options-resolver', '7.3', 'Defining nested options via "%s()" is deprecated and will be removed in Symfony 8.0, use "setOptions()" method instead.', __METHOD__);
+                $this->deprecatedNestedOptions[$option] = true;
+
                 // Store closure for later evaluation
                 $this->nested[$option][] = $value;
                 $this->defaults[$option] = [];
@@ -245,8 +247,13 @@ public function setDefault(string $option, mixed $value): static
             }
         }
 
-        // This option is not lazy nor nested anymore
-        unset($this->lazy[$option], $this->nested[$option]);
+        // This option is not lazy anymore
+        unset($this->lazy[$option]);
+
+        // BC layer. Remove in Symfony 8.0.
+        if (isset($this->deprecatedNestedOptions[$option])) {
+            unset($this->nested[$option]);
+        }
 
         // Yet undefined options can be marked as resolved, because we only need
         // to resolve options with lazy closures, normalizers or validation
@@ -403,6 +410,30 @@ public function getDefinedOptions(): array
         return array_keys($this->defined);
     }
 
+    /**
+     * Defines nested options.
+     *
+     * @param \Closure(self $resolver, Options $parent): void $nested
+     *
+     * @return $this
+     */
+    public function setOptions(string $option, \Closure $nested): static
+    {
+        if ($this->locked) {
+            throw new AccessException('Nested options cannot be defined from a lazy option or normalizer.');
+        }
+
+        // Store closure for later evaluation
+        $this->nested[$option][] = $nested;
+        $this->defaults[$option] = [];
+        $this->defined[$option] = true;
+
+        // Make sure the option is processed
+        unset($this->resolved[$option]);
+
+        return $this;
+    }
+
     public function isNested(string $option): bool
     {
         return isset($this->nested[$option]);
@@ -947,6 +978,23 @@ public function offsetGet(mixed $option, bool $triggerDeprecation = true): mixed
 
         $value = $this->defaults[$option];
 
+        // Resolve the option if the default value is lazily evaluated
+        if (isset($this->lazy[$option])) {
+            // If the closure is already being called, we have a cyclic dependency
+            if (isset($this->calling[$option])) {
+                throw new OptionDefinitionException(\sprintf('The options "%s" have a cyclic dependency.', $this->formatOptions(array_keys($this->calling))));
+            }
+
+            $this->calling[$option] = true;
+            try {
+                foreach ($this->lazy[$option] as $closure) {
+                    $value = $closure($this, $value);
+                }
+            } finally {
+                unset($this->calling[$option]);
+            }
+        }
+
         // Resolve the option if it is a nested definition
         if (isset($this->nested[$option])) {
             // If the closure is already being called, we have a cyclic dependency
@@ -958,7 +1006,6 @@ public function offsetGet(mixed $option, bool $triggerDeprecation = true): mixed
                 throw new InvalidOptionsException(\sprintf('The nested option "%s" with value %s is expected to be of type array, but is of type "%s".', $this->formatOptions([$option]), $this->formatValue($value), get_debug_type($value)));
             }
 
-            // The following section must be protected from cyclic calls.
             $this->calling[$option] = true;
             try {
                 $resolver = new self();
@@ -989,29 +1036,6 @@ public function offsetGet(mixed $option, bool $triggerDeprecation = true): mixed
             }
         }
 
-        // Resolve the option if the default value is lazily evaluated
-        if (isset($this->lazy[$option])) {
-            // If the closure is already being called, we have a cyclic
-            // dependency
-            if (isset($this->calling[$option])) {
-                throw new OptionDefinitionException(\sprintf('The options "%s" have a cyclic dependency.', $this->formatOptions(array_keys($this->calling))));
-            }
-
-            // The following section must be protected from cyclic
-            // calls. Set $calling for the current $option to detect a cyclic
-            // dependency
-            // BEGIN
-            $this->calling[$option] = true;
-            try {
-                foreach ($this->lazy[$option] as $closure) {
-                    $value = $closure($this, $value);
-                }
-            } finally {
-                unset($this->calling[$option]);
-            }
-            // END
-        }
-
         // Validate the type of the resolved option
         if (isset($this->allowedTypes[$option])) {
             $valid = true;

src/Symfony/Component/OptionsResolver/Tests/Debug/OptionsResolverIntrospectorTest.php

@@ -263,4 +263,13 @@ public function testGetDeprecationMessageThrowsOnNotDefinedOption()
         $debug = new OptionsResolverIntrospector($resolver);
         $debug->getDeprecation('foo');
     }
+
+    public function testGetClosureNested()
+    {
+        $resolver = new OptionsResolver();
+        $resolver->setOptions('foo', $closure = function (OptionsResolver $resolver) {});
+
+        $debug = new OptionsResolverIntrospector($resolver);
+        $this->assertSame([$closure], $debug->getNestedOptions('foo'));
+    }
 }