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

Keywords reserved cannot be detected with PHP 5.3 platform #4

Closed
llaville opened this issue May 28, 2015 · 8 comments
Closed

Keywords reserved cannot be detected with PHP 5.3 platform #4

llaville opened this issue May 28, 2015 · 8 comments

Comments

@llaville
Copy link
Contributor

When your run on a PHP 5.3 plaform to detect if keywords reserved are used. E.g

php phpmig -s to54 /path/to/keywords_reserved.php

With source file (keywords_reserved.php)

<?php

class Trait
{
    const FOO = "foo";
}

class finally
{
    function __construct()
    {
        $trait = new Trait;

        $foo = Trait::FOO;
    }
}

You got following error

File: keywords_reserved.php
--------------------------------------------------------------------------------
    3 | PARSE      | * | NONE | Syntax error, unexpected T_TRAIT, expecting T_STRING on line 3
--------------------------------------------------------------------------------

Reason is that PHP-Parser is not yet able to catch such condition, until now. I've proposed an improvement nikic/PHP-Parser#202

And soon, I'll propose a migration analyser on my php-compatinfo project, based on Sniff pattern.
See llaville/php-compatinfo#202
I must gave you a credit, because this future analyser (that is available to detect even keywords reserved) is due to a deeper analysis of your project architecture.

@llaville
Copy link
Contributor Author

You may be perharps interested by the presentation I wrote llaville/php-compatinfo#202 (comment) to explain how I solved this issue on my project.

@monque
Copy link
Owner

monque commented Jun 3, 2015

Your comments evokes my memory of developing this project...

I remember I have 3 solutions about the same problem how to handle reserved keywords ?

  • Using lexer in PHP 5.3, trait is handled as a T_STRING
    • pros: reserved keywords will be checked as expected.
    • cons: parse failed if real trait appeared.
  • Using emulative lexel, trait is handled as a T_TRAIT
    • pros: all new features (trait, callable ...) will be checked.
    • cons: parse failed if trait as a identifier (class name, function name...). (typical error I want to check)
  • Additional process for reserved keywords: parse 2 times, first for reserved keywords, second for syntax
    • cons: complex...

After discussion, conclusion is that users want to check incompatible things and make sure his code runing on new version of PHP.

So the second solution meets requirement, It'll emit a Syntax error if a reserved keyword appeared in wrong place, and all new features will be checked.

And the most important is, that's just simple, and do all we needs.

@llaville
Copy link
Contributor Author

llaville commented Jun 4, 2015

Don't forget to have a look on my new version of Emulative Lexer ( llaville/php-reflect@c828107) that solved the problem

@llaville
Copy link
Contributor Author

@monque Did you have time to have a look on my new Emulative lexer copy (see previous comment).

Since then, I've begun to write lot of sniffs and especially the KeyWordReservedSniff that checks for keywords uses. It works fine for me on all PHP versions

@llaville
Copy link
Contributor Author

@monque Be sure to pick the latest php-reflect version on sniffer branch. See last commit llaville/php-reflect@4be20c3 that fixed (I hope) the last detection issue

@monque
Copy link
Owner

monque commented Jul 3, 2015

@llaville This file KeyWordReservedSniff makes a perfect solution.

Thanks for your contribution, I'm too busy to develop new feature..., so sorry for this.

Will you add all these "sniffs" for version compatibility to php-compat-info ?

@llaville
Copy link
Contributor Author

llaville commented Jul 3, 2015

@monque For next version 4.4.0 of php-compatinfo the compatibility analyser will be kept as actual. I planned to rewrite it usign sniffs.
But I'll introduced in the same time, a new migration analyser that will used all sniffs as currently defined in php-compatinfo migration_analyser branch.

@llaville
Copy link
Contributor Author

I'll closed this issue, because php-compatinfo 4.5.x is the last version that will support PHP 5.3 and my version of Emulative Lexer to detect keywords.
Next major version that will come soon now ( 5.0 ) will support PHP 5.4 or above, and then dropped this version of Emulative Lexer.

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

2 participants