Skip to content

[Validator] Set Length::$allowEmptyString to false when a NotBlank contraint is defined #34081

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
Oct 23, 2019

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Oct 23, 2019

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Since #31528, we are told to do this kind of changes to our entities:

     /**
      * @Assert\NotBlank()
-     * @Assert\Length(min="10")
+     * @Assert\Length(min="10", allowEmptyString=false)
      */
     public $description;

But the NotBlank already says it - this is just boilerplate. More critically, this also means we cannot write annotations that are compatible with 3.4, 4.4 and 5.0 at the same time, making FC/BC hard.

By setting Length::$allowEmptyString to false when a NotBlank contraint is defined, we fix both issues.

/cc @ogizanagi

Copy link
Contributor

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

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

That makes sense 👍

nicolas-grekas added a commit that referenced this pull request Oct 23, 2019
…NotBlank contraint is defined (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Validator] Set Length::$allowEmptyString to false when a NotBlank contraint is defined

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Since #31528, we are told to do this kind of changes to our entities:
```diff
     /**
      * @Assert\NotBlank()
-     * @Assert\Length(min="10")
+     * @Assert\Length(min="10", allowEmptyString=false)
      */
     public $description;
```

But the `NotBlank` already says it - this is just boilerplate. More critically, this also means we cannot write annotations that are compatible with 3.4, 4.4 and 5.0 at the same time, making FC/BC hard.

By setting `Length::$allowEmptyString` to `false` when a `NotBlank` contraint is defined, we fix both issues.

/cc @ogizanagi

Commits
-------

840f7e7 [Validator] Set Length::$allowEmptyString to false when a NotBlank contraint is defined
@nicolas-grekas nicolas-grekas merged commit 840f7e7 into symfony:4.4 Oct 23, 2019
@nicolas-grekas nicolas-grekas deleted the v-zero-len branch October 23, 2019 12:17
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
nicolas-grekas added a commit that referenced this pull request Nov 21, 2019
…as-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Form] group constraints when calling the validator

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Follow up of #34081
Spotted during the workshop at SymfonyCon, while trying to fix deprecation notices on symfony-demo:
the Form component currently passes constraints one by one for validation, effectively preventing the validator from taking care of cross-constraints dependencies.

This PR fixes it.

This will prevent ppl from having to fix things like
> Using the "Symfony\Component\Validator\Constraints\Length" constraint with the "min" option without setting the "allowEmptyString" one is deprecated and defaults to true. In 5.0, it will become optional and default to false.

Commits
-------

d15f77f [Form] group constraints when calling the validator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants