Skip to content

[Validator] Ignore validation of ExpressionLanguageSyntax against empty string #41904

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
Closed
wants to merge 1 commit into from
Closed

[Validator] Ignore validation of ExpressionLanguageSyntax against empty string #41904

wants to merge 1 commit into from

Conversation

jderusse
Copy link
Member

@jderusse jderusse commented Jun 29, 2021 •
edited
Loading

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

I have the following object

class Foo
{
  #[Assert\ExpressionLanguageSyntax(allowedVariables: ['x', 'y'])]
  public ?string $exp = null;
}

When exp is null, the validator yield an UnexpectedValueException because null is not a string.

A workaround would be to wrap the constrain in a AtLeastOneOf.

class Foo
{
    /**
     * @Assert\AtLeastOneOf({
     *     @Assert\Blank(),
     *     @Assert\ExpressionLanguageSyntax(allowedVariables={"x", "y"}),
     * })
     */
    public ?string $exp = null;
}

But it's not nice:

  • it does not work with php8 attributes
  • the error message displayed to the user is now wrap into "This value should satisfy at least one of the following constraints ... "

This PR ignore validation when the input is null or empty (as we do for many other validators (datetime, bic, ...))

@jderusse jderusse added this to the 5.2 milestone Jun 29, 2021
@carsonbot carsonbot changed the title Ignore validation of ExpressionLanguageSyntax against empty string [Validator] Ignore validation of ExpressionLanguageSyntax against empty string Jun 29, 2021
@HeahDude
Copy link
Contributor

Duplicate of #41329?

@jderusse
Copy link
Member Author

Duplicate of #41329?

Thank you! indeed, I missed this PR ..

Thinking about it, #41329 is a new feature for 5.4 that adds a new setting to allow passing null.

This PR is a bug fix for 5.2 that always allow null (like many other validators)

Both PR fixes the same issue. I let other members of the team team decide which one they want. Feel free to close this PR

@derrabus
Copy link
Member

@jderusse The other PR started like yours. The deprecation has been introduced after my comment. #41329 (comment)

@derrabus
Copy link
Member

Let‘s not have two competing PRs for the same issue.

@derrabus derrabus closed this Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Status: Needs Review Validator
Projects
None yet
Milestone
5.2
Development

Successfully merging this pull request may close these issues.
4 participants
@jderusse @HeahDude @derrabus @carsonbot