Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Validator] add Validation::createCallable()#31466
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You should remove the two unrelated commits
1edd0a0 to94d121fCompareUpdated it, guess my fork got out of sync |
instead of a new CallableValidator, which isn't really a validator from the component pov (as it's not a true ValidatorInterface implementation) ... what about |
janvernieuwe commentedJun 7, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@ro0NL that can work too, it should be pretty easy as something like this in the Validation class: publicstaticfunctioncreateCallable(Constraint...$constraints):callable {$validator =self::createValidator();returnstaticfunction ($value)use ($constraints,$validator) {$violations =$validator->validate($value,$constraints);if (0 !==$violations->count()) {thrownewLogicException((string)$violations); }return$value; }; } Don't know if it is desirable to create a new Validator instance for each new callable though, but it shouldn't be too bad. |
6fe4e80 to015b0c8CompareFailures on travis don't seem to be related to any of my code changes as far as i can see... |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| return static function ($value) use ($constraints, $validator) { | ||
| $violations = $validator->validate($value, $constraints); | ||
| if (0 !== $violations->count()) { | ||
| throw new LogicException((string) $violations); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This should be a RuntimeException to me, or better: a class derived from RuntimeException modelled after ValidationFailedException from the Messenger component. WDYTN
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Added a new exception for this, let me know if you want to change the naming or anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nice feature 👍
can you please rebase+retarget for master? |
I won't immediately have time for retargetting. So if it can be done on the merger's side, that'd be great! |
a77379c to2e4f2acCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
(I rebased the PR for master)
Thank you@janvernieuwe. |
…euwe)This PR was merged into the 5.1-dev branch.Discussion----------[Validator] add Validation::createCallable()| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |This is an initial PR to check/discuss the implementation of a callable validator.If there is interest in merging this, I will gladly update the docs and such.The use case is mainly for validation of console questions, since the default validation has been removed in the latest version and i could not find an easy solution to replace it (if there already is some solution for this, i'm not aware of it) and the question helper uses callables.This small class allows the standard symfony validators to be used in console questions, or any other location that requires a callable validator.Example use case:```php$io = new SymfonyStyle($input, $output);$required = new CallableValidator([new NotBlank()]);$wsdl = $io->ask('Wsdl location (URL or path to file)', null, $required);```As said before, this is by no means the final version, but I would like to know if there is interest in merging this (and receive some feedback about the implementation) before I put any more effort into this.Commits-------2e4f2ac [Validator] add Validation::createCallable()I see an issue here: the callable creation always creates a new Validator object instead of allowing to use an existing one. This makes it incompatible with the DI integration, and so forbids using any constraint validator which has some dependency. |
@stof good catch. What's your reco here? Revert? Improving? (in this case, could you work on a PR?) |
Well, the variadic signature makes it hard to improve, as we cannot add an optional parameter to pass a validator instance. |
My proposal: change the constraints from a variadic argument to an array argument (or maybe |
Btw, this is the main reason why I don't like variadic arguments: they lock the signature entirely. |
…eCallable() (nicolas-grekas)This PR was merged into the 5.1 branch.Discussion----------[Validator] allow passing a validator to Validation::createCallable()| Q | A| ------------- | ---| Branch? | 5.1| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -As spotted by@stof in#31466 (comment)Commits-------1357cbf [Validator] allow passing a validator to Validation::createCallable()
…eCallable() (nicolas-grekas)This PR was merged into the 5.1 branch.Discussion----------[Validator] allow passing a validator to Validation::createCallable()| Q | A| ------------- | ---| Branch? | 5.1| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -As spotted by@stof insymfony/symfony#31466 (comment)Commits-------1357cbf8ed [Validator] allow passing a validator to Validation::createCallable()
This PR was merged into the 5.1 branch.Discussion----------Fix tests namespaces| Q | A| ------------- | ---| Branch? | 5.1 <!-- see below -->| Bug fix? | no| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets | N/A <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->| License | MIT| Doc PR | N/AFixes namespaces for tests introduced in#34869 竪, spotted by travis generating Composer warnings:```Deprecation Notice: Class Symfony\Component\Console\Tests\Output\NullOutputFormatterStyleTest located in ./src/Symfony/Component/Console/Tests/Formatter/NullOutputFormatterStyleTest.php does not comply with psr-4 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///home/travis/.phpenv/versions/7.4.9/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201Deprecation Notice: Class Symfony\Component\Console\Tests\Output\NullOutputFormatterTest located in ./src/Symfony/Component/Console/Tests/Formatter/NullOutputFormatterTest.php does not comply with psr-4 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///home/travis/.phpenv/versions/7.4.9/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201Deprecation Notice: Class Symfony\Component\Validator\Tests\Validator\ValidationTest located in ./src/Symfony/Component/Validator/Tests/ValidationTest.php does not comply with psr-4 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///home/travis/.phpenv/versions/7.4.9/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:201```Commits-------4e68c90 Fix tests namespaces
Uh oh!
There was an error while loading.Please reload this page.
This is an initial PR to check/discuss the implementation of a callable validator.
If there is interest in merging this, I will gladly update the docs and such.
The use case is mainly for validation of console questions, since the default validation has been removed in the latest version and i could not find an easy solution to replace it (if there already is some solution for this, i'm not aware of it) and the question helper uses callables.
This small class allows the standard symfony validators to be used in console questions, or any other location that requires a callable validator.
Example use case:
As said before, this is by no means the final version, but I would like to know if there is interest in merging this (and receive some feedback about the implementation) before I put any more effort into this.