Fix code reflection - getBody/getContents method #5245
Conversation
| * @link http://github.com/zendframework/zf2 for the canonical source repository | ||
| * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com) | ||
| * @license http://framework.zend.com/license/new-bsd New BSD License | ||
| * @package Zend_Code |
|
@Ocramius can you confirm that on this function : function1()
{
return 'foo';
}getBody is : return 'foo';getContent is : function1()
{
return 'foo';
}? |
|
@blanchonvincent never used that API - are there no other tests for it? |
|
@Ocramius The prototype is public function getContents($includeDocBlock = true)So, if the contents is : {
return ....;
}what is the contents with docblock ? I think there is a bug, and the contents must be : function foo()
{
return ....;
}and with docblock : /**
* bla bla bla
*/
function foo()
{
return ....;
} |
|
@blanchonvincent yep, I agree on that. I'd just first check what the consumers of that method are before applying any change. |
|
@Ocramius, Ok, let me know |
|
@blanchonvincent no, I was suggesting you to grep for its usage :) |
|
@Ocramius, Look never used, and $useDockBlock param doesn't work, and lot of time, the function return an empty result ... so I fix that. |
|
PR is finished -- better code coverage & lot of fixs |
|
@Ocramius since you did original review; could you please review a last time? |
|
|
||
| $content = false; | ||
| if ($this->isClosure()) { | ||
| preg_match('#function\s*\([^\)]*\)\s*(use\s*\([^\)]+\))?\s*\{(.*\;)?\s*\}#s', $functionLine, $matches); |
There was a problem hiding this comment.
Woah, this witchcraft needs a comment :-)
There was a problem hiding this comment.
Does this handle also closures containing closures?
There was a problem hiding this comment.
@Ocramius I don't see any reason why it wouldn't -- the body regex is looking for any number of terminated statements.
There was a problem hiding this comment.
@Ocramius, yep, look at the tests
|
|
||
| $body = false; | ||
| if ($this->isClosure()) { | ||
| preg_match('#function\s*\([^\)]*\)\s*(use\s*\([^\)]+\))?\s*\{(.*\;)\s*\}#s', $functionLine, $matches); |
There was a problem hiding this comment.
Also: these regexes look constant. Can you move them to either a class constant or a private property?
There was a problem hiding this comment.
They're not -- there are subtle differences in what each is matching.
|
@blanchonvincent Please rebase the PR for fix merge conflicts |
…ator-functiongenerator Fix code reflection - getBody/getContents method Conflicts: library/Zend/Code/Reflection/FunctionReflection.php library/Zend/Code/Reflection/MethodReflection.php tests/ZendTest/Code/Reflection/FunctionReflectionTest.php tests/ZendTest/Code/Reflection/MethodReflectionTest.php
- trailing whitespace, braces - Added functions.php to the exclude list for php-cs-fixer, as the tests are testing specific formatting issues.
ghost
assigned 
|
Merged to develop for release with 2.3.0, as it represents new functionality. I took care of the merge conflicts. |
This reverts commit 76074d4.
Zend\Code\Reflection getBody function is often broken with some code format