Skip to content

Conversation

@fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Oct 3, 2019

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

$this->context->getGroup() returns string or null.

ContextualValidatorInterface::validate() 3rd argument accepts an array but it must not contain null (its contract doesn't allow it). If it does, it will fail in RecursiveContextualValidator::validateInGroup() for example because of the string scalar type on the $group argument. (on 4.4)

Note that in our "common" usage of the Valid constraint, the group in its validator will never be null because this constraint has a special treatment. However, if this validator is reused in a custom way, it can fail.

@xabbuh
Copy link
Member

xabbuh commented Oct 7, 2019

Thank you @fancyweb.

xabbuh added a commit that referenced this pull request Oct 7, 2019
…yweb)

This PR was merged into the 3.4 branch.

Discussion
----------

[Validator] Fix ValidValidator group cascading usage

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

`$this->context->getGroup()` returns `string` or `null`.

`ContextualValidatorInterface::validate()` 3rd argument accepts an array but it must not contain `null` (its contract doesn't allow it). If it does, it will fail in `RecursiveContextualValidator::validateInGroup()` for example because of the `string` scalar type on the `$group` argument. (on 4.4)

Note that in our "common" usage of the `Valid` constraint, the group in its validator will never be `null` because this constraint has a special treatment. However, if this validator is reused in a custom way, it can fail.

Commits
-------

72684b0 [Validator] Fix ValidValidator group cascading usage
@xabbuh xabbuh merged commit 72684b0 into symfony:3.4 Oct 7, 2019
@fancyweb fancyweb deleted the validator-valid-validator-group branch October 7, 2019 09:40
This was referenced Oct 7, 2019
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