Skip to content

[Form] Created FormErrorBag #9099

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

Closed
wants to merge 3 commits into from
Closed
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?php if ($errors): ?>
<?php if (count($errors)): ?>
<ul>
<?php foreach ($errors as $error): ?>
<li><?php echo $error->getMessage() ?></li>
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ CHANGELOG
2.5.0
------

* [BC BREAK] changed getErrors() to return a FormErrorBag, which is countable,
traversable and can be converted to a string. This breaks BC if the return
value is used in array function like is_array().
* deprecated getErrorsAsString() in favor of converting getErrors() to a
string

* added an option for multiple files upload

2.4.0
Expand Down
55 changes: 27 additions & 28 deletions src/Symfony/Component/Form/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ class Form implements \IteratorAggregate, FormInterface
private $children;

/**
* The errors of this form
* @var FormError[] An array of FormError instances
* The errors of this form.
*
* @var FormErrorBag
*/
private $errors = array();
private $errors;

/**
* Whether this form was submitted
Expand Down Expand Up @@ -172,6 +173,7 @@ public function __construct(FormConfigInterface $config)

$this->config = $config;
$this->children = new OrderedHashMap();
$this->errors = new FormErrorBag();
}

public function __clone()
Expand Down Expand Up @@ -501,7 +503,7 @@ public function submit($submittedData, $clearMissing = true)

// Initialize errors in the very beginning so that we don't lose any
// errors added during listeners
$this->errors = array();
$this->errors = new FormErrorBag();

// Obviously, a disabled form should not change its data upon submission.
if ($this->isDisabled()) {
Expand Down Expand Up @@ -675,7 +677,7 @@ public function addError(FormError $error)
if ($this->parent && $this->config->getErrorBubbling()) {
$this->parent->addError($error);
} else {
$this->errors[] = $error;
$this->errors->addError($error);
}

return $this;
Expand Down Expand Up @@ -776,37 +778,34 @@ public function getClickedButton()
*/
public function getErrors()
{
return $this->errors;
}

/**
* Returns a string representation of all form errors (including children errors).
*
* This method should only be used to help debug a form.
*
* @param integer $level The indentation level (used internally)
*
* @return string A string representation of all errors
*/
public function getErrorsAsString($level = 0)
{
$errors = '';
foreach ($this->errors as $error) {
$errors .= str_repeat(' ', $level).'ERROR: '.$error->getMessage()."\n";
}
$errors = clone $this->errors;

foreach ($this->children as $key => $child) {
$errors .= str_repeat(' ', $level).$key.":\n";
if ($child instanceof self && $err = $child->getErrorsAsString($level + 4)) {
$errors .= $err;
} else {
$errors .= str_repeat(' ', $level + 4)."No errors\n";
$childrenErrors = $child->getErrors();

// for BC
if (is_array($childrenErrors)) {
$_collection = new FormErrorBag();
foreach ($childrenErrors as $error) {
$_collection->addError($error);
}
$childrenErrors = $_collection;
}

$errors->addCollection($key, $childrenErrors);
}

return $errors;
}

/**
* @deprecated Deprecated since version 2.5, to be removed in 3.0. Convert {@link getErrors()} to a string instead.
*/
public function getErrorsAsString($level = 0)
{
return $this->getErrors()->__toString($level);
}

/**
* {@inheritdoc}
*/
Expand Down
241 changes: 241 additions & 0 deletions src/Symfony/Component/Form/FormErrorBag.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Form;

use Symfony\Component\Form\Exception\InvalidArgumentException;

/**
* A bag of Form Errors.
*
* @author Wouter J <wouter@wouterj.nl>
*
* @since v2.5
*/
class FormErrorBag implements \RecursiveIterator, \Countable, \ArrayAccess
{
/**
* @var array An array of FormError and FormErrorBag instances
*/
protected $errors = array();

private $formName;

public function setFormName($name)
{
$this->formName = $name;
}

/**
* Adds a new form error.
*
* @param FormError $error
*/
public function addError(FormError $error)
{
$this->errors[] = $error;
}

/**
* Adds a new form error collection.
*
* @param string $formName
* @param FormErrorBag $collection
Copy link
Member

Choose a reason for hiding this comment

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

FormErrorBag|array

*/
public function addCollection($formName, $collection)
{
$collection->setFormName($formName);

$this->errors[$formName] = $collection;
}

public function current()
{
$current = current($this->errors);

if (!$current instanceof FormError) {
$this->next();

if ($this->valid()) {
$current = $this->current();
}
}

return $current;
}

public function key()
{
return isset($this->formName) ? $this->formName : key($this->errors);
}

public function next()
{
return next($this->errors);
}

public function rewind()
{
reset($this->errors);
}

public function valid()
{
while (current($this->errors) instanceof FormErrorBag) {
$this->next();

if (!$this->valid()) {
return false;
}
}

return null !== key($this->errors);
}

/**
* {@inheritDoc}
*/
public function hasChildren()
{
return current($this->errors) instanceof FormErrorBag;
}

/**
* {@inheritDoc}
*/
public function getChildren()
{
return current($this->errors);
}

public function count()
{
$count = 0;

foreach ($this->errors as $error) {
if ($error instanceof FormError) {
$count++;
}
}

return $count;
}

/**
* Counts all errors, including errors from children.
*
* @return int
*/
public function countAll()
{
$count = 0;

foreach ($this->errors as $error) {
if ($error instanceof FormErrorBag) {
$count += $error->countAll();
} else {
$count++;
}
}

return $count;
}

public function get($offset)
{
return $this->errors[$offset];
}

public function set($offset, $value)
{
$this->errors[$offset] = $value;
}

public function has($offset)
{
return isset($this->errors[$offset]);
}

public function all()
{
return $this->errors;
}

public function clear()
{
$this->replace();
}

public function remove($offset)
{
unset($this->errors[$offset]);
}

public function replace(array $errors = array())
{
$this->errors = $errors;
}

public function isEmpty()
{
return empty($this->errors);
}

public function keys()
{
return array_keys($this->errors);
}

public function __toString()
Copy link
Contributor

Choose a reason for hiding this comment

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

missing phpdoc for many methods

Copy link
Member Author

Choose a reason for hiding this comment

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

do we really want to have PHPdoc for PHP build-in methods, like __toString etc?

Copy link
Member

Choose a reason for hiding this comment

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

no

Copy link
Contributor

Choose a reason for hiding this comment

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

yes because otherwise it's not documented what the string representation actually returns

Copy link
Contributor

Choose a reason for hiding this comment

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

and this is one of the main points of FormErrorBag

Copy link
Contributor

Choose a reason for hiding this comment

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

also its not doccumented that it accepts a parameter

Copy link
Member Author

Choose a reason for hiding this comment

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

the string representation wasn't documented on getErrorsAsString() too, I don't really know why we should do it here. And the parameter is only for internal use.

Copy link
Contributor

Choose a reason for hiding this comment

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

where is the parameter used internally?

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 used to indent the children forms with 4 spaces

Copy link
Contributor

Choose a reason for hiding this comment

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

ok as long as its not used outside the class

{
$level = func_num_args() > 0 ? func_get_arg(0) : 0;
$errors = '';

foreach ($this->errors as $key => $error) {
if ($error instanceof self) {
$errors .= str_repeat(' ', $level).$key.":\n";
if ($err = $error->__toString($level + 4)) {
$errors .= $err;
} else {
$errors .= str_repeat(' ', $level + 4)."No errors\n";
}
} else {
$errors .= str_repeat(' ', $level).'ERROR: '.$error->getMessage()."\n";
}
}

return $errors;
}

public function offsetExists($offset)
{
return $this->has($offset) && $this->errors[$offset] instanceof FormError;
}

public function offsetGet($offset)
{
$error = $this->get($offset);

if ($error instanceof FormError) {
return $error;
}
}

public function offsetSet($offset, $value)
{
return $this->set($offset);
}

public function offsetUnset($offset)
{
return $this->remove($offset);
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/Form/FormInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public function all();
/**
* Returns all errors.
*
* @return FormError[] An array of FormError instances that occurred during validation
* @return FormErrorBag
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't there be an interface? since there is an interface for everything else

Copy link
Member Author

Choose a reason for hiding this comment

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

Ping @bschussek

Copy link
Contributor

Choose a reason for hiding this comment

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

hm there was no interface for FormError before as well. so it might no be needed because it would complicate things

*/
public function getErrors();

Expand Down
Loading