From c12115be286b1fb07230e389e66b67edab051ba0 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 8 Jul 2026 19:00:45 +0200 Subject: [PATCH 1/2] [Php56] Add missing parentheses for lower-precedence operands in PowToExpRector pow() converted to ** dropped required parentheses around operands that bind looser than the ** operator, changing semantics. Wrap unary operators, casts, ternary, compound assignments, print and yield operands when needed. Fixes rectorphp/rector#9804 --- .../Fixture/keep_bare_right_operand.php.inc | 25 +++++++ .../Fixture/lower_precedence_operands.php.inc | 33 ++++++++++ .../Php56/Rector/FuncCall/PowToExpRector.php | 65 ++++++++++++++++++- 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 rules-tests/Php56/Rector/FuncCall/PowToExpRector/Fixture/keep_bare_right_operand.php.inc create mode 100644 rules-tests/Php56/Rector/FuncCall/PowToExpRector/Fixture/lower_precedence_operands.php.inc diff --git a/rules-tests/Php56/Rector/FuncCall/PowToExpRector/Fixture/keep_bare_right_operand.php.inc b/rules-tests/Php56/Rector/FuncCall/PowToExpRector/Fixture/keep_bare_right_operand.php.inc new file mode 100644 index 00000000000..9221e549a72 --- /dev/null +++ b/rules-tests/Php56/Rector/FuncCall/PowToExpRector/Fixture/keep_bare_right_operand.php.inc @@ -0,0 +1,25 @@ + +----- + diff --git a/rules-tests/Php56/Rector/FuncCall/PowToExpRector/Fixture/lower_precedence_operands.php.inc b/rules-tests/Php56/Rector/FuncCall/PowToExpRector/Fixture/lower_precedence_operands.php.inc new file mode 100644 index 00000000000..a78172663d0 --- /dev/null +++ b/rules-tests/Php56/Rector/FuncCall/PowToExpRector/Fixture/lower_precedence_operands.php.inc @@ -0,0 +1,33 @@ + +----- + diff --git a/rules/Php56/Rector/FuncCall/PowToExpRector.php b/rules/Php56/Rector/FuncCall/PowToExpRector.php index de81227758e..46aaf08bb26 100644 --- a/rules/Php56/Rector/FuncCall/PowToExpRector.php +++ b/rules/Php56/Rector/FuncCall/PowToExpRector.php @@ -5,9 +5,23 @@ namespace Rector\Php56\Rector\FuncCall; use PhpParser\Node; +use PhpParser\Node\Expr; +use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\AssignOp; +use PhpParser\Node\Expr\AssignRef; use PhpParser\Node\Expr\BinaryOp\Pow; +use PhpParser\Node\Expr\BitwiseNot; +use PhpParser\Node\Expr\BooleanNot; +use PhpParser\Node\Expr\Cast; +use PhpParser\Node\Expr\ErrorSuppress; use PhpParser\Node\Expr\FuncCall; +use PhpParser\Node\Expr\Instanceof_; +use PhpParser\Node\Expr\Print_; +use PhpParser\Node\Expr\Ternary; use PhpParser\Node\Expr\UnaryMinus; +use PhpParser\Node\Expr\UnaryPlus; +use PhpParser\Node\Expr\Yield_; +use PhpParser\Node\Expr\YieldFrom; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\Rector\AbstractRector; use Rector\ValueObject\PhpVersionFeature; @@ -54,8 +68,14 @@ public function refactor(Node $node): ?Node $secondExpr = $node->getArgs()[1] ->value; - if ($firstExpr instanceof UnaryMinus) { - $firstExpr->setAttribute(AttributeKey::ORIGINAL_NODE, null); + // ** binds tighter than most operators, so operands with lower precedence must be + // wrapped in parentheses to keep the original semantics, e.g. pow(~3, 4) => (~3) ** 4 + if ($this->isLowerPrecedenceThanPowLeft($firstExpr)) { + $firstExpr->setAttribute(AttributeKey::WRAPPED_IN_PARENTHESES, true); + } + + if ($this->isLowerPrecedenceThanPowRight($secondExpr)) { + $secondExpr->setAttribute(AttributeKey::WRAPPED_IN_PARENTHESES, true); } return new Pow($firstExpr, $secondExpr); @@ -65,4 +85,45 @@ public function provideMinPhpVersion(): int { return PhpVersionFeature::EXP_OPERATOR; } + + /** + * Operators that bind looser than ** on the left-hand side, so pow($operand, $y) would be + * misparsed without parentheses. Unary operators and casts are legal bare on the right-hand + * side of ** (2 ** -3), so they only need wrapping when used as the left operand. + */ + private function isLowerPrecedenceThanPowLeft(Expr $expr): bool + { + // a plain Assign as left operand is already parenthesized by BetterStandardPrinter, + // wrapping it again here would produce a double set of parentheses + if ($expr instanceof Assign) { + return false; + } + + if ($expr instanceof UnaryMinus + || $expr instanceof UnaryPlus + || $expr instanceof BitwiseNot + || $expr instanceof BooleanNot + || $expr instanceof ErrorSuppress + || $expr instanceof Cast + || $expr instanceof Instanceof_) { + return true; + } + + return $this->isLowerPrecedenceThanPowRight($expr); + } + + /** + * Operators that would otherwise swallow the ** expression on either side, e.g. + * pow(2, $a ? 3 : 4) must become 2 ** ($a ? 3 : 4), not 2 ** $a ? 3 : 4. + */ + private function isLowerPrecedenceThanPowRight(Expr $expr): bool + { + return $expr instanceof Ternary + || $expr instanceof Assign + || $expr instanceof AssignRef + || $expr instanceof AssignOp + || $expr instanceof Print_ + || $expr instanceof Yield_ + || $expr instanceof YieldFrom; + } } From c303732f86b248461b5369351d59eae518dec71c Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 8 Jul 2026 19:06:00 +0200 Subject: [PATCH 2/2] [NodeAnalyzer] Extract PowOperandAnalyzer service from PowToExpRector Precedence checks for ** operands are reusable by any rule that builds a Pow node from arbitrary operands, e.g. compound **= handling. --- .../Php56/Rector/FuncCall/PowToExpRector.php | 66 ++-------------- src/NodeAnalyzer/PowOperandAnalyzer.php | 70 +++++++++++++++++ tests/NodeAnalyzer/PowOperandAnalyzerTest.php | 76 +++++++++++++++++++ 3 files changed, 154 insertions(+), 58 deletions(-) create mode 100644 src/NodeAnalyzer/PowOperandAnalyzer.php create mode 100644 tests/NodeAnalyzer/PowOperandAnalyzerTest.php diff --git a/rules/Php56/Rector/FuncCall/PowToExpRector.php b/rules/Php56/Rector/FuncCall/PowToExpRector.php index 46aaf08bb26..27b2f40bb32 100644 --- a/rules/Php56/Rector/FuncCall/PowToExpRector.php +++ b/rules/Php56/Rector/FuncCall/PowToExpRector.php @@ -5,23 +5,9 @@ namespace Rector\Php56\Rector\FuncCall; use PhpParser\Node; -use PhpParser\Node\Expr; -use PhpParser\Node\Expr\Assign; -use PhpParser\Node\Expr\AssignOp; -use PhpParser\Node\Expr\AssignRef; use PhpParser\Node\Expr\BinaryOp\Pow; -use PhpParser\Node\Expr\BitwiseNot; -use PhpParser\Node\Expr\BooleanNot; -use PhpParser\Node\Expr\Cast; -use PhpParser\Node\Expr\ErrorSuppress; use PhpParser\Node\Expr\FuncCall; -use PhpParser\Node\Expr\Instanceof_; -use PhpParser\Node\Expr\Print_; -use PhpParser\Node\Expr\Ternary; -use PhpParser\Node\Expr\UnaryMinus; -use PhpParser\Node\Expr\UnaryPlus; -use PhpParser\Node\Expr\Yield_; -use PhpParser\Node\Expr\YieldFrom; +use Rector\NodeAnalyzer\PowOperandAnalyzer; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\Rector\AbstractRector; use Rector\ValueObject\PhpVersionFeature; @@ -34,6 +20,11 @@ */ final class PowToExpRector extends AbstractRector implements MinPhpVersionInterface { + public function __construct( + private readonly PowOperandAnalyzer $powOperandAnalyzer + ) { + } + public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( @@ -70,11 +61,11 @@ public function refactor(Node $node): ?Node // ** binds tighter than most operators, so operands with lower precedence must be // wrapped in parentheses to keep the original semantics, e.g. pow(~3, 4) => (~3) ** 4 - if ($this->isLowerPrecedenceThanPowLeft($firstExpr)) { + if ($this->powOperandAnalyzer->isLowerPrecedenceAsLeftOperand($firstExpr)) { $firstExpr->setAttribute(AttributeKey::WRAPPED_IN_PARENTHESES, true); } - if ($this->isLowerPrecedenceThanPowRight($secondExpr)) { + if ($this->powOperandAnalyzer->isLowerPrecedenceAsRightOperand($secondExpr)) { $secondExpr->setAttribute(AttributeKey::WRAPPED_IN_PARENTHESES, true); } @@ -85,45 +76,4 @@ public function provideMinPhpVersion(): int { return PhpVersionFeature::EXP_OPERATOR; } - - /** - * Operators that bind looser than ** on the left-hand side, so pow($operand, $y) would be - * misparsed without parentheses. Unary operators and casts are legal bare on the right-hand - * side of ** (2 ** -3), so they only need wrapping when used as the left operand. - */ - private function isLowerPrecedenceThanPowLeft(Expr $expr): bool - { - // a plain Assign as left operand is already parenthesized by BetterStandardPrinter, - // wrapping it again here would produce a double set of parentheses - if ($expr instanceof Assign) { - return false; - } - - if ($expr instanceof UnaryMinus - || $expr instanceof UnaryPlus - || $expr instanceof BitwiseNot - || $expr instanceof BooleanNot - || $expr instanceof ErrorSuppress - || $expr instanceof Cast - || $expr instanceof Instanceof_) { - return true; - } - - return $this->isLowerPrecedenceThanPowRight($expr); - } - - /** - * Operators that would otherwise swallow the ** expression on either side, e.g. - * pow(2, $a ? 3 : 4) must become 2 ** ($a ? 3 : 4), not 2 ** $a ? 3 : 4. - */ - private function isLowerPrecedenceThanPowRight(Expr $expr): bool - { - return $expr instanceof Ternary - || $expr instanceof Assign - || $expr instanceof AssignRef - || $expr instanceof AssignOp - || $expr instanceof Print_ - || $expr instanceof Yield_ - || $expr instanceof YieldFrom; - } } diff --git a/src/NodeAnalyzer/PowOperandAnalyzer.php b/src/NodeAnalyzer/PowOperandAnalyzer.php new file mode 100644 index 00000000000..3c8a194f02c --- /dev/null +++ b/src/NodeAnalyzer/PowOperandAnalyzer.php @@ -0,0 +1,70 @@ +isLowerPrecedenceAsRightOperand($expr); + } + + /** + * Operators that would otherwise swallow the ** expression on either side, e.g. + * pow(2, $a ? 3 : 4) must become 2 ** ($a ? 3 : 4), not 2 ** $a ? 3 : 4. + */ + public function isLowerPrecedenceAsRightOperand(Expr $expr): bool + { + return $expr instanceof Ternary + || $expr instanceof Assign + || $expr instanceof AssignRef + || $expr instanceof AssignOp + || $expr instanceof Print_ + || $expr instanceof Yield_ + || $expr instanceof YieldFrom; + } +} diff --git a/tests/NodeAnalyzer/PowOperandAnalyzerTest.php b/tests/NodeAnalyzer/PowOperandAnalyzerTest.php new file mode 100644 index 00000000000..f5a918e4503 --- /dev/null +++ b/tests/NodeAnalyzer/PowOperandAnalyzerTest.php @@ -0,0 +1,76 @@ +powOperandAnalyzer = new PowOperandAnalyzer(); + } + + #[DataProvider('provideLeftOperand')] + public function testLeftOperand(Expr $expr, bool $expected): void + { + $this->assertSame($expected, $this->powOperandAnalyzer->isLowerPrecedenceAsLeftOperand($expr)); + } + + #[DataProvider('provideRightOperand')] + public function testRightOperand(Expr $expr, bool $expected): void + { + $this->assertSame($expected, $this->powOperandAnalyzer->isLowerPrecedenceAsRightOperand($expr)); + } + + /** + * @return iterable + */ + public static function provideLeftOperand(): iterable + { + yield [new BitwiseNot(new Int_(3)), true]; + yield [new UnaryMinus(new Int_(3)), true]; + yield [new CastInt(new Variable('a')), true]; + yield [new Instanceof_(new Variable('a'), new Name('DateTime')), true]; + yield [new Ternary(new Variable('a'), new Int_(1), new Int_(2)), true]; + yield [new AssignPlus(new Variable('a'), new Int_(4)), true]; + + // a plain Assign is already parenthesized by the printer on the left side + yield [new Assign(new Variable('a'), new Int_(4)), false]; + yield [new Variable('a'), false]; + yield [new Int_(3), false]; + } + + /** + * @return iterable + */ + public static function provideRightOperand(): iterable + { + yield [new Ternary(new Variable('a'), new Int_(1), new Int_(2)), true]; + yield [new Assign(new Variable('a'), new Int_(4)), true]; + yield [new AssignPlus(new Variable('a'), new Int_(4)), true]; + + // unary operators and casts are legal bare on the right side of ** + yield [new BitwiseNot(new Int_(3)), false]; + yield [new UnaryMinus(new Int_(3)), false]; + yield [new CastInt(new Variable('a')), false]; + yield [new Variable('a'), false]; + } +}