Idea: Use tolerant-php-parser for error tolerance (Time consuming, requires php7)

[TysonAndre]

I'm mentioning this because it wasn't mentioned in other issues.
This is probably a long term project, and probably couldn't be started until php 5.6 support is dropped. (ext-suggest might work in composer.json)

Benefits of tolerant-php-parser:

• Better recovery from parse errors. If nikic/php-parser encounters an error (E.g. { without a match in a method), it gives up on parsing nodes in the rest of the file.

  Error tolerance is useful if there were plans to run psalm as a background process that could respond to analysis requests (e.g. Language Server Protocol)

• Reduced CPU usage from parsing and error diagnostics. (I assume that serialize()/igbinary_serialize() would be smaller on disk and faster to unserialize as well, but haven't benchmarked that)

  See Adopt Microsoft/tolerant-php-parser felixfbecker/php-language-server#357

  Traversing the tree would hopefully also be faster, but I haven't benchmarked that.

Drawbacks of tolerant-php-parser

• A few known bugs for uncommon syntax: https://github.com/Microsoft/tolerant-php-parser/issues?q=is%3Aissue+is%3Aopen+label%3Abug

  Might fail to catch invalid syntax or incorrectly warn about rare valid syntax

• Would break any existing plugins that used \PHPParser\Node

• Requires php 7 to run, and I think token_get_all() may differ between php 5 and php 7 (may be revisited)

• Missing some APIs (E.g. to convert from a complex string literal to the raw string. Probably feasible to implement)

Other notes:

A hybrid approach of converting tolerant-php-parser to php-parser (Only in cases where php-parser had at least one error) could work in theory. (https://github.com/TysonAndre/tolerant-php-parser-to-php-ast/ is an example of converting one AST format to another)

• Writing a project to convert from one ast to another is time consuming to implement, debug, and test
• The only benefit is better recovery from parse errors on syntactically invalid files, which isn't important right now

[muglug]

The only benefit is better recovery from parse errors on syntactically invalid files, which isn't important right now

Yeah, that's the key thing for me. And at the moment the parsing performance difference is negligible given how expensive analysis is.

But obviously a language server would need this, along with a couple of other Psalm features. I've created the server mode milestone to track that support.

Other thoughts

I'm currently ok with Psalm acting as a gatekeeper rather than as an assistant, and I slightly worry that LSP support might sidetrack the project. It would need to be usable at Vimeo's scale, where analysis on a single (much-too-large) file can take over a second (with all dependencies already loaded into memory).

But it would also be very cool.

[muglug]

I went with PHP Parser 4 instead, it improved its error tolerance substantially.