merged branch Tobion/formhaserrors (PR #5356)

Commits
-------

d5eb4f7 [Form] fix phpdoc of Form::hasErrors
5cb8264 [Form] deprecated Form::hasErrors that isn't part of the Interface

Discussion
----------

[Form] deprecated Form::hasErrors that isn't part of the Interface

This method is not part of FormInterface, so I deprecated it as it cannot be used reliably. This is consistent with other hassers that were deprecated like `hasChildren` where one should use `count` instead.

---------------------------------------------------------------------------

by stof at 2012-08-26T19:11:19Z

You should deprecate it, not remove it

---------------------------------------------------------------------------

by Tobion at 2012-08-26T19:17:35Z

oh right. I thought it was added in 2.1 and thus can be removed but it's also in 2.0.
Done.

---------------------------------------------------------------------------

by bschussek at 2012-08-29T11:00:32Z

👍
This commit is contained in:
Fabien Potencier 2012-08-29 13:42:45 +02:00
commit 867e31ca39
4 changed files with 54 additions and 54 deletions

View File

@ -708,7 +708,7 @@ class Form implements \IteratorAggregate, FormInterface
throw new \LogicException('You cannot call isValid() on a form that is not bound.'); throw new \LogicException('You cannot call isValid() on a form that is not bound.');
} }
if ($this->hasErrors()) { if (count($this->errors) > 0) {
return false; return false;
} }
@ -724,15 +724,15 @@ class Form implements \IteratorAggregate, FormInterface
} }
/** /**
* Returns whether or not there are errors. * Returns whether there are errors associated with this form.
* *
* @return Boolean true if form is bound and not valid * @return Boolean
*
* @deprecated Deprecated since version 2.1, to be removed in 2.3. Count
* {@link getErrors()} instead.
*/ */
public function hasErrors() public function hasErrors()
{ {
// Don't call isValid() here, as its semantics are slightly different
// Forms are not valid if their children are invalid, but
// hasErrors() returns only true if a form itself has errors
return count($this->errors) > 0; return count($this->errors) > 0;
} }

View File

@ -184,7 +184,7 @@ interface FormInterface extends \ArrayAccess, \Traversable, \Countable
public function addError(FormError $error); public function addError(FormError $error);
/** /**
* Returns whether the form is valid. * Returns whether the form and all children are valid.
* *
* @return Boolean * @return Boolean
*/ */

View File

@ -124,9 +124,9 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->mapper->mapViolation($violation, $parent); $this->mapper->mapViolation($violation, $parent);
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $child->getErrors(), $child->getName() . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $child->getErrors(), $child->getName() . ' should have an error, but has none');
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName() . ' should not have an error, but has one');
} }
public function testFollowDotRules() public function testFollowDotRules()
@ -149,9 +149,9 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->mapper->mapViolation($violation, $parent); $this->mapper->mapViolation($violation, $parent);
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $child->getName() . ' should not have an error, but has one');
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName() . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $grandGrandChild->getErrors(), $grandGrandChild->getName() . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $grandGrandChild->getErrors(), $grandGrandChild->getName() . ' should have an error, but has none');
} }
@ -172,9 +172,9 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->mapper->mapViolation($violation, $parent); $this->mapper->mapViolation($violation, $parent);
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $child->getName() . ' should not have an error, but has one');
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName() . ' should not have an error, but has one');
} }
public function testAbortVirtualFormMappingIfNotSynchronized() public function testAbortVirtualFormMappingIfNotSynchronized()
@ -194,9 +194,9 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->mapper->mapViolation($violation, $parent); $this->mapper->mapViolation($violation, $parent);
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $child->getName() . ' should not have an error, but has one');
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName() . ' should not have an error, but has one');
} }
public function testAbortDotRuleMappingIfNotSynchronized() public function testAbortDotRuleMappingIfNotSynchronized()
@ -218,9 +218,9 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->mapper->mapViolation($violation, $parent); $this->mapper->mapViolation($violation, $parent);
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $child->getName() . ' should not have an error, but has one');
$this->assertFalse($grandChild->hasErrors(), $grandChild->getName() . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName() . ' should not have an error, but has one');
} }
public function provideDefaultTests() public function provideDefaultTests()
@ -732,15 +732,15 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
if (self::LEVEL_0 === $target) { if (self::LEVEL_0 === $target) {
$this->assertEquals(array($this->getFormError()), $parent->getErrors(), $parent->getName() . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $parent->getErrors(), $parent->getName() . ' should have an error, but has none');
$this->assertFalse($child->hasErrors(), $childName . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $childName . ' should not have an error, but has one');
$this->assertFalse($grandChild->hasErrors(), $grandChildName . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChildName . ' should not have an error, but has one');
} elseif (self::LEVEL_1 === $target) { } elseif (self::LEVEL_1 === $target) {
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $child->getErrors(), $childName . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $child->getErrors(), $childName . ' should have an error, but has none');
$this->assertFalse($grandChild->hasErrors(), $grandChildName . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChildName . ' should not have an error, but has one');
} else { } else {
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertFalse($child->hasErrors(), $childName . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $childName . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none');
} }
} }
@ -1199,20 +1199,20 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->mapper->mapViolation($violation, $parent); $this->mapper->mapViolation($violation, $parent);
if ($target !== self::LEVEL_0) { if ($target !== self::LEVEL_0) {
$this->assertFalse($distraction->hasErrors(), 'distraction should not have an error, but has one'); $this->assertCount(0, $distraction->getErrors(), 'distraction should not have an error, but has one');
} }
if (self::LEVEL_0 === $target) { if (self::LEVEL_0 === $target) {
$this->assertEquals(array($this->getFormError()), $parent->getErrors(), $parent->getName() . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $parent->getErrors(), $parent->getName() . ' should have an error, but has none');
$this->assertFalse($child->hasErrors(), $childName . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $childName . ' should not have an error, but has one');
$this->assertFalse($grandChild->hasErrors(), $grandChildName . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChildName . ' should not have an error, but has one');
} elseif (self::LEVEL_1 === $target) { } elseif (self::LEVEL_1 === $target) {
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $child->getErrors(), $childName . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $child->getErrors(), $childName . ' should have an error, but has none');
$this->assertFalse($grandChild->hasErrors(), $grandChildName . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChildName . ' should not have an error, but has one');
} else { } else {
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertFalse($child->hasErrors(), $childName . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $childName . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none');
} }
} }
@ -1385,24 +1385,24 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->mapper->mapViolation($violation, $parent); $this->mapper->mapViolation($violation, $parent);
if (self::LEVEL_0 === $target) { if (self::LEVEL_0 === $target) {
$this->assertFalse($errorChild->hasErrors(), $errorName . ' should not have an error, but has one'); $this->assertCount(0, $errorChild->getErrors(), $errorName . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $parent->getErrors(), $parent->getName() . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $parent->getErrors(), $parent->getName() . ' should have an error, but has none');
$this->assertFalse($child->hasErrors(), $childName . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $childName . ' should not have an error, but has one');
$this->assertFalse($grandChild->hasErrors(), $grandChildName . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChildName . ' should not have an error, but has one');
} elseif (self::LEVEL_1 === $target) { } elseif (self::LEVEL_1 === $target) {
$this->assertFalse($errorChild->hasErrors(), $errorName . ' should not have an error, but has one'); $this->assertCount(0, $errorChild->getErrors(), $errorName . ' should not have an error, but has one');
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $child->getErrors(), $childName . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $child->getErrors(), $childName . ' should have an error, but has none');
$this->assertFalse($grandChild->hasErrors(), $grandChildName . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChildName . ' should not have an error, but has one');
} elseif (self::LEVEL_1B === $target) { } elseif (self::LEVEL_1B === $target) {
$this->assertEquals(array($this->getFormError()), $errorChild->getErrors(), $errorName . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $errorChild->getErrors(), $errorName . ' should have an error, but has none');
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertFalse($child->hasErrors(), $childName . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $childName . ' should not have an error, but has one');
$this->assertFalse($grandChild->hasErrors(), $grandChildName . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChildName . ' should not have an error, but has one');
} else { } else {
$this->assertFalse($errorChild->hasErrors(), $errorName . ' should not have an error, but has one'); $this->assertCount(0, $errorChild->getErrors(), $errorName . ' should not have an error, but has one');
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertFalse($child->hasErrors(), $childName . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $childName . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none');
} }
} }
@ -1449,15 +1449,15 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
if (self::LEVEL_0 === $target) { if (self::LEVEL_0 === $target) {
$this->assertEquals(array($this->getFormError()), $parent->getErrors(), $parent->getName() . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $parent->getErrors(), $parent->getName() . ' should have an error, but has none');
$this->assertFalse($child->hasErrors(), $childName . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $childName . ' should not have an error, but has one');
$this->assertFalse($grandChild->hasErrors(), $grandChildName . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChildName . ' should not have an error, but has one');
} elseif (self::LEVEL_1 === $target) { } elseif (self::LEVEL_1 === $target) {
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $child->getErrors(), $childName . ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $child->getErrors(), $childName . ' should have an error, but has none');
$this->assertFalse($grandChild->hasErrors(), $grandChildName . ' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChildName . ' should not have an error, but has one');
} else { } else {
$this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName() . ' should not have an error, but has one');
$this->assertFalse($child->hasErrors(), $childName . ' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $childName . ' should not have an error, but has one');
$this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none'); $this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none');
} }
} }

View File

@ -231,12 +231,12 @@ class SimpleFormTest extends AbstractFormTest
{ {
$this->form->addError(new FormError('Error!')); $this->form->addError(new FormError('Error!'));
$this->assertTrue($this->form->hasErrors()); $this->assertCount(1, $this->form->getErrors());
} }
public function testHasNoErrors() public function testHasNoErrors()
{ {
$this->assertFalse($this->form->hasErrors()); $this->assertCount(0, $this->form->getErrors());
} }
/** /**