symfony/symfony — pull request #12018 by webmozart (2014-09-24 14:53:49+00:00)
Title: [Form] Changed default value of "required" option to false

| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | ?
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -

This is an attempt to set the default value of the "required" option of all form types to false. Let's list the usage scenarios that we have at the moment:

validation constraints | "required" value | HTML5 attribute
--- | --- | ---
`NotNull`/`NotBlank` | none | `required`
none | none | `required`
`NotNull`/`NotBlank` | `true` | `required`
none | `true` | `required`
`NotNull`/`NotBlank` | `false` | none
none | `false` | none

By setting the default of the "required" option to false, only one case changes:

validation constraints | "required" value | HTML5 attribute
--- | --- | ---
none | none | none

This obviously makes more sense than validating a value on the client side that isn't validated on the server side.

**Consequences**

* Text fields without explicit "required" option and without server-side validation won't have the `required` attribute anymore
* Select fields without explicit "required" option and without server-side validation will display an empty value by default
* Unit tests break if the tested behavior depends on the "required" option and that option is not explicitly set
* We need to inspect the "constraints" option to check whether it contains a `NotNull`/`NotBlank` constraint. In that case, "required" should be set to true by default

The only major consequence I see is if people don't use the Validator component for validation, but some custom mechanism. In that case, we can't detect whether a field is validated or not, so all fields without explicit "required" option loose their HTML attribute. However:

1. I'm not aware of anybody using the Form component without the Validator component.
2. This affects client-side validation only, not server-side validation.

So even there, I don't think the consequences are very serious.

What do you think?

**Todos** (in case a majority votes for this PR)

- [ ] Inspect "constraints" option to set "required" option to true when `NotBlank`/`NotNull` is given