Skip to content

feat: format new without parenthesis in PHP 8.4 #2422

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

claytonrcarter
Copy link
Contributor

@claytonrcarter claytonrcarter commented Apr 17, 2025

This adds support for formatting things like (new Foo())->bar as new Foo()->bar. Support for this syntax was added in PHP 8.4 and very recently merged and released in php-parser.

Note that php-parser 3.2.2 introduced a parser bug that affects how we format specifically placed comments in class constant assignments. My opinion is that this is an extreme edge case issue and it's worth the temporary regression in order to gain support for one of the highlighted features of php 8.4. (There is already a pending PR to fix this in php-parser, and we can rollout an update to fix it here when that lands.

  • bumps php-parser to the newly released v3.2.3
  • breaking: removes (temporarily) support for some comments in class constant assigments
  • adds a new phpVersion option for 8.4
  • adds support for the above syntax, with tests
  • misc light refactoring to support these changes

Verification

  1. I used this to format all text fixture files and confirmed that now (relevant) issues were introduced.
  2. I used this to format a small Laravel project of mine (300+ files, 27k lines of code) and confirmed that the tests still passed.

Try it out

<?php

class Foo 
{
    public $bar = 123;
}

echo (new Foo)->bar;

This should be reformatted to

-echo (new Foo)->bar;
+echo new Foo()->bar;

And php foo.php should print 123 in both cases (though the latter will fail in php <= 8.3).

@claytonrcarter
Copy link
Contributor Author

Marking as draft for now because I want to do some additional testing, and it looks like there are CI failures.

This is a feature regression, but it's needed to workaround a defect in php-parser introduced in v3.2.2. I feel like this is an acceptable regression that we can un-regress once the pending fix is merged and released in php-parser.

Ref: glayzzle/php-parser#1152
needsParens() is already being called with 2 params, but the fn
signature didn't reflect that
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.

1 participant