Skip to content

[OptionsResolver] Trigger deprecation only if the option is used #28878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 24, 2018
Merged

[OptionsResolver] Trigger deprecation only if the option is used #28878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Symfony/Component/OptionsResolver/Options.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*
* @author Bernhard Schussek <bschussek@gmail.com>
* @author Tobias Schultze <http://tobion.de>
*
* @method mixed offsetGet(string $option, bool $triggerDeprecation = true)
*/
interface Options extends \ArrayAccess, \Countable
{
Expand Down
20 changes: 17 additions & 3 deletions src/Symfony/Component/OptionsResolver/OptionsResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ class OptionsResolver implements Options
*/
private $deprecated = array();

/**
* The list of options provided by the user.
*/
private $given = array();

/**
* Whether the instance is locked for reading.
*
Expand Down Expand Up @@ -744,6 +749,7 @@ public function resolve(array $options = array())

// Override options set by the user
foreach ($options as $option => $value) {
$clone->given[$option] = true;
$clone->defaults[$option] = $value;
unset($clone->resolved[$option], $clone->lazy[$option]);
}
Expand Down Expand Up @@ -772,7 +778,8 @@ public function resolve(array $options = array())
/**
* Returns the resolved value of an option.
*
* @param string $option The option name
* @param string $option The option name
* @param bool $triggerDeprecation Whether to trigger the deprecation or not (true by default)
*
* @return mixed The option value
*
Expand All @@ -784,14 +791,20 @@ public function resolve(array $options = array())
* @throws OptionDefinitionException If there is a cyclic dependency between
* lazy options and/or normalizers
*/
public function offsetGet($option)
public function offsetGet($option/*, bool $triggerDeprecation = true*/)
{
if (!$this->locked) {
throw new AccessException('Array access is only supported within closures of lazy options and normalizers.');
}

$triggerDeprecation = 1 === \func_num_args() || \func_get_arg(1);

// Shortcut for resolved options
if (array_key_exists($option, $this->resolved)) {
if ($triggerDeprecation && isset($this->deprecated[$option]) && \is_string($this->deprecated[$option])) {
@trigger_error(strtr($this->deprecated[$option], array('%name%' => $option)), E_USER_DEPRECATED);
}

return $this->resolved[$option];
}

Expand Down Expand Up @@ -920,7 +933,8 @@ public function offsetGet($option)
}

// Check whether the option is deprecated
if (isset($this->deprecated[$option])) {
// and it is provided by the user or is being called from a lazy evaluation
if ($triggerDeprecation && isset($this->deprecated[$option]) && (isset($this->given[$option]) || ($this->calling && \is_string($this->deprecated[$option])))) {
$deprecationMessage = $this->deprecated[$option];

if ($deprecationMessage instanceof \Closure) {
Expand Down
128 changes: 112 additions & 16 deletions src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,12 +491,12 @@ public function testSetDeprecatedFailsIfInvalidDeprecationMessageType()
public function testLazyDeprecationFailsIfInvalidDeprecationMessageType()
{
$this->resolver
->setDefault('foo', true)
->setDefined('foo')
->setDeprecated('foo', function (Options $options, $value) {
return false;
})
;
$this->resolver->resolve();
$this->resolver->resolve(array('foo' => null));
}

/**
Expand All @@ -506,16 +506,15 @@ public function testLazyDeprecationFailsIfInvalidDeprecationMessageType()
public function testFailsIfCyclicDependencyBetweenDeprecation()
{
$this->resolver
->setDefault('foo', null)
->setDefault('bar', null)
->setDefined(array('foo', 'bar'))
->setDeprecated('foo', function (Options $options, $value) {
$options['bar'];
})
->setDeprecated('bar', function (Options $options, $value) {
$options['foo'];
})
;
$this->resolver->resolve();
$this->resolver->resolve(array('foo' => null, 'bar' => null));
}

public function testIsDeprecated()
Expand All @@ -539,10 +538,15 @@ public function testIsNotDeprecatedIfEmptyString()
/**
* @dataProvider provideDeprecationData
*/
public function testDeprecationMessages(\Closure $configureOptions, array $options, ?array $expectedError)
public function testDeprecationMessages(\Closure $configureOptions, array $options, ?array $expectedError, int $expectedCount)
{
$count = 0;
error_clear_last();
set_error_handler(function () { return false; });
set_error_handler(function () use (&$count) {
++$count;

return false;
});
$e = error_reporting(0);

$configureOptions($this->resolver);
Expand All @@ -555,6 +559,7 @@ public function testDeprecationMessages(\Closure $configureOptions, array $optio
unset($lastError['file'], $lastError['line']);

$this->assertSame($expectedError, $lastError);
$this->assertSame($expectedCount, $count);
}

public function provideDeprecationData()
Expand All @@ -571,6 +576,7 @@ function (OptionsResolver $resolver) {
'type' => E_USER_DEPRECATED,
'message' => 'The option "foo" is deprecated.',
),
1,
);

yield 'It deprecates an option with custom message' => array(
Expand All @@ -588,20 +594,27 @@ function (OptionsResolver $resolver) {
'type' => E_USER_DEPRECATED,
'message' => 'The option "foo" is deprecated, use "bar" option instead.',
),
2,
);

yield 'It deprecates a missing option with default value' => array(
yield 'It deprecates an option evaluated in another definition' => array(
function (OptionsResolver $resolver) {
// defined by superclass
$resolver
->setDefaults(array('foo' => null, 'bar' => null))
->setDefault('foo', null)
->setDeprecated('foo')
;
// defined by subclass
$resolver->setDefault('bar', function (Options $options) {
return $options['foo']; // It triggers a deprecation
});
},
array('bar' => 'baz'),
array(),
array(
'type' => E_USER_DEPRECATED,
'message' => 'The option "foo" is deprecated.',
),
1,
);

yield 'It deprecates allowed type and value' => array(
Expand All @@ -623,20 +636,46 @@ function (OptionsResolver $resolver) {
'type' => E_USER_DEPRECATED,
'message' => 'Passing an instance of "stdClass" to option "foo" is deprecated, pass its FQCN instead.',
),
1,
);

yield 'It ignores deprecation for missing option without default value' => array(
yield 'It triggers a deprecation based on the value only if option is provided by the user' => array(
function (OptionsResolver $resolver) {
$resolver
->setDefined(array('foo', 'bar'))
->setDeprecated('foo')
->setDefined('foo')
->setAllowedTypes('foo', array('null', 'bool'))
->setDeprecated('foo', function (Options $options, $value) {
if (!\is_bool($value)) {
return 'Passing a value different than true or false is deprecated.';
}

return '';
})
->setDefault('baz', null)
->setAllowedTypes('baz', array('null', 'int'))
->setDeprecated('baz', function (Options $options, $value) {
if (!\is_int($value)) {
return 'Not passing an integer is deprecated.';
}

return '';
})
->setDefault('bar', function (Options $options) {
$options['baz']; // It does not triggers a deprecation

return $options['foo']; // It does not triggers a deprecation
})
;
},
array('bar' => 'baz'),
null,
array('foo' => null), // It triggers a deprecation
array(
'type' => E_USER_DEPRECATED,
'message' => 'Passing a value different than true or false is deprecated.',
),
1,
);

yield 'It ignores deprecation if closure returns an empty string' => array(
yield 'It ignores a deprecation if closure returns an empty string' => array(
function (OptionsResolver $resolver) {
$resolver
->setDefault('foo', null)
Expand All @@ -647,6 +686,7 @@ function (OptionsResolver $resolver) {
},
array('foo' => Bar::class),
null,
0,
);

yield 'It deprecates value depending on other option value' => array(
Expand All @@ -668,6 +708,62 @@ function (OptionsResolver $resolver) {
'type' => E_USER_DEPRECATED,
'message' => 'Using the "date_format" option when the "widget" option is set to "single_text" is deprecated.',
),
1,
);

yield 'It triggers a deprecation for each evaluation' => array(
function (OptionsResolver $resolver) {
$resolver
// defined by superclass
->setDefined('foo')
->setDeprecated('foo')
// defined by subclass
->setDefault('bar', function (Options $options) {
return $options['foo']; // It triggers a deprecation
})
->setNormalizer('bar', function (Options $options, $value) {
$options['foo']; // It triggers a deprecation
$options['foo']; // It triggers a deprecation

return $value;
})
;
},
array('foo' => 'baz'), // It triggers a deprecation
array(
'type' => E_USER_DEPRECATED,
'message' => 'The option "foo" is deprecated.',
),
4,
);

yield 'It ignores a deprecation if no option is provided by the user' => array(
function (OptionsResolver $resolver) {
$resolver
->setDefined('foo')
->setDefault('bar', null)
->setDeprecated('foo')
->setDeprecated('bar')
;
},
array(),
null,
0,
);

yield 'It explicitly ignores a depreciation' => array(
function (OptionsResolver $resolver) {
$resolver
->setDefault('foo', null)
->setDeprecated('foo')
->setDefault('bar', function (Options $options) {
return $options->offsetGet('foo', false);
})
;
},
array(),
null,
0,
);
}

Expand Down