Skip to content

Conversation

@samsonasik
Copy link
Contributor

@samsonasik samsonasik commented Dec 6, 2025

@TomasVotruba this is detected on our rector-symfony rule:

-new DateTime('now');
+new DateTime('now';

can be run with:

vendor/bin/phpunit test/PhpParser/PrettyPrinterTest.php --filter "addArgNewWithoutParentheses" 
PHPUnit 9.6.29 by Sebastian Bergmann and contributors.

F                                                                   1 / 1 (100%)

Time: 00:00.007, Memory: 12.00 MB

There was 1 failure:

1) PhpParser\PrettyPrinterTest::testFormatPreservingPrint with data set "Users/samsonasik/www/PHP-Parser/test/code/formatPreservation/addArgNewWithoutParentheses.test#0" ('Add Arg New Without Parenthes....test)', '<?php\nnew DateTime;', '$stmts[0]->expr->args = [new ...w'))];', '<?php\nnew DateTime('now');', null)
Add Arg New Without Parentheses (/Users/samsonasik/www/PHP-Parser/test/code/formatPreservation/addArgNewWithoutParentheses.test)
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 '<?php
-new DateTime('now');'
+new DateTime'now'<?php
+new DateTime;'

/Users/samsonasik/www/PHP-Parser/test/PhpParser/PrettyPrinterTest.php:236

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

that only green when using () on the very first.

which new without parentheses got invalid diff, and we tricked to set origNode null there.

This test for it, hopefully can be fixed here instead.

@TomasVotruba
Copy link
Contributor

TomasVotruba commented Dec 6, 2025

@samsonasik Intersting. I'm almost certain this must have worked before.
Which version is the first to introduce this bug?

@samsonasik
Copy link
Contributor Author

samsonasik commented Dec 6, 2025

It never working before iirc, when you have new DateTime() with parentheses, add [new Arg()] will working ok, but when we have new DateTime without parentheses, then add the arg, it become lost the ) closing parentheses.

new DateTime() -> add Arg -> new DateTime('now') ✅ works
new DateTime -> add Arg -> new DateTime('now' ❌  not works

@nikic
Copy link
Owner

nikic commented Dec 6, 2025

Thanks for the report! The way insertion into an empty argument list usually works is by finding the next ( and inserting after it. Here no ( will be found and the code will end up doing complete nonsense.

nikic added a commit that referenced this pull request Dec 6, 2025
@nikic
Copy link
Owner

nikic commented Dec 6, 2025

I believe 8c360e2 should fix the issue.

@samsonasik
Copy link
Contributor Author

@nikic yes, that's works, thank you. Btw, could you look on my and @TomasVotruba PRs if you have a chance, thank you.

@samsonasik samsonasik closed this Dec 7, 2025
@samsonasik samsonasik deleted the format-add-new-arg branch December 7, 2025 08:43
@TomasVotruba
Copy link
Contributor

Thank you 👍

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

Successfully merging this pull request may close these issues.

3 participants