Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Validator] Add Validation::createIsValidCallable() that returns a boolean instead of exception #40240

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 12, 2021
Merged

[Validator] Add Validation::createIsValidCallable() that returns a boolean instead of exception #40240

merged 1 commit into from
Mar 12, 2021

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Feb 18, 2021 •
edited
Loading

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #36820
License MIT
Doc PR tbd

This adds a Validator::createValidCallable() (I'm very open for other name suggestions) that returns a boolean instead of exceptions. This allows usingit in places where booleans are expected, for instance in the referenced OptionsResolver case:

$resolver->setAllowedValues('name', Validation::createValidCallable(
    new Assert\Length(['min' => 10 ])
));

@wouterj wouterj requested a review from yceruto as a code owner February 18, 2021 13:57
@carsonbot carsonbot changed the title [OptionsResolver] Support Validation::createClosure() as allowed value closure Support Validation::createClosure() as allowed value closure Feb 18, 2021
@wouterj wouterj mentioned this pull request Feb 18, 2021
Closed
@wouterj wouterj force-pushed the issue-36820/options-resolver-validation branch from 61fd825 to 7bd6fcb Compare February 18, 2021 14:00
@wouterj wouterj mentioned this pull request Feb 18, 2021
Open
@jvasseur
Copy link
Contributor

jvasseur commented Feb 18, 2021 •
edited
Loading

I'm not fond of creation a special case in the option resolver component like this.

Maybe we could instead create a new Validation::createValidCallable (or another name) method that returns a boolean instead of throwing which could be useful for more cases that just passing it to the option resolver

@stof
Copy link
Member

stof commented Feb 18, 2021

I tend to agree with @jvasseur here. That sounds like a better approach

@yceruto
Copy link
Member

yceruto commented Feb 18, 2021

I agree with @jvasseur too.

@wouterj
Copy link
Member Author

wouterj commented Feb 18, 2021

I agree, I'll update the PR later today.

@wouterj wouterj force-pushed the issue-36820/options-resolver-validation branch from 7bd6fcb to 73a717e Compare February 18, 2021 16:45
@carsonbot carsonbot changed the title Support Validation::createClosure() as allowed value closure [Validator] Support Validation::createClosure() as allowed value closure Feb 18, 2021
@wouterj wouterj changed the title [Validator] Support Validation::createClosure() as allowed value closure [Validator] Add Validation::createValidClosure() that returns a boolean instead of exception Feb 18, 2021
@wouterj
Copy link
Member Author

wouterj commented Feb 18, 2021

PR updated. Thanks @jvasseur for the great suggestion!

@nicolas-grekas nicolas-grekas added this to the 5.x milestone Feb 18, 2021
@jvasseur
Copy link
Contributor

@javiereguiluz I would expect a function named isValid to directly do the validation and return a bool instead of creating a callable.

@javiereguiluz
Copy link
Member

@jvasseur you are right! I mixed things up ... as usually happens to me with the validator. Sorry! I deleted the original comment.

@wouterj wouterj force-pushed the issue-36820/options-resolver-validation branch 3 times, most recently from c51a2f3 to 565bc52 Compare March 6, 2021 23:14
@wouterj wouterj force-pushed the issue-36820/options-resolver-validation branch from 565bc52 to abdedda Compare March 7, 2021 11:16
@wouterj wouterj force-pushed the issue-36820/options-resolver-validation branch from abdedda to e731f5f Compare March 7, 2021 12:57
@wouterj wouterj changed the title [Validator] Add Validation::createValidClosure() that returns a boolean instead of exception [Validator] Add Validation::createIsValidCallable() that returns a boolean instead of exception Mar 7, 2021
@fabpot
Copy link
Member

fabpot commented Mar 12, 2021

Thank you @wouterj.

@fabpot fabpot merged commit 6c0102c into symfony:5.x Mar 12, 2021
@fabpot fabpot mentioned this pull request Mar 12, 2021
Closed
@wouterj wouterj deleted the issue-36820/options-resolver-validation branch March 12, 2021 09:01
@fabpot fabpot mentioned this pull request Apr 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@stof stof stof left review comments

@ro0NL ro0NL ro0NL left review comments

@fabpot fabpot fabpot approved these changes

@yceruto yceruto yceruto approved these changes

Assignees
No one assigned
Labels
Feature Status: Reviewed Validator
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.

[RFC][OptionsResolver] Constraints
9 participants
@wouterj @jvasseur @stof @yceruto @javiereguiluz @fabpot @nicolas-grekas @ro0NL @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.