[2.5][Form] solved dependency to ValidatorInterface, fix #11036 #11350
Conversation
| @@ -35,9 +37,23 @@ public static function getSubscribedEvents() | |||
| return array(FormEvents::POST_SUBMIT => 'validateForm'); | |||
| } | |||
|
|
|||
| public function __construct(ValidatorInterface $validator, ViolationMapperInterface $violationMapper) | |||
| /** | |||
| * @param ValidatorInterface $validator The validator requires an instance of ValidatorInterface | |||
There was a problem hiding this comment.
should be ValidatorInterface|LegacyValidatorInterface
|
@stof thank you for your feedback, I will update the code. I mixed the comments with symfony version and symfony validator api version. |
|
My suggestion for the new phpdoc comment is now /**
* @param ValidatorInterface|LegacyValidatorInterface validator The validator requires an instance of ValidatorInterface
* since validator apiVersion 2.5 instance of {@link Symfony\Component\Validator\Validator\ValidatorInterface}
* until validator apiVersion 2.4 instance of {@link Symfony\Component\Validator\ValidatorInterface}
*/Is this comment now clear enough? do you have a better wording? |
|
@sebastianblum I would just remove the param description entirely and keep only |
|
@stof I removed the param description, you are right, it is enough |
| { | ||
| if (!$validator instanceof ValidatorInterface && !$validator instanceof LegacyValidatorInterface) { | ||
| throw new \InvalidArgumentException('Validator must be instance of ValidatorInterface.'); |
There was a problem hiding this comment.
the message should be changed to include the real interface names (as done in my previous comment)
|
I updated the Exception message |
| if ($validator instanceof ValidatorInterface) { | ||
| $this->validator = $validator; | ||
|
|
||
| /** @var \Symfony\Component\Validator\Mapping\ClassMetadata $metadata */ |
There was a problem hiding this comment.
Instead of repeating this, you can leave were it was before, and add use instead of FQCN.
There was a problem hiding this comment.
Also IIRC proper phpdoc here should be:
use Symfony\Component\Validator\Mapping\ClassMetadata;
/** @var $metadata ClassMetadata */There was a problem hiding this comment.
the order is /** @var ClassMetadata $metadata */
|
Apart from cs 👍 |
|
I don't understand why the test is failing can someone please help me |
|
@sebastianblum can you rebase? The issue was fixed in master. |
|
👍 |
|
@sebastianblum Can you rebase on master instead of merging? Thanks. |
|
I hope I did it the rebase correct, didn't I? |
|
yep, looks good. 👍 |
|
Isn't it something we need to merge in 2.5? |
|
@fabpot it is |
|
Yes, it is a bug fix for 2.5 and higher. |
|
@fabpot could this be merged to be included in the 2.5.2 release ? Given that the PR is ready, it would be a shame to keep it pending until after the release while it breaks things for some users wanting to use the 2.5 API only |
|
Yes, I'm going to merge it before next release. |
|
Thank you @sebastianblum. |
…11036 (Sebastian Blum) This PR was submitted for the master branch but it was merged into the 2.5 branch instead (closes #11350). Discussion ---------- [2.5][Form] solved dependency to ValidatorInterface, fix #11036 | Q | A | ------------- | --- | Bug fix? | [yes] | New feature? | [no] | BC breaks? | [no] | Deprecations? | [no] | Tests pass? | [yes] | Fixed tickets | #11036, #11345 | License | MIT | Doc PR | Since Symfony 2.5 The problem was that the form component has a hardcoded depencency to the deprecated validator component (api Version 2.4) The pull request fixes the dependency to the validator component and supports now both implementations, apiVersion 2.5 and apiVersion 2.4 of the validator component. @symfony Core Members please review the changes https://github.com/sebastianblum/symfony/blob/0a1e9c208f8730219bebf89f6696b246a0c88da7/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php I'm not sure if it was the right solution Commits ------- 705d67b [2.5][Form] solved dependency to ValidatorInterface, fix #11036
|
thank you @fabpot for merging. |
…11036 (Sebastian Blum) This PR was squashed before being merged into the 2.6-dev branch (closes #11350). Discussion ---------- [2.5][Form] solved dependency to ValidatorInterface, fix #11036 | Q | A | ------------- | --- | Bug fix? | [yes] | New feature? | [no] | BC breaks? | [no] | Deprecations? | [no] | Tests pass? | [yes] | Fixed tickets | #11036, #11345 | License | MIT | Doc PR | Since Symfony 2.5 The problem was that the form component has a hardcoded depencency to the deprecated validator component (api Version 2.4) The pull request fixes the dependency to the validator component and supports now both implementations, apiVersion 2.5 and apiVersion 2.4 of the validator component. @symfony Core Members please review the changes https://github.com/sebastianblum/symfony/blob/0a1e9c208f8730219bebf89f6696b246a0c88da7/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php I'm not sure if it was the right solution Commits ------- ab765c9 [2.5][Form] solved dependency to ValidatorInterface, fix #11036
|
Thanks for this fix! |
Since Symfony 2.5
The problem was that the form component has a hardcoded depencency to the deprecated validator component (api Version 2.4)
The pull request fixes the dependency to the validator component and supports now both implementations, apiVersion 2.5 and apiVersion 2.4 of the validator component.
@symfony Core Members
please review the changes https://github.com/sebastianblum/symfony/blob/0a1e9c208f8730219bebf89f6696b246a0c88da7/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php
I'm not sure if it was the right solution