Skip to content

[Config] type specific check for emptiness #15170

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
Aug 1, 2015
Merged

[Config] type specific check for emptiness #15170

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
9 changes: 9 additions & 0 deletions src/Symfony/Component/Config/Definition/BooleanNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,13 @@ protected function validateType($value)
throw $ex;
}
}

/**
* {@inheritdoc}
*/
protected function isValueEmpty($value)
{
// a boolean value cannot be empty
return false;
}
}
9 changes: 9 additions & 0 deletions src/Symfony/Component/Config/Definition/NumericNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,13 @@ protected function finalizeValue($value)

return $value;
}

/**
* {@inheritdoc}
*/
protected function isValueEmpty($value)
{
// a numeric value cannot be empty
return false;
}
}
8 changes: 8 additions & 0 deletions src/Symfony/Component/Config/Definition/ScalarNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,12 @@ protected function validateType($value)
throw $ex;
}
}

/**
* {@inheritdoc}
*/
protected function isValueEmpty($value)
{
return null === $value || '' === $value;
}
}
18 changes: 17 additions & 1 deletion src/Symfony/Component/Config/Definition/VariableNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ protected function validateType($value)
*/
protected function finalizeValue($value)
{
if (!$this->allowEmptyValue && empty($value)) {
if (!$this->allowEmptyValue && $this->isValueEmpty($value)) {
$ex = new InvalidConfigurationException(sprintf(
'The path "%s" cannot contain an empty value, but got %s.',
$this->getPath(),
Expand Down Expand Up @@ -113,4 +113,20 @@ protected function mergeValues($leftSide, $rightSide)
{
return $rightSide;
}

/**
* Evaluates if the given value is to be treated as empty.
*
* By default, PHP's empty() function is used to test for emptiness. This
* method may be overridden by subtypes to better match their understanding
* of empty data.
*
* @param mixed $value
*
* @return bool
*/
protected function isValueEmpty($value)
{
return empty($value);
}
}
13 changes: 13 additions & 0 deletions src/Symfony/Component/Config/Tests/Definition/BooleanNodeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ public function testNormalize($value)
$this->assertSame($value, $node->normalize($value));
}

/**
* @dataProvider getValidValues
*
* @param bool $value
*/
public function testValidNonEmptyValues($value)
{
$node = new BooleanNode('test');
$node->setAllowEmptyValue(false);

$this->assertSame($value, $node->finalize($value));
}

public function getValidValues()
{
return array(
Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Component/Config/Tests/Definition/FloatNodeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ public function testNormalize($value)
$this->assertSame($value, $node->normalize($value));
}

/**
* @dataProvider getValidValues
*
* @param int $value
*/
public function testValidNonEmptyValues($value)
{
$node = new FloatNode('test');
$node->setAllowEmptyValue(false);

$this->assertSame($value, $node->finalize($value));
}

public function getValidValues()
{
return array(
Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Component/Config/Tests/Definition/IntegerNodeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ public function testNormalize($value)
$this->assertSame($value, $node->normalize($value));
}

/**
* @dataProvider getValidValues
*
* @param int $value
*/
public function testValidNonEmptyValues($value)
{
$node = new IntegerNode('test');
$node->setAllowEmptyValue(false);

$this->assertSame($value, $node->finalize($value));
}

public function getValidValues()
{
return array(
Expand Down
47 changes: 47 additions & 0 deletions src/Symfony/Component/Config/Tests/Definition/ScalarNodeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,51 @@ public function getInvalidValues()
array(new \stdClass()),
);
}

/**
* @dataProvider getValidNonEmptyValues
*
* @param mixed $value
*/
public function testValidNonEmptyValues($value)
{
$node = new ScalarNode('test');
$node->setAllowEmptyValue(false);

$this->assertSame($value, $node->finalize($value));
}

public function getValidNonEmptyValues()
{
return array(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about "0" ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well shouldn't that value be there as well ?

Maybe I'm wrong, I just wanted to make sure you didn't accidentally miss it :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true that we could have added it, but I think it's not absolutely necessary. Of course, if you think it deserves its own test, please don't hesitate to make a pull request for it. :)

array(false),
array(true),
array('foo'),
array(0),
array(1),
array(0.0),
array(0.1),
);
}

/**
* @dataProvider getEmptyValues
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
*
* @param mixed $value
*/
public function testNotAllowedEmptyValuesThrowException($value)
{
$node = new ScalarNode('test');
$node->setAllowEmptyValue(false);
$node->finalize($value);
}

public function getEmptyValues()
{
return array(
array(null),
array(''),
);
}
}