search

llaville / php-compatinfo

Library that find out the minimum version and the extensions required for a piece of code to run
https://llaville.github.io/php-compatinfo/7.1/
Other
373 stars 20 forks source link

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

Closed arabcoders closed 6 years ago

arabcoders commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

Correction is available at https://github.com/llaville/php-reflect/commit/6469b1edb626ac8a3646b6677b4e55b40d9622c7

remicollet commented 6 years ago

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 commented 6 years ago

You're right : sprintf syntax was invalid in latest commit : here is the fix https://github.com/llaville/php-reflect/commit/8635309f04c1c97c39a298aca7307326277a2f0e

remicollet commented 6 years ago

still unusable on PHP 7.1+

llaville commented 6 years ago

Please explain in which condition ?

Here is my context of text :

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

remicollet commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Fix is now available with 3 commits :

llaville commented 6 years ago

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 commented 6 years ago

LGTM

llaville commented 6 years ago

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 commented 6 years ago

New versions are now available !