Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Utils::getArity fails on callables in array format #7

Closed

Conversation

tklever
Copy link
Contributor

@tklever tklever commented Jun 7, 2015

This patch allows Utils::getArity to correctly return the arity on callables passed in array format

i.e.

array($this, 'method')
array('SomeClass', 'someMethod')

@@ -24,6 +25,7 @@ public function callablesWithVaryingArity()
}, 2],
'invokable' => [new Dispatch(), 5],
'interface' => [new MiddlewarePipe(), 2], // 2 REQUIRED arguments!
'callable' => [array(new NormalHandler(), 'handle'), 3]
Copy link
Member

Choose a reason for hiding this comment

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

Please, use short array syntax

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Force of habit I suppose.

@weierophinney
Copy link
Member

Two notes:

  • First, read the CONTRIBUTING.md; bugfixes should be submitted against the master branch, not the develop branch (that branch is reserved for new features). Right now it's not a big deal, as they're identical, but as the project grows, this will become increasingly important.
  • Second, in reviewing, I came up with another case: Class::method. When I added that to the data providers, it failed; I've got code I'll commit with the merge that will resolve it, however. :)

Thanks for the patch, @tklever !

weierophinney added a commit that referenced this pull request Jun 15, 2015
Utils::getArity fails on callables in array format
weierophinney added a commit that referenced this pull request Jun 15, 2015
- Added StaticHandler test asset with a static method.
- Added arity data providers for:
  - `['classname', 'methodname']`
  - `'classname::methodname'`
- Last provider required another addition to the code to properly check arity of
  that static method.
weierophinney added a commit that referenced this pull request Jun 15, 2015
weierophinney added a commit that referenced this pull request Jun 15, 2015
@tklever
Copy link
Contributor Author

tklever commented Jun 16, 2015

@weierophinney No Problem, thank YOU for your continued hard work on all things Zend (and PSR7 and Middleware, etc., etc., etc.)

I pushed this against develop vs. master because I saw it as an enhancement (we're supporting an additional callback format) vs. a bug (we support this and it's broken). I can certainly see cases to be made for either direction, but I'm glad it was good enough to get merged.

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

Successfully merging this pull request may close these issues.

3 participants