Incorrect behavior of PowToExpRector: missing parentheses around unary operators (except for minus), ternary, Elvis, composed assignments, 'print', 'yield' or 'yield from'

[glop-glop]

Bug Report

Subject	Details
Rector version	last dev-main (rectorphp/rector-src@1ccbb46b)

Hello,

As mentioned in #9800 (comment), it seems that rule PowToExpRector also misses some cases where parentheses are required.

From what I could test, it seems that it correctly adds parentheses around binary operators, comparisons, unary minus, and simple assignment, but it misses other cases (roughly the same ones as LogicalToBooleanRector): unary operators other than minus (such as ~, !, (int), etc.), ternary, Elvis, composed assignments, and (although they do not make much actual sense in this context) print, yield and yield from constructs.

Minimal PHP Code Causing Issue

See: https://getrector.com/demo/0ea443a9-c0ad-40c5-bf0e-a9c4281a5f74

<?php

# Unary operators
pow(~3, 4);        # → equivalent to (~3) ** 4, evaluates to 256
pow(!3, 4);        # → equivalent to (!3) ** 4, evaluates to 0
pow((int) 3.8, 4); # → equivalent to ((int) 3.8) ** 4, evaluates to 81

# Composed assignments
$a = 1;
pow($a += 4, 3);   # → equivalent to ($a += 4) ** 3, evaluates to 125 and sets $a to 5

# Ternary and short-ternary (Elvis) operators, left-hand side
$a = 2;
pow($a ? 4 : 3, 2); # → equivalent to ($a ? 4 : 3) ** 2, evaluates to 16
pow($a ?: 3, 2);    # → equivalent to ($a ?: 3) ** 2, evaluates to 4

# Ternary and short-ternary (Elvis) operators, right-hand side
$b = 0;
pow(4, $b ? 3 : 2); # → equivalent to 4 ** ($b ? 3 : 2), evaluates to 16
pow(4, $b ?: 2);    # → equivalent to 4 ** ($b ?: 2), evaluates to 16

# 'print', 'yield', 'yield from' constructs
pow(print 4, 3);        # → equivalent to (print 4) ** 3, prints 4, evaluates to 1
pow(yield 4, 3);        # → equivalent to (yield 4) ** 3, yields 4, evaluates to 0
pow(yield from [4], 3); # → equivalent to (yield from [4]) ** 3, yields 4, evaluates to 0

Currently, Rector rewrites them without parentheses, which is incorrect:

<?php

# Unary operators
~3 ** 4;        # → evaluates to -82 instead of 256
!3 ** 4;        # → evaluates to false instead of 0
(int) 3.8 ** 4; # → evaluates to 208 instead of 81

# Composed assignments
$a = 1;
$a += 4 ** 3;   # → evaluates to 65 instead of 125, and sets $a to 65 instead of 5

# Ternary and short-ternary (Elvis) operators, left-hand side
$a = 2;
$a ? 4 : 3 ** 2; # → evaluates to 4 instead of 16
$a ?: 3 ** 2;    # → evaluates to 2 instead of 4

# Ternary and short-ternary (Elvis) operators, right-hand side
$b = 0;
4 ** $b ? 3 : 2; # → evaluates to 3 instead of 16
4 ** $b ?: 2;    # → evaluates to 1 instead of 16

# 'print', 'yield', 'yield from' constructs
print 4 ** 3;        # → prints 64 instead of 4, evaluates to 1
yield 4 ** 3;        # → yields 64 instead of 4, and evaluates to null instead of 0
yield from [4] ** 3; # → PHP Fatal error:  Uncaught TypeError: Unsupported operand types: array ** int

Expected Behaviour

Rector should add parentheses around expressions whose root operator has lower precedence than **:

<?php

# Unary operators
(~3) ** 4;        # → evaluates to 256 (OK)
(!3) ** 4;        # → evaluates to 0 (OK)
((int) 3.8) ** 4; # → evaluates to 81 (OK)

# Composed assignments
$a = 1;
($a += 4) ** 3;   # → evaluates to 125 and sets $a to 5 (OK)

# Ternary and short-ternary (Elvis) operators, left-hand side
$a = 2;
($a ? 4 : 3) ** 2; # → evaluates to 16 (OK)
($a ?: 3) ** 2;    # → evaluates to 4 (OK)

# Ternary and short-ternary (Elvis) operators, right-hand side
$b = 0;
4 ** ($b ? 3 : 2); # → evaluates to 16 (OK)
4 ** ($b ?: 2);    # → evaluates to 16 (OK)

# 'print', 'yield', 'yield from' constructs
(print 4) ** 3;        # → prints 4, evaluates to 1 (OK)
(yield 4) ** 3;        # → yields 4, evaluates to 0 (OK)
(yield from [4]) ** 3; # → yields 4, evaluates to 0 (OK)

I'm still not familiar enough with the code to offer a fix, but maybe the tests on lines 499 and 504 of BetterStandardPrinter.php would need to consider other node types as well? And if Assign nodes were also to be considered there, maybe lines 481 to 489 would become unnecessary, just like lines 57 to 59 in PowToExpRector.php, which handle the particular case of UnaryMinus nodes?

Thank you very much! :)

Cheers,
Glop

[glop-glop]

Great, thank you very much for this fix! All my test cases run perfectly fine, now :)

Cheers,
Glop