fmt producing bad code - new without parentheses

[bendavies]

hi

in my composer.json I have

"php": "^8.3",

fmt on is formatting code valid for
https://wiki.php.net/rfc/new_without_parentheses
which is only valid for 8.4

It shouldn't do this, right?

[nhedger]

You may be correct, but I think including a minimal reproduction snippet would help.

[azjezz]

can you please share a code example that results in PHP 8.4 only syntax? i tried couple of options but i couldn't get it to output new without parens.

ref:

mago/crates/formatter/src/parens.rs

Lines 175 to 190 in 6177bc1

	if let Expression::Instantiation(new) = expression {
	if new.arguments.is_none() {
	// parentheses are required if the instantiation has no arguments
	// e.g. `new Foo->baz()` should be `(new Foo)->baz()`
	return true;
	}
	// parentheses are not required if the instantiation has arguments
	// e.g. `new Foo()->baz()`.
	//
	// but this is only allowed in PHP 8.4, so for now, we add
	// parentheses to be safe, in the future, we can add an option
	// to remove them.
	//
	// TODO(azjezz): we should add an option to remove parentheses.
	return true;

[bendavies]

https://github.com/brick/date-time/blob/db33701540dd80d8c55d529d697582688f9e4cbb/src/Parser/IsoParsers.php#L41

git clone [email protected]:brick/date-time.git
cd date-time
cp ~/Downloads/mago-0.0.6-aarch64-apple-darwin/mago .
curl https://raw.githubusercontent.com/carthage-software/mago/refs/heads/main/examples/mago-full.toml -o mago.toml
chmod +x mago
xattr -rd com.apple.quarantine mago
./mago fmt
git diff src/Parser/IsoParsers.php

[bendavies]

there some other interesting diffs here...
different issue but this doesn't look equivalent. or is it?

[jdreesen]

I think it is equivalent, because the operator precedence of && is higher than ||.

But I think the parenthesis should stay, as it could be confusing otherwise.

[bendavies]

indeed, confirmed equivalent with the test suite and modifying it further.
but yes, hard to grok without the parens.

[azjezz]

@bendavies the reason that paren is removed is that we actually remove all parens from around expressions, and put them back only when needed.

we can add a setting to add parens in that case.

ref:

mago/crates/formatter/src/parens.rs

Line 19 in 6177bc1

fn need_parens(&mut self, node: Node<'a>) -> bool {

[bendavies]

No problem, it's not a bug. Can raise another issue. I hope my reproducer on the original issue was good for you.

[azjezz]

fixed.