Skip to content

[Validator] Remove calls to non-existing method #17357

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
Jan 14, 2016

Conversation

paradajozsef
Copy link
Contributor

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

This PR removes some useless code after #16024.

1. ConstraintValidator::buildViolation() marked as deprecated in 2.8, and has been removed in 3.0.

But all the Symfony/Component/Validator/Constraints/*validators still making calls to this parent method.

2. Correct me if I'm wrong, but this condition:

$this->context instanceof ExecutionContextInterface

in the Symfony/Component/Validator/Constraints/* validators is useless since 3.0, because the $context can only be ExecutionContextInterface. I guess the porpuse of this condition was, that in 2.8 there was a legacy interface too.

And AFAIK, the $context is always initialized before validation, so no need to check that $context is whether null or not.

3. The return value of ExecutionContextInterface::getViolations(), is in a different namespace, so it should be used.

@Tobion
Copy link
Contributor

Tobion commented Jan 14, 2016

Thanks for the detailed investigation. LGTM 👍
Status: Reviewed

@dunglas
Copy link
Member

dunglas commented Jan 14, 2016

👍

@fabpot
Copy link
Member

fabpot commented Jan 14, 2016

Thank you @paradajozsef.

@fabpot fabpot merged commit 37fb4e2 into symfony:3.0 Jan 14, 2016
fabpot added a commit that referenced this pull request Jan 14, 2016
…ozsef)

This PR was merged into the 3.0 branch.

Discussion
----------

[Validator] Remove calls to non-existing method

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

This PR removes some useless code after #16024.

**1.** [ConstraintValidator::buildViolation()](https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/Validator/ConstraintValidator.php#L64) marked as deprecated in [2.8](https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/Validator/ConstraintValidator.php#L64), and has been removed in [3.0](https://github.com/symfony/symfony/blob/3.0/src/Symfony/Component/Validator/ConstraintValidator.php#L51).

But all the ```Symfony/Component/Validator/Constraints/*```validators still making calls to this parent method.

**2.** Correct me if I'm wrong, but this condition:
```php
$this->context instanceof ExecutionContextInterface
```
in the ```Symfony/Component/Validator/Constraints/*``` validators is useless since 3.0, because the ```$context``` can only be ```ExecutionContextInterface```. I guess the porpuse of this condition was, that in 2.8 there was a [legacy interface](https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/Validator/Context/ExecutionContextInterface.php#L63) too.

And AFAIK, the ```$context``` is [always initialized](https://github.com/symfony/symfony/blob/3.0/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php#L842) before validation, so no need to check that ```$context``` is whether null or not.

**3.** The return value of [ExecutionContextInterface::getViolations()](https://github.com/symfony/symfony/blob/3.0/src/Symfony/Component/Validator/Context/ExecutionContextInterface.php#L239), is in a different namespace, so it should be used.

Commits
-------

37fb4e2 Remove calls to non-existing method
@paradajozsef paradajozsef deleted the remove-non-existing-calls branch January 14, 2016 08:08
Tobion added a commit that referenced this pull request Jan 16, 2016
This PR was merged into the 3.0 branch.

Discussion
----------

Remove remaining calls to non-existing method

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

This removes the ```$this->buildViolation``` calls (which is a dead method) from validators in 3.0, as #17357 did. I just found 3 more. I hope it's not a problem, that it's not in 2 separate PR (Doctrine Bridge, Form). But I can split them if I have to. :)

Commits
-------

d7e3254 [3.0] Remove calls to not-existent method
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.

5 participants