Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Validator] Improve constraint default option check #30737

Merged

Conversation

vudaltsov
Copy link
Contributor

@vudaltsov vudaltsov commented Mar 28, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Any constraint without default option used as annotation with unnamed first argument (for example, @Assert\Collection(1)) throws an exception with an ugly message The options "" do not exist in constraint Collection.

This PR makes constraint check the default option in the annotation case in the same way it checks it in the "real" code case. So the exception will be No default option is configured for constraint Collection.

@vudaltsov vudaltsov changed the title [Validator] Fix constraint annotation undefined default option checking [Validator] Improve constraint default option check Mar 28, 2019
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Mar 28, 2019
fabpot added a commit that referenced this pull request Mar 29, 2019
…(vudaltsov)

This PR was squashed before being merged into the 3.4 branch (closes #30736).

Discussion
----------

[Validator] Fix annotation default for @count and @Length

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | is it worth mentioning?

`Count` and `Length` constraints allow to pass the exact value as the constructor argument when used in code.

```php
new Length(5);
// is same as
new Length(['min' => 5, 'max' => 5]);
```

At the same time when using them as annotations, `@Assert\Length(5)` throws `The options "" do not exist in constraint Symfony\\Component\\Validator\\Constraints\\Length` (fix for ugly exception is proposed in #30737). This happens because annotation's default value is passed as `value`. Since `Length` does not have a default option, `value` is replaced with `''`.

This PR fixes this inconsistency.

Commits
-------

7bfb8c1 [Validator] Fix annotation default for @count and @Length
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comment to be fixed.

src/Symfony/Component/Validator/Constraint.php Outdated Show resolved Hide resolved
@vudaltsov
Copy link
Contributor Author

@fabpot , done. Also fixed other messages in the Constraint class.

@fabpot fabpot force-pushed the constraint-annotation-undefined-default-option branch from b04345d to 915912e Compare March 29, 2019 17:25
@fabpot
Copy link
Member

fabpot commented Mar 29, 2019

Thank you @vudaltsov.

@fabpot fabpot merged commit 915912e into symfony:3.4 Mar 29, 2019
fabpot added a commit that referenced this pull request Mar 29, 2019
…tsov)

This PR was squashed before being merged into the 3.4 branch (closes #30737).

Discussion
----------

[Validator] Improve constraint default option check

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Any constraint without default option used as annotation with unnamed first argument (for example, `@Assert\Collection(1)`) throws an exception with an ugly message `The options "" do not exist in constraint Collection`.

This PR makes constraint check the default option in the annotation case in the same way it checks it in the "real" code case. So the exception will be `No default option is configured for constraint Collection.`

Commits
-------

915912e [Validator] Improve constraint default option check
@vudaltsov vudaltsov deleted the constraint-annotation-undefined-default-option branch April 1, 2019 15:13
This was referenced Apr 2, 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.

None yet

4 participants