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

Contract for accessing constraint violation list iterator #38290

Closed
Wirone opened this issue Sep 24, 2020 · 4 comments
Closed

Contract for accessing constraint violation list iterator #38290

Wirone opened this issue Sep 24, 2020 · 4 comments

Comments

@Wirone
Copy link
Contributor

Wirone commented Sep 24, 2020

Symfony version(s) affected: 5.1.5

Description
Currently Symfony\Component\Validator\ConstraintViolationListInterface does not offer method for accessing iterator. There is getIterator() as a part ConstraintViolationList concrete implementation which also implements IteratorAggregate interface, however Symfony\Component\Messenger\Exception\ValidationFailedException::getViolations() return type points to interface, not implementation. In result, it's "impossible" to access iterator when type-hinting to ConstraintViolationListInterface interface.

How to reproduce

<?php

use Symfony\Component\Messenger\Exception\ValidationFailedException;
use Symfony\Component\Validator\ConstraintViolationInterface;

try {
    ...
} catch (ValidationFailedException $e) {
    dd(array_map(
        static fn (ConstraintViolationInterface $violation) => [
            $violation->getPropertyPath(),
            $violation->getMessage(),
        ],
        // Psalm complains: Method Symfony\Component\Validator\ConstraintViolationListInterface::getIterator does not exist
        (array) $e->getViolations()->getIterator()
    ));
}

Possible Solution
Move implements \IteratorAggregate from ConstraintViolationList to ConstraintViolationListInterface (as extends \IteratorAggregate, obviously). I've tried it locally and Psalm doesn't raise an error.

Additional context
In Symfony app it works (because of implementation), the problem is with static analysis.

@ro0NL
Copy link
Contributor

ro0NL commented Sep 24, 2020

ConstraintViolationListInterface is a \Traversable :)

so foreach($violations) or iterator_to_array($violations) just works at the contract level

@Wirone
Copy link
Contributor Author

Wirone commented Sep 24, 2020

@ro0NL I didn't think that way, thanks to you I've changed code to:

array_map(
    static fn (ConstraintViolationInterface $violation) => [
        $violation->getPropertyPath(),
        $violation->getMessage(),
    ],
    [...$e->getViolations()]
)

and it works (Rector fixed iterator_to_array($e->getViolations) into [...$e->getViolations]).

Still, would be good to have more user-friendly (explicit) contract for accessing items. Feel free to close it as non-bug if you feel that current contract is correct.

@ro0NL
Copy link
Contributor

ro0NL commented Sep 24, 2020

@Wirone i think we can close yes, Symfony doesnt really define templated generic phpdocs currently. Instead it could be handled in plugin/stub files per tool (e.g. phpstan, psalm).

@implements \Traversable<ConstraintViolationInterface>

@fabpot fabpot closed this as completed Sep 24, 2020
@Wirone
Copy link
Contributor Author

Wirone commented Sep 25, 2020

@ro0NL I didn't want to add support for generics in phpDoc, I thought about adding explicit method to ConstraintViolationListInterface for accessing all items as array/iterator. It's not crucial now, as you pointed out how it can be accessed on language level, but for beginners it might not be as obvious 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants