rectorphp
Catch parser exceptions and add more info to them about what file is causing the exeception
Feature Request
When rector is run on a directory with bad/legacy code in it, it will sometimes crash due to issue in the code. For example :
Fatal error: Uncaught LogicException: leaveNode() returned invalid value of type integer in /var/www/html/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:150
I would be nice if rector could catch these errors, and tell me where in the code that we are parsing/changing this issue is causing the rector run to fail.
Don't know if this is possible but i think this would help legacy code bases a lot in adjusting to rector. Since than you can manually fix that 1 error en rerun rector until the entire directory is fixed.
Diff
-Fatal error: Uncaught LogicException: leaveNode() returned invalid value of type integer in /var/www/html/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:150
+Parse error: Code on line 123 in LegacyClass is causing a php parsing error : Uncaught LogicException: leaveNode() returned invalid value of type integer in /var/www/html/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:150;
Hi, that would be great addition. Could you narrow down the minimal code that is causing this?
In this case It's the calls to traverseNode of PhpParser. I'm still looking for the cause in my legacy project.
But there are more traverse functions in PhpParser that can throw exceptions (ex: traverseArray).
https://github.com/nikic/PHP-Parser/blob/4abdcde5f16269959a834e4e58ea0ba0938ab133/lib/PhpParser/NodeTraverser.php#L109
It says it will return a Node but has no indication that a LogicException could be thrown.
so all calls in Rector to traverseNode could catch the LogicException. I would also be good if nikic/php-parser would add those exceptions to its docblocks
Could you create simple reproducible repository that cause it? Thank you.
Like i said above, I'm still looking for the cause in my legacy project. When I find it I can try to provide a way to reproduce it, in the meanwhile I pointed out the cause of the error.
After doing some more debugging. The rector rule causing the issue is "NodeRemovingPostRector". It returns "\PhpParser\NodeTraverser::REMOVE_NODE" in some cases which is supported in traverseArray but not in traverseNode.
https://github.com/nikic/PHP-Parser/blob/4abdcde5f16269959a834e4e58ea0ba0938ab133/lib/PhpParser/NodeTraverser.php#L191
https://github.com/nikic/PHP-Parser/blob/4abdcde5f16269959a834e4e58ea0ba0938ab133/lib/PhpParser/NodeTraverser.php#L109
I don't know enough about the PHP parser to know if this is just forgotten here and is an issue for nikic/PHP-Parser or that it is intensionally.
But I think a try catch with a continue around the traverse method of 'PostFileProcessor' could at least solve it from crashing for now. It's not ideal but rector will not crash it will just ignore the file. If you agree i would be more than happy to provide a PR for this.
https://github.com/rectorphp/rector/blob/dd31e833a99eeaf522a2690cc390c5e922316f29/packages/PostRector/Application/PostFileProcessor.php#L49
We need to have reproducible code that crash without the patch, you can narrow half by half until found a rule that caused it, --debug should help
We're narrowing issues to keep focus on active and engaging contributor to keep project growing. It's been almost 2 months since last reaction here, so we're closing it. Thank you for understanding.
Feel free to re-open with failing test case or demo link with minimal reproducible code: https://getrector.org/demo