Custom Rector rule removing blank line and ?>

[freiondrej-lmc]

Hello,
I'm writing a custom rector rule. It has been an amazing experience, Rector has been a really good companion! 👏
Anyway, I'm following the new Rector book on how to write a rule using TDD, so I have several fixtures to check everything works as expected. I encountered a very strange behavior with one of the fixtures. It contains a class with two node changes being applied, the rule works with PhpParser\Node\Stmt\Expression. However, the test fails showing me that there were two unexpected changes as well - 1) a blank line between the use statements and the class definition is removed, 2) the ?> at the very end of the file is removed. The fixtures where only one node change is applied don't have this problem.
It is very well possible that I'm doing something wrong in the way the two nodes are manipulated, but I suspect this to be a different problem because I'm definitely not manipulating neither blank line before the class nor the closing ?> tag.
Tried with rector 0.11.60.
Do you have any idea what should I do to prevent this behavior? Thank you! 🙂

[TomasVotruba]

Hi Ondra,

thanks for sharing your use case.

That's probably job of Standard printer output:
https://github.com/nikic/PHP-Parser/blob/99a24b6a556cb2f85c48f62a5311cbc1f8b6eb92/lib/PhpParser/PrettyPrinterAbstract.php#L223

As for the spacing, it should be handled by coding standrad, not by AST.
There is a section in the book that shows difference between tokens and nodes, where you'll find more.

[TomasVotruba]

Btw, working with Expression is way too generic. It's like subscribing to Node. Better use more specific node like ClassMethod or String_ etc.

[freiondrej-lmc]

@TomasVotruba Hi Tomas, thank you for an incredibly fast response 🚀 thank you for pointing me in the direction of the PrettyPrinter, I will try xdebugging why it's doing this.
As for the Expression - I was previously subscribed to a MethodCall indeed, but I'm refactoring a chained method call and this seemed like the only possibility to me as I sometimes need to remove the last node. But thinking about it more, it would probably be feasible still with the MethodCall, I just got frightened 😆 I will definitely try changing that.
I will post an update here with some progress, hopefully today or sometime next week. Thanks again :)

[TomasVotruba]

But thinking about it more, it would probably be feasible still with the MethodCall, I just got frightened laughing I will definitely try changing that.

Good challange! :) Chained method calls if one of the hardest nodes to process, but I think you can handle it 👍

Good luck 💪