Skip to content

[OptionsResolver] Display full nested option hierarchy in exceptions #33295

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.

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
Sep 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 38 additions & 18 deletions src/Symfony/Component/OptionsResolver/OptionsResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ class OptionsResolver implements Options
*/
private $locked = false;

private $parentsOptions = [];

private static $typeAliases = [
'boolean' => 'bool',
'integer' => 'int',
Expand Down Expand Up @@ -423,7 +425,7 @@ public function setDeprecated(string $option, $deprecationMessage = 'The option
}

if (!isset($this->defined[$option])) {
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist, defined options are: "%s".', $option, implode('", "', array_keys($this->defined))));
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist, defined options are: "%s".', $this->formatOptions([$option]), implode('", "', array_keys($this->defined))));
}

if (!\is_string($deprecationMessage) && !$deprecationMessage instanceof \Closure) {
Expand Down Expand Up @@ -481,7 +483,7 @@ public function setNormalizer($option, \Closure $normalizer)
}

if (!isset($this->defined[$option])) {
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $option, implode('", "', array_keys($this->defined))));
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $this->formatOptions([$option]), implode('", "', array_keys($this->defined))));
}

$this->normalizers[$option] = [$normalizer];
Expand Down Expand Up @@ -526,7 +528,7 @@ public function addNormalizer(string $option, \Closure $normalizer, bool $forceP
}

if (!isset($this->defined[$option])) {
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $option, implode('", "', array_keys($this->defined))));
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $this->formatOptions([$option]), implode('", "', array_keys($this->defined))));
}

if ($forcePrepend) {
Expand Down Expand Up @@ -569,7 +571,7 @@ public function setAllowedValues($option, $allowedValues)
}

if (!isset($this->defined[$option])) {
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $option, implode('", "', array_keys($this->defined))));
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $this->formatOptions([$option]), implode('", "', array_keys($this->defined))));
}

$this->allowedValues[$option] = \is_array($allowedValues) ? $allowedValues : [$allowedValues];
Expand Down Expand Up @@ -610,7 +612,7 @@ public function addAllowedValues($option, $allowedValues)
}

if (!isset($this->defined[$option])) {
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $option, implode('", "', array_keys($this->defined))));
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $this->formatOptions([$option]), implode('", "', array_keys($this->defined))));
}

if (!\is_array($allowedValues)) {
Expand Down Expand Up @@ -651,7 +653,7 @@ public function setAllowedTypes($option, $allowedTypes)
}

if (!isset($this->defined[$option])) {
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $option, implode('", "', array_keys($this->defined))));
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $this->formatOptions([$option]), implode('", "', array_keys($this->defined))));
}

$this->allowedTypes[$option] = (array) $allowedTypes;
Expand Down Expand Up @@ -686,7 +688,7 @@ public function addAllowedTypes($option, $allowedTypes)
}

if (!isset($this->defined[$option])) {
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $option, implode('", "', array_keys($this->defined))));
throw new UndefinedOptionsException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $this->formatOptions([$option]), implode('", "', array_keys($this->defined))));
}

if (!isset($this->allowedTypes[$option])) {
Expand Down Expand Up @@ -793,7 +795,7 @@ public function resolve(array $options = [])
ksort($clone->defined);
ksort($diff);

throw new UndefinedOptionsException(sprintf((\count($diff) > 1 ? 'The options "%s" do not exist.' : 'The option "%s" does not exist.').' Defined options are: "%s".', implode('", "', array_keys($diff)), implode('", "', array_keys($clone->defined))));
throw new UndefinedOptionsException(sprintf((\count($diff) > 1 ? 'The options "%s" do not exist.' : 'The option "%s" does not exist.').' Defined options are: "%s".', $this->formatOptions(array_keys($diff)), implode('", "', array_keys($clone->defined))));
}

// Override options set by the user
Expand All @@ -809,7 +811,7 @@ public function resolve(array $options = [])
if (\count($diff) > 0) {
ksort($diff);

throw new MissingOptionsException(sprintf(\count($diff) > 1 ? 'The required options "%s" are missing.' : 'The required option "%s" is missing.', implode('", "', array_keys($diff))));
throw new MissingOptionsException(sprintf(\count($diff) > 1 ? 'The required options "%s" are missing.' : 'The required option "%s" is missing.', $this->formatOptions(array_keys($diff))));
}

// Lock the container
Expand Down Expand Up @@ -860,10 +862,10 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
// Check whether the option is set at all
if (!isset($this->defaults[$option]) && !\array_key_exists($option, $this->defaults)) {
if (!isset($this->defined[$option])) {
throw new NoSuchOptionException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $option, implode('", "', array_keys($this->defined))));
throw new NoSuchOptionException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $this->formatOptions([$option]), implode('", "', array_keys($this->defined))));
}

throw new NoSuchOptionException(sprintf('The optional option "%s" has no value set. You should make sure it is set with "isset" before reading it.', $option));
throw new NoSuchOptionException(sprintf('The optional option "%s" has no value set. You should make sure it is set with "isset" before reading it.', $this->formatOptions([$option])));
}

$value = $this->defaults[$option];
Expand All @@ -872,17 +874,19 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
if (isset($this->nested[$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.', implode('", "', array_keys($this->calling))));
throw new OptionDefinitionException(sprintf('The options "%s" have a cyclic dependency.', $this->formatOptions(array_keys($this->calling))));
}

if (!\is_array($value)) {
throw new InvalidOptionsException(sprintf('The nested option "%s" with value %s is expected to be of type array, but is of type "%s".', $option, $this->formatValue($value), $this->formatTypeOf($value)));
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), $this->formatTypeOf($value)));
}

// The following section must be protected from cyclic calls.
$this->calling[$option] = true;
try {
$resolver = new self();
$resolver->parentsOptions = $this->parentsOptions;
$resolver->parentsOptions[] = $option;
foreach ($this->nested[$option] as $closure) {
$closure($resolver, $this);
}
Expand All @@ -897,7 +901,7 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
// 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.', implode('", "', array_keys($this->calling))));
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
Expand Down Expand Up @@ -932,10 +936,10 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
$keys = array_keys($invalidTypes);

if (1 === \count($keys) && '[]' === substr($keys[0], -2)) {
throw new InvalidOptionsException(sprintf('The option "%s" with value %s is expected to be of type "%s", but one of the elements is of type "%s".', $option, $this->formatValue($value), implode('" or "', $this->allowedTypes[$option]), $keys[0]));
throw new InvalidOptionsException(sprintf('The option "%s" with value %s is expected to be of type "%s", but one of the elements is of type "%s".', $this->formatOptions([$option]), $this->formatValue($value), implode('" or "', $this->allowedTypes[$option]), $keys[0]));
}

throw new InvalidOptionsException(sprintf('The option "%s" with value %s is expected to be of type "%s", but is of type "%s".', $option, $this->formatValue($value), implode('" or "', $this->allowedTypes[$option]), implode('|', array_keys($invalidTypes))));
throw new InvalidOptionsException(sprintf('The option "%s" with value %s is expected to be of type "%s", but is of type "%s".', $this->formatOptions([$option]), $this->formatValue($value), implode('" or "', $this->allowedTypes[$option]), implode('|', array_keys($invalidTypes))));
}
}

Expand Down Expand Up @@ -989,7 +993,7 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
if ($deprecationMessage instanceof \Closure) {
// 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.', implode('", "', array_keys($this->calling))));
throw new OptionDefinitionException(sprintf('The options "%s" have a cyclic dependency.', $this->formatOptions(array_keys($this->calling))));
}

$this->calling[$option] = true;
Expand All @@ -1012,7 +1016,7 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
// 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.', implode('", "', array_keys($this->calling))));
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
Expand Down Expand Up @@ -1195,4 +1199,20 @@ private function formatValues(array $values): string

return implode(', ', $values);
}

private function formatOptions(array $options): string
{
if ($this->parentsOptions) {
$prefix = array_shift($this->parentsOptions);
if ($this->parentsOptions) {
$prefix .= sprintf('[%s]', implode('][', $this->parentsOptions));
}

$options = array_map(static function (string $option) use ($prefix): string {
return sprintf('%s[%s]', $prefix, $option);
}, $options);
}

return implode('", "', $options);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1993,7 +1993,7 @@ public function testIsNestedOption()
public function testFailsIfUndefinedNestedOption()
{
$this->expectException('Symfony\Component\OptionsResolver\Exception\UndefinedOptionsException');
$this->expectExceptionMessage('The option "foo" does not exist. Defined options are: "host", "port".');
$this->expectExceptionMessage('The option "database[foo]" does not exist. Defined options are: "host", "port".');
$this->resolver->setDefaults([
'name' => 'default',
'database' => function (OptionsResolver $resolver) {
Expand All @@ -2008,7 +2008,7 @@ public function testFailsIfUndefinedNestedOption()
public function testFailsIfMissingRequiredNestedOption()
{
$this->expectException('Symfony\Component\OptionsResolver\Exception\MissingOptionsException');
$this->expectExceptionMessage('The required option "host" is missing.');
$this->expectExceptionMessage('The required option "database[host]" is missing.');
$this->resolver->setDefaults([
'name' => 'default',
'database' => function (OptionsResolver $resolver) {
Expand All @@ -2023,7 +2023,7 @@ public function testFailsIfMissingRequiredNestedOption()
public function testFailsIfInvalidTypeNestedOption()
{
$this->expectException('Symfony\Component\OptionsResolver\Exception\InvalidOptionsException');
$this->expectExceptionMessage('The option "logging" with value null is expected to be of type "bool", but is of type "NULL".');
$this->expectExceptionMessage('The option "database[logging]" with value null is expected to be of type "bool", but is of type "NULL".');
$this->resolver->setDefaults([
'name' => 'default',
'database' => function (OptionsResolver $resolver) {
Expand Down