8000 feature #59618 [OptionsResolver] Deprecate defining nested options vi… · symfony/symfony@ffdbc83 · GitHub
[go: up one dir, main page]
Skip to content

Commit ffdbc83

Browse files
fabpotfabpot
committed
feature #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 #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
2 parents 1a1f81c + b0bb9a1 commit ffdbc83

11 files changed

+852
-99
lines changed

‎UPGRADE-7.3.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,25 @@ FrameworkBundle
9494
* Deprecate the `--show-arguments` option of the `container:debug` command, as arguments are now always shown
9595
* Deprecate the `framework.validation.cache` config option
9696

97+
OptionsResolver
98+
---------------
99+
100+
* Deprecate defining nested options via `setDefault()`, use `setOptions()` instead
101+
102+
*Before*
103+
```php
104+
$resolver->setDefault('option', function (OptionsResolver $resolver) {
105+
// ...
106+
});
107+
```
108+
109+
*After*
110+
```php
111+
$resolver->setOptions('option', function (OptionsResolver $resolver) {
112+
// ...
113+
});
114+
```
115+
97116
SecurityBundle
98117
--------------
99118

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ protected function configureOptions(OptionsResolver $resolver): void
170170
$resolver->setAllowedTypes('debug', 'bool');
171171
$resolver->setDefault('referrals', false);
172172
$resolver->setAllowedTypes('referrals', 'bool');
173-
$resolver->setDefault('options', function (OptionsResolver $options, Options $parent) {
173+
$resolver->setOptions('options', function (OptionsResolver $options, Options $parent) {
174174
$options->setDefined(array_map('strtolower', array_keys((new \ReflectionClass(ConnectionOptions::class))->getConstants())));
175175

176176
if (true === $parent['debug']) {

‎src/Symfony/Component/Ldap/composer.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,13 @@
1818
"require": {
1919
"php": ">=8.2",
2020
"ext-ldap": "*",
21-
"symfony/options-resolver": "^6.4|^7.0"
21+
"symfony/options-resolver": "^7.3"
2222
},
2323
"require-dev": {
2424
"symfony/security-core": "^6.4|^7.0",
2525
"symfony/security-http": "^6.4|^7.0"
2626
},
2727
"conflict": {
28-
"symfony/options-resolver": "<6.4",
2928
"symfony/security-core": "<6.4"
3029
},
3130
"autoload": {

‎src/Symfony/Component/OptionsResolver/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ CHANGELOG
55
---
66

77
* Support union type in `OptionResolver::setAllowedTypes()` method
8+
* Add `OptionsResolver::setOptions()` and `OptionConfigurator::options()` methods
9+
* Deprecate defining nested options via `setDefault()`, use `setOptions()` instead
810

911
6.4
1012
---

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,14 @@ public function getDeprecation(string $option): array
101101
{
102102
return ($this->get)('deprecated', $option, \sprintf('No deprecation was set for the "%s" option.', $option));
103103
}
104+
105+
/**
106+
* @return \Closure[]
107+
*
108+
* @throws NoConfigurationException when no nested option is configured
109+
*/
110+
public function getNestedOptions(string $option): array
111+
{
112+
return ($this->get)('nested', $option, \sprintf('No nested option was set for the "%s" option.', $option));
113+
}
104114
}

‎src/Symfony/Component/OptionsResolver/OptionConfigurator.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,18 @@ public function ignoreUndefined(bool $ignore = true): static
143143

144144
return $this;
145145
}
146+
147+
/**
148+
* Defines nested options.
149+
*
150+
* @param \Closure(OptionsResolver $resolver, Options $parent): void $nested
151+
*
152+
* @return $this
153+
*/
154+
public function options(\Closure $nested): static
155+
{
156+
$this->resolver->setOptions($this->name, $nested);
157+
158+
return $this;
159+
}
146160
}

‎src/Symfony/Component/OptionsResolver/OptionsResolver.php

Lines changed: 66 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,13 @@ class OptionsResolver implements Options
6464
*/
6565
private array $nested = [];
6666

67+
/**
68+
* BC layer. Remove in Symfony 8.0.
69+
*
70+
* @var array<string, true>
71+
*/
72+
private array $deprecatedNestedOptions = [];
73+
6774
/**
6875
* The names of required options.
6976
*/
@@ -178,20 +185,6 @@ class OptionsResolver implements Options
178185
* is spread across different locations of your code, such as base and
179186
* sub-classes.
180187
*
181-
* If you want to define nested options, you can pass a closure with the
182-
* following signature:
183-
*
184-
* $options->setDefault('database', function (OptionsResolver $resolver) {
185-
* $resolver->setDefined(['dbname', 'host', 'port', 'user', 'pass']);
186-
* }
187-
*
188-
* To get access to the parent options, add a second argument to the closure's
189-
* signature:
190-
*
191-
* function (OptionsResolver $resolver, Options $parent) {
192-
* // 'default' === $parent['connection']
193-
* }
194-
*
195188
* @return $this
196189
*
197190
* @throws AccessException If called from a lazy option or normalizer
@@ -226,13 +219,22 @@ public function setDefault(string $option, mixed $value): static
226219
$this->lazy[$option][] = $value;
227220
$this->defined[$option] = true;
228221

229-
// Make sure the option is processed and is not nested anymore
230-
unset($this->resolved[$option], $this->nested[$option]);
222+
// Make sure the option is processed
223+
unset($this->resolved[$option]);
224+
225+
// BC layer. Remove in Symfony 8.0.
226+
if (isset($this->deprecatedNestedOptions[$option])) {
227+
unset($this->nested[$option]);
228+
}
231229

232230
return $this;
233231
}
234232

233+
// Remove in Symfony 8.0.
235234
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()))) {
235+
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__);
236+
$this->deprecatedNestedOptions[$option] = true;
237+
236238
// Store closure for later evaluation
237239
$this->nested[$option][] = $value;
238240
$this->defaults[$option] = [];
@@ -245,8 +247,13 @@ public function setDefault(string $option, mixed $value): static
245247
}
246248
}
247249

248-
// This option is not lazy nor nested anymore
249-
unset($this->lazy[$option], $this->nested[$option]);
250+
// This option is not lazy anymore
251+
unset($this->lazy[$option]);
252+
253+
// BC layer. Remove in Symfony 8.0.
254+
if (isset($this->deprecatedNestedOptions[$option])) {
255+
unset($this->nested[$option]);
256+
}
250257

251258
// Yet undefined options can be marked as resolved, because we only need
252259
// to resolve options with lazy closures, normalizers or validation
@@ -403,6 +410,30 @@ public function getDefinedOptions(): array
403410
return array_keys($this->defined);
404411
}
405412

413+
/**
414+
* Defines nested options.
415+
*
416+
* @param \Closure(self $resolver, Options $parent): void $nested
417+
*
418+
* @return $this
419+
*/
420+
public function setOptions(string $option, \Closure $nested): static
421+
{
422+
if ($this->locked) {
423+
throw new AccessException('Nested options cannot be defined from a lazy option or normalizer.');
424+
}
425+
426+
// Store closure for later evaluation
427+
$this->nested[$option][] = $nested;
428+
$this->defaults[$option] = [];
429+
$this->defined[$option] = true;
430+
431+
// Make sure the option is processed
432+
unset($this->resolved[$option]);
433+
434+
return $this;
435+
}
436+
406437
public function isNested(string $option): bool
407438
{
408439
return isset($this->nested[$option]);
@@ -947,6 +978,23 @@ public function offsetGet(mixed $option, bool $triggerDeprecation = true): mixed
947978

948979
$value = $this->defaults[$option];
949980

981+
// Resolve the option if the default value is lazily evaluated
982+
if (isset($this->lazy[$option])) {
983+
// If the closure is already being called, we have a cyclic dependency
984+
if (isset($this->calling[$option])) {
985+
throw new OptionDefinitionException(\sprintf('The options "%s" have a cyclic dependency.', $this->formatOptions(array_keys($this->calling))));
986+
}
987+
988+
$this->calling[$option] = true;
989+
try {
990+
foreach ($this->lazy[$option] as $closure) {
991+
$value = $closure($this, $value);
992+
}
993+
} finally {
994+
unset($this->calling[$option]);
995+
}
996+
}
997+
950998
// Resolve the option if it is a nested definition
951999
if (isset($this->nested[$option])) {
9521000
// 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
9581006
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)));
9591007
}
9601008

961-
// The following section must be protected from cyclic calls.
9621009
$this->calling[$option] = true;
9631010
try {
9641011
$resolver = new self();
@@ -989,29 +1036,6 @@ public function offsetGet(mixed $option, bool $triggerDeprecation = true): mixed
9891036
}
9901037
}
9911038

992-
// Resolve the option if the default value is lazily evaluated
993-
if (isset($this->lazy[$option])) {
994-
// If the closure is already being called, we have a cyclic
995-
// dependency
996-
if (isset($this->calling[$option])) {
997-
throw new OptionDefinitionException(\sprintf('The options "%s" have a cyclic dependency.', $this->formatOptions(array_keys($this->calling))));
998-
}
999-
1000-
// The following section must be protected from cyclic
1001-
// calls. Set $calling for the current $option to detect a cyclic
1002-
// dependency
1003-
// BEGIN
1004-
$this->calling[$option] = true;
1005-
try {
1006-
foreach ($this->lazy[$option] as $closure) {
1007-
$value = $closure($this, $value);
1008-
}
1009-
} finally {
1010-
unset($this->calling[$option]);
1011-
}
1012-
// END
1013-
}
1014-
10151039
// Validate the type of the resolved option
10161040
if (isset($this->allowedTypes[$option])) {
10171041
$valid = true;

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,4 +263,13 @@ public function testGetDeprecationMessageThrowsOnNotDefinedOption()
263263
$debug = new OptionsResolverIntrospector($resolver);
264264
$debug->getDeprecation('foo');
265265
}
266+
267+
public function testGetClosureNested()
268+
{
269+
$resolver = new OptionsResolver();
270+
$resolver->setOptions('foo', $closure = function (OptionsResolver $resolver) {});
271+
272+
$debug = new OptionsResolverIntrospector($resolver);
273+
$this->assertSame([$closure], $debug->getNestedOptions('foo'));
274+
}
266275
}

0 commit comments

Comments
 (0)
0