Skip to content

[Console] InvalidArgumentException is thrown under wrong condition #17818

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

Conversation

robinkanters
Copy link

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

When an object is passed to setAutocompleterValues that implements \Countable and \Traversable, an InvalidArgumentException is thrown, even though its message says that the argument should implement both of those interfaces.

Instead, the exception should be thrown if the parameter fails to implement either \Countable or \Traversable, like the exception message says.

Disclaimer: I've been looking at this line for 10min, and I've become convinced it should be changed according to my PR, but I could be wrong.

…able AND doesn't implement Countable, not when it doesn't implement Traversable but DOES implement Countable
@xabbuh
Copy link
Member

xabbuh commented Feb 16, 2016

👍

Status: Reviewed

@dosten
Copy link
Contributor

dosten commented Feb 16, 2016

👍 but this must be merged into 2.7

@robinkanters
Copy link
Author

Shall I make a new PR or will someone merge manually?

@dosten
Copy link
Contributor

dosten commented Feb 16, 2016

Someone will merge this manually in the correct branch.

@robinkanters
Copy link
Author

Okay thanks

On Tue, Feb 16, 2016, 23:12 Diego Saint Esteben notifications@github.com
wrote:

Someone will merge this manually in the correct branch.


Reply to this email directly or view it on GitHub
#17818 (comment).

@fabpot
Copy link
Member

fabpot commented Feb 17, 2016

Thank you @robinkanters.

@fabpot fabpot closed this Feb 17, 2016
fabpot added a commit that referenced this pull request Feb 17, 2016
…ondition (robinkanters)

This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #17818).

Discussion
----------

[Console] InvalidArgumentException is thrown under wrong condition

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

When an object is passed to `setAutocompleterValues` that implements `\Countable` and `\Traversable`, an `InvalidArgumentException` is thrown, even though its message says that the argument should implement both of those interfaces.

Instead, the exception should be thrown if the parameter fails to implement either `\Countable` or `\Traversable`, like the exception message says.

_Disclaimer: I've been looking at this line for 10min, and I've become convinced it should be changed according to my PR, but I could be wrong._

Commits
-------

8eea65e The exception should be thrown if an object doesn't implement Traversable AND doesn't implement Countable, not when it doesn't implement Traversable but DOES implement Countable
@robinkanters robinkanters deleted the bugfix/logic-instanceof-countable branch February 17, 2016 08:29
This was referenced Feb 28, 2016
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