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

[PropertyAccess] Handle interfaces in the invalid argument exception #21360

Closed
wants to merge 2 commits into from

Conversation

fancyweb
Copy link
Contributor

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

Before :
Expected argument of type "dule\MenuBundle\Entity\AbstractMenu::setMenuElements() must implement interface Doctrine\Common\Collections\Collection", "array" given

After :
Expected argument of type "Doctrine\Common\Collections\Collection", "array" given

@xabbuh
Copy link
Member

xabbuh commented Jan 20, 2017

Can you add a test for this?

@nicolas-grekas nicolas-grekas added this to the 2.7 milestone Jan 24, 2017

/**
* @expectedException \Symfony\Component\PropertyAccess\Exception\InvalidArgumentException
* @expectedExceptionMessage Expected argument of type "Countable", "string" given
Copy link
Member

Choose a reason for hiding this comment

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

I would have expected a test checking for the new "must implement interface" string in the exception message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't get it : the "must implement interface" comes from the PHP exception that is handled and then the InvalidArgumentException is thrown.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, you are right. I misread the changes you made.

Copy link
Member

@xabbuh xabbuh left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@fabpot
Copy link
Member

fabpot commented Jan 25, 2017

Thank you @fancyweb.

fabpot added a commit that referenced this pull request Jan 25, 2017
… exception (fancyweb)

This PR was squashed before being merged into the 2.7 branch (closes #21360).

Discussion
----------

[PropertyAccess] Handle interfaces in the invalid argument exception

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

Before :
`Expected argument of type "dule\MenuBundle\Entity\AbstractMenu::setMenuElements() must implement interface Doctrine\Common\Collections\Collection", "array" given`

After :
`Expected argument of type "Doctrine\Common\Collections\Collection", "array" given`

Commits
-------

be52b39 [PropertyAccess] Handle interfaces in the invalid argument exception
@fabpot fabpot closed this Jan 25, 2017
@fabpot fabpot mentioned this pull request Jan 28, 2017
This was referenced Feb 6, 2017
@fancyweb fancyweb deleted the patch-2 branch March 24, 2017 07:40
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.

None yet

5 participants