ZF2 Zend\Code bugfix

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

ZF2 Zend\Code bugfix

NickBelhomme
Hi Ralph, Matthew,

Will try the mailing lists instead of posting the mails always to personal
mailboxes...

I have been taking a look at \Zend\Code which is heavily used by Di
component.

My first impressions:
Because it works with tokens it could work quite nice, BUT at the moment it
is quite error prone and also the unit tests aren't really satisfying...

Let me explain my case:

In the current implementation a parameter list such as:
public function __construct(Request $request, Response $response, Grid
$grid, Inventory $personalInventory)

Will fail, because of the MethodScanner passing a token array to
ParameterScanner. and the latter doesn't know how to handle it because of
the whitespaces following the ,

array(4) {
  [0]=>
  array(3) {
    [0]=>
    int(371)
    [1]=>
    string(1) " "
    [2]=>
    int(103)
  }
  [1]=>
  array(3) {
    [0]=>
    int(307)
    [1]=>
    string(8) "Response"
    [2]=>
    int(103)
  }
  [2]=>
  array(3) {
    [0]=>
    int(371)
    [1]=>
    string(1) " "
    [2]=>
    int(103)
  }
  [3]=>
  array(3) {
    [0]=>
    int(309)
    [1]=>
    string(9) "$response"
    [2]=>
    int(103)
  }
}

ParameterScanner does the following:

        if ($token[0] !== T_VARIABLE) {
            while (true) {
                if ($token[0] == T_WHITESPACE) {
                    break;
                }
                $this->class .= $token[1];
                $token = $this->tokens[++$tokenIndex];
            }
        }


The solution can be solved in two ways.
Or we go from the approach that MethodScanner MUST trim the tokens and thus
passes correct tokens to the ParameterScanner
OR we go from the assumption that the methodscanner does it job correctly
and that the tokens passed are really the correct start and end of the
parameter definition. In this case the tokens have to be fixed in the
ParameterScanner itself.

For this I have added a trim function, take a look at :
https://github.com/NickBelhomme/zf2/blob/bugfix_tokentrim/library/Zend/Code/Scanner/ParameterScanner.php
This is just a quick idea, and can be improved but it clearly shows how the
issue could be fixed.

Regarding the Unit Test:
It really only checks parameter 2.
If you add the test with for example parameter 3 you would get a surprising
result and warning.
I know it is just a quick testcase, but at the moment
ZendTest\Code\TestAsset\BarClass is wrongly setup:
protected function three(\ArrayObject $o, &$t = 2, FooBarBaz\BazBarFoo $bbf
= self::BAR)

FooBarBaz\BazBarFoo $bbf = self::BAR is a typed parameter and PHP doesn't
like the default value:
PHPUnit 3.5.14 by Sebastian Bergmann.
Fatal error: Default value for parameters with a class type hint can only be
NULL in
C:\Webserver\Apache2.2\htdocs\zf2\tests\Zend\Code\TestAsset\BarClass.php on
line 28
So the BarClass needs to be fixed so we can do proper unit testing against
the Type forcing with different codestyles (whitespace or not)

Grtz,
Nick