[Validator] Add attributes documentation of composite constraints #15541
Conversation
2c2bfe1 to
07a39ed
Compare
|
Thanks for this Alexander! I have two comments: Nested attribute code syntaxIs there a commonly used syntax for this? I'm asking because instead of this: #[
Assert\All([
new Assert\NotBlank,
new Assert\Length(min: 5),
])
]
protected $favoriteColors = [];I'd prefer to use this: #[Assert\All([
new Assert\NotBlank,
new Assert\Length(min: 5),
])]
protected $favoriteColors = [];But this is just a personal preference, so let's ping the @symfony/team-symfony-docs to ask for their opinion. Warning message about needing PHP 8.1 to run this codeThat's a good question and I don't have a good answer. Maybe we can add a comment just before each class like the following? .. code-block:: php-attributes
// src/Localization/Place.php
namespace App\Localization;
use App\Validator\Constraints as AcmeAssert;
use Symfony\Component\Validator\Constraints as Assert;
// IMPORTANT: the code of this class requires using PHP 8.1 or higher
class Place
{
// ...
}But I'm not sure ... so let's ping @symfony/team-symfony-docs again to ask what do they think. Thanks! |
| new Assert\Length(min: 10), | ||
| ]) | ||
| ] | ||
| protected $password; |
There was a problem hiding this comment.
We should rename this to plainPassword, in nearly every application, this should be a hash, which does not need to be validated 🤔
There was a problem hiding this comment.
You're right, making the change!
|
Thank you for your feedback @javiereguiluz! About the code syntax, I have to say that I don't particularly have an opinion either. I based myself on code written by @derrabus, where you can find this in some tests of the framework: #[
Assert\NotNull,
Assert\Range(min: 3),
]
public $firstName;About the warning message for PHP 8.1, for now I'll add the comment like you did and we'll see if a better solution appears. |
07a39ed to
ed56590
Compare
…e-daubois) This PR was merged into the 5.4 branch. Discussion ---------- [Validator] Add support of nested attributes | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #38503 | License | MIT | Doc PR | symfony/symfony-docs#15541 Although the RFC (https://wiki.php.net/rfc/new_in_initializers) is in the voting phase (until 14 July), it is already well on its way to passing. Based on `@nikic`'s development (php/php-src#7153), this makes the development of support possible. It will obviously take a little while before this pull request is merged. If this pull request is OK for you, I'll get to work on writing the existing documentation for the attribute validation constraints.  Not sure about the Symfony version to target, as `AtLeastOneOf` has been introduced in 5.1. Although, I couldn't find attributes validation documentation for 4.4. Commits ------- 1449450 [Validator] Add support of nested attributes for composite constraints
…e-daubois) This PR was merged into the 5.4 branch. Discussion ---------- [Validator] Add support of nested attributes | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #38503 | License | MIT | Doc PR | symfony/symfony-docs#15541 Although the RFC (https://wiki.php.net/rfc/new_in_initializers) is in the voting phase (until 14 July), it is already well on its way to passing. Based on `@nikic`'s development (php/php-src#7153), this makes the development of support possible. It will obviously take a little while before this pull request is merged. If this pull request is OK for you, I'll get to work on writing the existing documentation for the attribute validation constraints.  Not sure about the Symfony version to target, as `AtLeastOneOf` has been introduced in 5.1. Although, I couldn't find attributes validation documentation for 4.4. Commits ------- 1449450e2f [Validator] Add support of nested attributes for composite constraints
|
I think this can be merged now or not? |
ed56590 to
7099e3f
Compare
|
You're right @malteschlueter. Thank you @alexandre-daubois! I've slightly updated your changes, to use the CS proposed by Javier and to add versionadded directives below the code examples (to emphasize the fact that these attributes are not supported in 5.3, which is different for all other constraint). |
Related pull-request: symfony/symfony#41994
Not really sure how to precise it only works for attributes in PHP 8.1, not 8.0. If you have a solution, please let me know!