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

bug in parser? [] causes the phpcompatinfot o skip file #239

Closed
arabcoders opened this issue Dec 8, 2017 · 18 comments
Closed

bug in parser? [] causes the phpcompatinfot o skip file #239

arabcoders opened this issue Dec 8, 2017 · 18 comments
Assignees

Comments

@arabcoders
Copy link

if we use the new list syntax e.g [$foo,$bar,$baz] instead of list($foo,$bar,$baz) phpcompatinfo bugs out and skip the whole file without notice.

<?php

class foo
{
    const BAZ = [1,15,0];
    public function bar() 
    {
         [ $page, $perpage, $start ] = static::BAZ;
    }
}
E:\PHP>phpcompatinfo analyser:run test.php --ansi

Data Source Analysed

Directories                                          1
Files                                                1

No extension found

No namespace found

No interface found

No trait found

No class found

No function found

No constant found

No condition found
@llaville
Copy link
Owner

llaville commented Dec 9, 2017

Hello, and thanks for reporting

BTW, this list short syntax was implemented in PHP 7.1

I understand that we should not be able to parse code on PHP platform 7.1 or greater with compatinfo 5.0

Be aware that a new major version 6.0 is on way (branch 5.1 was canceled and will be remove in next days). See note.

As the php-parser version 2.x was implemented in php-reflect, I've to release a new version 4.2 of this one to avoid installation on PHP plateform 7.1+

Stay tuned.

@llaville
Copy link
Owner

Finally, I've decided to not block installation with plateform 7.1 or 7.2.

Reason: it's not necessary (especially if you want to use other features) and will not solved this issue.

Alternative solution, i've implemented, and will release in next hour, is to check if analyser run on plateform 7.1+ and stop analysis.

That will give a console output error such as

phpCompatInfo 5.0-dev is unable to parse PHP scripts with version 7.1.12

@remicollet
Copy link
Contributor

phpCompatInfo 5.0-dev is unable to parse PHP scripts with version 7.1.12

Not true, is able to parse PHP script which requires php < 7.1, which is very different.
I use daily, running php 7.0, 7.1, and 7.2 without any issue.

@llaville
Copy link
Owner

@remicollet Perharps the message is not correct.
But if you try to run the source code of this issue, php-parse 2.x raise a syntax error (that is correct, because list short syntax is available since PHP 7.1 ==> https://wiki.php.net/rfc/short_list_syntax). This exception is catched by php-compatinfo, but give an empty result that is not the good way.

I prefer to have an error message (warning) that allow user to know why !

@llaville
Copy link
Owner

@remicollet Perharps this message is more correct

phpCompatInfo 5.0-dev is unable to parse PHP scripts with syntax PHP 7.1 or greater

Your opinion ?

@llaville llaville self-assigned this Dec 11, 2017
@llaville
Copy link
Owner

Correction is available at llaville/php-reflect@6469b1e

@remicollet
Copy link
Contributor

remicollet commented Dec 11, 2017

IIUC, current fix proposal make phpcompatinfo unusable.

-1 from me

This exception is catched by php-compatinfo

So properly report such parser error, only in this case

@llaville
Copy link
Owner

You're right : sprintf syntax was invalid in latest commit : here is the fix llaville/php-reflect@8635309

@remicollet
Copy link
Contributor

still unusable on PHP 7.1+

@llaville
Copy link
Owner

Please explain in which condition ?

Here is my context of text :

  • script to test is the source provided in this issue
  • plateform PHP 7.1.12 on windows
  • command run
    C:\home\github\php-compat-info>\UwAmp\bin\php\php-7.1.12\php bin\phpcompatinfo analyser:run tests\fixtures\gh239.php --no-plugins

phpCompatInfo 5.0-dev is unable to parse PHP scripts with syntax PHP 7.1 or greater

@remicollet
Copy link
Contributor

Please understand me correctly:

1/ I use phpcompatinfo daily with PHP 7.1.12 and 7.2.0: IT WORKS

2/ For this very specific case, it the parser fails it should report it

  phpCompatInfo 5.0-dev is unable to parse 'xxxx.php' file

@llaville
Copy link
Owner

Yes of course i understand.
This message seems a bit too lite. But as we cannot know if php-parser syntax error is due to a platforme constraint or just a normal syntax error status, i'll apply a new fix that reflect point 2/

Thanks for your feedback

@llaville
Copy link
Owner

New fix will be able to produce errors raised by php-parser

Here is a preview with structure analyser

Data Source Analysed

Directories                                          3
Files                                              196
Errors                                               2

Errors found

 > Syntax error, unexpected '=' on line 11 in file /home/github/php-compat-info/tests/fixtures\gh239.php

 > Syntax error, unexpected T_TRAIT, expecting T_STRING on line 3 in file /home/github/php-compat-info/tests/fixtures\sniffs\keywords_reserved.php

Structure
  Namespaces                                         5
  Interfaces                                         6
  Traits                                             4
  Classes                                           55

.... and more

Raw output (verbose level 3) preview

Array
(
    [files] => Array
        (
            [0] => /home/github/php-compat-info/tests/fixtures\array_keys.18881d.php
            [1] => /home/github/php-compat-info/tests/fixtures\array_keys.18881o.php
... and more 
            [195] => /home/github/php-compat-info/tests/fixtures\substr_count.18881o.php
        )

    [errors] => Array
        (
            [/home/github/php-compat-info/tests/fixtures\gh239.php] => Syntax error, unexpected '=' on line 11
            [/home/github/php-compat-info/tests/fixtures\sniffs\keywords_reserved.php] => Syntax error, unexpected T_TRAIT, expecting T_STRING on line 3
        )

    [Bartlett\Reflect\Analyser\StructureAnalyser] => Array
        (
            [namespaces] => 5
            [interfaces] => 6
            [traits] => 4
            [classes] => 55
... and more

Of course all custom analysers should be revisited to adapt new errors entry (free to do what you want) !

@llaville
Copy link
Owner

Fix is now available with 3 commits :

@llaville
Copy link
Owner

Running on script example, php-compatinfo will output

Data Source Analysed

Directories                                          1
Files                                                1
Errors                                               1

Errors found

 > Syntax error, unexpected '=' on line 11 in file /home/github/php-compat-info/tests/fixtures\gh239.php

No extension found

No namespace found

No interface found

No trait found

No class found

No function found

No constant found

No condition found

Feedback are welcome, before to release new versions (of php-reflect and php-compatinfo)

@remicollet
Copy link
Contributor

LGTM

@llaville
Copy link
Owner

ok so i'll prepare a new release 4.2.0 for php-reflect and a new release 5.0.10 for php-compatinfo (with reflect constraint raise to 4.2 min)

llaville added a commit that referenced this issue Dec 12, 2017
@llaville
Copy link
Owner

New versions are now available !

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

No branches or pull requests

3 participants