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

fix iterable::class that does not exist #1315

Merged
merged 2 commits into from
Apr 25, 2021
Merged

Conversation

Tobion
Copy link
Contributor

@Tobion Tobion commented Apr 23, 2021

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

foreach (self::SUPPORTED_FORMATS as $format) {
$methods[] = [
'direction' => GraphNavigatorInterface::DIRECTION_SERIALIZATION,
'type' => iterable::class,
Copy link
Contributor Author

@Tobion Tobion Apr 23, 2021

Choose a reason for hiding this comment

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

iterable is a psydo class and does not really exist. this is resolved as a class in the current namespace, i.e. JMS\Serializer\Handler\iterable which does not exist. this is why phpstan complained rightfully.
also the whole registration of iterable is pointless because iterables are already handled directly in the graph visitor (like for array, int etc). so to my understading, this part is irrelevant.

Copy link
Collaborator

@goetas goetas Apr 25, 2021

Choose a reason for hiding this comment

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

makes sense, then why not put just 'iterable' as string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it wouldn't do anything? iterables are not handled via registration.

@Tobion Tobion mentioned this pull request Apr 23, 2021
/**
* @param mixed $data
*/
public function deserializeIterable(
Copy link
Collaborator

@goetas goetas Apr 25, 2021

Choose a reason for hiding this comment

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

does it mean that we do not have anymore the deserialization for iterable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we do. it's inside the graph visistor/navigator itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see

case 'iterable':
return $this->visitor->visitArray($data, $type);

Copy link
Collaborator

Choose a reason for hiding this comment

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

uh, i forgot about that!

@goetas goetas merged commit 654df8e into schmittjoh:master Apr 25, 2021
@goetas
Copy link
Collaborator

goetas commented Apr 25, 2021

thanks!

@Tobion Tobion deleted the fix-iterable branch April 25, 2021 19:44
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.

2 participants