Skip to content

[Validator] Remove property and method targets from the optional and required constraints #11014

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

Merged
merged 1 commit into from
May 31, 2014

Conversation

jakzal
Copy link
Contributor

@jakzal jakzal commented May 29, 2014

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #11013
License MIT
Doc PR

At the moment both constraints can only be defined on other annotations. Both constraints are only mentioned in the docs in the context of the Collection.

Defining the required or optional annotation directly on a field or method
throws a ClassNotFoundException, since the constraint validator factory tries to load the validator (which does not exist):

ClassNotFoundException: Attempted to load class "OptionalValidator"
from namespace "Symfony\Component\Validator\Constraints" 
in /var/www/server/vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Validator/ConstraintValidatorFactory.php line 71. 
Do you need to "use" it from another namespace?

By applying this patch the end user will get a more helpful error message:

[Semantical Error] Annotation @Assert\Optional is not allowed to be declared on property Acme\DemoBundle\Entity\Contact::$message. 
You may only use this annotation on these code elements: ANNOTATION.

…required constraints.

At the moment both constraints can only be defined on other annotations (specifically, the Collection annotation). Defining the required or optional annotation directly on a field or method throws a ClassNotFoundException, since the constraint validator factory tries to load the validator (which does not exist).
@stof
Copy link
Member

stof commented May 30, 2014

👍

@fabpot
Copy link
Member

fabpot commented May 31, 2014

Good catch, thanks @jakzal.

@fabpot fabpot merged commit 9c2616e into symfony:2.3 May 31, 2014
fabpot added a commit that referenced this pull request May 31, 2014
…tional and required constraints (jakzal)

This PR was merged into the 2.3 branch.

Discussion
----------

[Validator] Remove property and method targets from the optional and required constraints

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |  #11013
| License       | MIT
| Doc PR        |

At the moment both constraints can only be defined on other annotations. Both constraints are [only mentioned in the docs in the context of the Collection](http://symfony.com/doc/current/reference/constraints/Collection.html#required-and-optional-field-constraints).

Defining the required or optional annotation directly on a field or method
throws a ClassNotFoundException, since the constraint validator factory tries to load the validator (which does not exist):

```
ClassNotFoundException: Attempted to load class "OptionalValidator"
from namespace "Symfony\Component\Validator\Constraints"
in /var/www/server/vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Validator/ConstraintValidatorFactory.php line 71.
Do you need to "use" it from another namespace?
```

By applying this patch the end user will get a more helpful error message:

```
[Semantical Error] Annotation @Assert\Optional is not allowed to be declared on property Acme\DemoBundle\Entity\Contact::$message.
You may only use this annotation on these code elements: ANNOTATION.
```

Commits
-------

9c2616e [Validator] Remove property and method targets from the optional and required constraints.
@jakzal jakzal deleted the optional-required-target-fix branch May 31, 2014 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants