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

More error recovery #262

Closed
felixfbecker opened this issue Mar 30, 2016 · 5 comments
Closed

More error recovery #262

felixfbecker opened this issue Mar 30, 2016 · 5 comments

Comments

@felixfbecker
Copy link
Contributor

I'm trying to use this library to provide autocompletion for an editor. This means often the code will not be complete:

$object = new DateTime;
$object-> // cursor here

the code above will parse with throwOnError: false, but only the first statement. It would be nice the parser could see the incomplete statement and guess a node, like

class PhpParser\Node\Expr\PropertyFetch#11 (3) {
    public $var =>
    class PhpParser\Node\Expr\Variable#10 (2) {
      public $name => string(6) "object"
    }
    public $name => null
  }
@nikic
Copy link
Owner

nikic commented Apr 18, 2016

It should be possible to implement this (not in a generic way, but handling specific cases like this one). However I'm not sure if it's worth the effort. I would generally recommend against using this library for implementing editor auto-complete (or rather, only use it for the analysis part) and detect this based on tokens instead. The current error recovery is more aimed at "be able to analyze other methods in this file while it's being modified" rather than "be able to analyze the currently modified expression". Editor-quality error recovery is a pretty hard nut and probably not in scope for this project.

@felixfbecker
Copy link
Contributor Author

@nikic I also thought about using tokens only but that would mean I would have to implement a whole lot of stuff that is already so well implemented in this AST parser. It might be easy for something simple like $object->property but for something like Namespace\Class::create()->property[0]->method() I would love to use a real AST - even if that statement is not finished with a semicolon yet. Could you maybe get me on the right track how I where I would have to implement this? Should I subclass the parser?

@nikic
Copy link
Owner

nikic commented Apr 20, 2016

I've just pushed a small improvement to error recovery in 96cbd48. This should handle the case of statements without a trailing semicolon, but will not handle the case of $foo-> (and no following label). One interesting issue with that case is

$foo-> // cursor here
$bar->foo();

this will parse as valid PHP $foo->$bar->foo(), even though it's likely not what was intended.

If you want to work on better error recovery, this is done by sprinking additional error rules in the grammar. Currently we have only two such rules in https://github.com/nikic/PHP-Parser/blob/master/grammar/php7.y#L182, which mean that we can handle an expression followed by an error at some point, or just an error altogether (without preceding expression).

For example, to support $foo-> what one would have to do is add an | error rule to property_name in https://github.com/nikic/PHP-Parser/blob/master/grammar/php7.y#L781. This rule could be | error { $$ = null; } for a null property name (as you suggest). Alternatively (and I would recommend this as "null" is not acceptable everywhere due to type hints) one could introduce an Expr\Error node, in which case the rule should be | error { $$ = Expr\Error[]; }.

After modifying the grammar, you'll have to rebuild the parser, see https://github.com/nikic/PHP-Parser/tree/master/grammar#building-the-parser. (If you're on Windows, I can send you a kmyacc build.)

Error recovery tests are here: https://github.com/nikic/PHP-Parser/blob/master/test/code/parser/errorHandling/recovery.test It may be convenient to test only the php7 parser, this can be done by putting a !!php7 at the top of the expected output.

@nikic
Copy link
Owner

nikic commented Jul 25, 2016

Added support for recovering from -> errors in 09086fb and 977cbab. I'll consider this issue as resolved, for now.

@nikic nikic closed this as completed Jul 25, 2016
@felixfbecker
Copy link
Contributor Author

I just tried this out and this is absolutely amazing! Great work!

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

No branches or pull requests

2 participants