From abab3ef44d45645c3e304ad5c0c847d62b63523a Mon Sep 17 00:00:00 2001 From: schlndh Date: Fri, 12 Jul 2024 12:42:24 +0200 Subject: [PATCH 1/5] report mixed in binary operator --- conf/config.level2.neon | 7 +- src/Collectors/Registry.php | 12 +- src/Rules/DirectRegistry.php | 12 +- src/Rules/LazyRegistry.php | 12 +- .../Operators/InvalidBinaryOperationRule.php | 147 +++++----- .../InvalidBinaryOperationRuleTest.php | 269 +++++++++++++++++- .../Operators/data/invalid-binary-mixed.php | 157 ++++++++++ 7 files changed, 538 insertions(+), 78 deletions(-) create mode 100644 tests/PHPStan/Rules/Operators/data/invalid-binary-mixed.php diff --git a/conf/config.level2.neon b/conf/config.level2.neon index ae718efb17..ec0c93f33f 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -30,7 +30,6 @@ rules: - PHPStan\Rules\Generics\UsedTraitsRule - PHPStan\Rules\Methods\CallPrivateMethodThroughStaticRule - PHPStan\Rules\Methods\IncompatibleDefaultParameterTypeRule - - PHPStan\Rules\Operators\InvalidBinaryOperationRule - PHPStan\Rules\Operators\InvalidUnaryOperationRule - PHPStan\Rules\Operators\InvalidComparisonOperationRule - PHPStan\Rules\PhpDoc\FunctionConditionalReturnTypeRule @@ -138,3 +137,9 @@ services: - class: PHPStan\Rules\Pure\PureMethodRule + - + class: PHPStan\Rules\Operators\InvalidBinaryOperationRule + arguments: + bleedingEdge: %featureToggles.bleedingEdge% + tags: + - phpstan.rules.rule diff --git a/src/Collectors/Registry.php b/src/Collectors/Registry.php index 65b80118b2..9bb7123322 100644 --- a/src/Collectors/Registry.php +++ b/src/Collectors/Registry.php @@ -35,7 +35,17 @@ public function __construct(array $collectors) public function getCollectors(string $nodeType): array { if (!isset($this->cache[$nodeType])) { - $parentNodeTypes = [$nodeType] + class_parents($nodeType) + class_implements($nodeType); + $parents = class_parents($nodeType); + if ($parents === false) { + $parents = []; + } + + $implements = class_implements($nodeType); + if ($implements === false) { + $implements = []; + } + + $parentNodeTypes = [$nodeType] + $parents + $implements; $collectors = []; foreach ($parentNodeTypes as $parentNodeType) { diff --git a/src/Rules/DirectRegistry.php b/src/Rules/DirectRegistry.php index 7ee53824ee..16550a08c2 100644 --- a/src/Rules/DirectRegistry.php +++ b/src/Rules/DirectRegistry.php @@ -35,7 +35,17 @@ public function __construct(array $rules) public function getRules(string $nodeType): array { if (!isset($this->cache[$nodeType])) { - $parentNodeTypes = [$nodeType] + class_parents($nodeType) + class_implements($nodeType); + $parents = class_parents($nodeType); + if ($parents === false) { + $parents = []; + } + + $implements = class_implements($nodeType); + if ($implements === false) { + $implements = []; + } + + $parentNodeTypes = [$nodeType] + $parents + $implements; $rules = []; foreach ($parentNodeTypes as $parentNodeType) { diff --git a/src/Rules/LazyRegistry.php b/src/Rules/LazyRegistry.php index a9a1a728ec..674b05ed2b 100644 --- a/src/Rules/LazyRegistry.php +++ b/src/Rules/LazyRegistry.php @@ -32,7 +32,17 @@ public function __construct(private Container $container) public function getRules(string $nodeType): array { if (!isset($this->cache[$nodeType])) { - $parentNodeTypes = [$nodeType] + class_parents($nodeType) + class_implements($nodeType); + $parents = class_parents($nodeType); + if ($parents === false) { + $parents = []; + } + + $implements = class_implements($nodeType); + if ($implements === false) { + $implements = []; + } + + $parentNodeTypes = [$nodeType] + $parents + $implements; $rules = []; $rulesFromContainer = $this->getRulesFromContainer(); diff --git a/src/Rules/Operators/InvalidBinaryOperationRule.php b/src/Rules/Operators/InvalidBinaryOperationRule.php index 4fb2167030..5c687ea51d 100644 --- a/src/Rules/Operators/InvalidBinaryOperationRule.php +++ b/src/Rules/Operators/InvalidBinaryOperationRule.php @@ -26,6 +26,7 @@ class InvalidBinaryOperationRule implements Rule public function __construct( private ExprPrinter $exprPrinter, private RuleLevelHelper $ruleLevelHelper, + private bool $bleedingEdge, ) { } @@ -44,81 +45,81 @@ public function processNode(Node $node, Scope $scope): array return []; } - if ($scope->getType($node) instanceof ErrorType) { - $leftName = '__PHPSTAN__LEFT__'; - $rightName = '__PHPSTAN__RIGHT__'; - $leftVariable = new Node\Expr\Variable($leftName); - $rightVariable = new Node\Expr\Variable($rightName); - if ($node instanceof Node\Expr\AssignOp) { - $identifier = 'assignOp'; - $newNode = clone $node; - $newNode->setAttribute('phpstan_cache_printer', null); - $left = $node->var; - $right = $node->expr; - $newNode->var = $leftVariable; - $newNode->expr = $rightVariable; - } else { - $identifier = 'binaryOp'; - $newNode = clone $node; - $newNode->setAttribute('phpstan_cache_printer', null); - $left = $node->left; - $right = $node->right; - $newNode->left = $leftVariable; - $newNode->right = $rightVariable; - } - - if ($node instanceof Node\Expr\AssignOp\Concat || $node instanceof Node\Expr\BinaryOp\Concat) { - $callback = static fn (Type $type): bool => !$type->toString() instanceof ErrorType; - } else { - $callback = static fn (Type $type): bool => !$type->toNumber() instanceof ErrorType; - } - - $leftType = $this->ruleLevelHelper->findTypeToCheck( - $scope, - $left, - '', - $callback, - )->getType(); - if ($leftType instanceof ErrorType) { - return []; - } - - $rightType = $this->ruleLevelHelper->findTypeToCheck( - $scope, - $right, - '', - $callback, - )->getType(); - if ($rightType instanceof ErrorType) { - return []; - } - - if (!$scope instanceof MutatingScope) { - throw new ShouldNotHappenException(); - } - - $scope = $scope - ->assignVariable($leftName, $leftType, $leftType) - ->assignVariable($rightName, $rightType, $rightType); - - if (!$scope->getType($newNode) instanceof ErrorType) { - return []; - } - - return [ - RuleErrorBuilder::message(sprintf( - 'Binary operation "%s" between %s and %s results in an error.', - substr(substr($this->exprPrinter->printExpr($newNode), strlen($leftName) + 2), 0, -(strlen($rightName) + 2)), - $scope->getType($left)->describe(VerbosityLevel::value()), - $scope->getType($right)->describe(VerbosityLevel::value()), - )) - ->line($left->getStartLine()) - ->identifier(sprintf('%s.invalid', $identifier)) - ->build(), - ]; + if (!$scope->getType($node) instanceof ErrorType && !$this->bleedingEdge) { + return []; + } + + $leftName = '__PHPSTAN__LEFT__'; + $rightName = '__PHPSTAN__RIGHT__'; + $leftVariable = new Node\Expr\Variable($leftName); + $rightVariable = new Node\Expr\Variable($rightName); + if ($node instanceof Node\Expr\AssignOp) { + $identifier = 'assignOp'; + $newNode = clone $node; + $newNode->setAttribute('phpstan_cache_printer', null); + $left = $node->var; + $right = $node->expr; + $newNode->var = $leftVariable; + $newNode->expr = $rightVariable; + } else { + $identifier = 'binaryOp'; + $newNode = clone $node; + $newNode->setAttribute('phpstan_cache_printer', null); + $left = $node->left; + $right = $node->right; + $newNode->left = $leftVariable; + $newNode->right = $rightVariable; + } + + if ($node instanceof Node\Expr\AssignOp\Concat || $node instanceof Node\Expr\BinaryOp\Concat) { + $callback = static fn (Type $type): bool => !$type->toString() instanceof ErrorType; + } else { + $callback = static fn (Type $type): bool => !$type->toNumber() instanceof ErrorType; + } + + $leftType = $this->ruleLevelHelper->findTypeToCheck( + $scope, + $left, + '', + $callback, + )->getType(); + if ($leftType instanceof ErrorType) { + return []; + } + + $rightType = $this->ruleLevelHelper->findTypeToCheck( + $scope, + $right, + '', + $callback, + )->getType(); + if ($rightType instanceof ErrorType) { + return []; + } + + if (!$scope instanceof MutatingScope) { + throw new ShouldNotHappenException(); + } + + $scope = $scope + ->assignVariable($leftName, $leftType, $leftType) + ->assignVariable($rightName, $rightType, $rightType); + + if (!$scope->getType($newNode) instanceof ErrorType) { + return []; } - return []; + return [ + RuleErrorBuilder::message(sprintf( + 'Binary operation "%s" between %s and %s results in an error.', + substr(substr($this->exprPrinter->printExpr($newNode), strlen($leftName) + 2), 0, -(strlen($rightName) + 2)), + $scope->getType($left)->describe(VerbosityLevel::value()), + $scope->getType($right)->describe(VerbosityLevel::value()), + )) + ->line($left->getStartLine()) + ->identifier(sprintf('%s.invalid', $identifier)) + ->build(), + ]; } } diff --git a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php index 35b3489fd8..0fe63ac5cf 100644 --- a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php @@ -15,11 +15,16 @@ class InvalidBinaryOperationRuleTest extends RuleTestCase { + private bool $checkExplicitMixed = false; + + private bool $checkImplicitMixed = false; + protected function getRule(): Rule { return new InvalidBinaryOperationRule( new ExprPrinter(new Printer()), - new RuleLevelHelper($this->createReflectionProvider(), true, false, true, false, false, true, false), + new RuleLevelHelper($this->createReflectionProvider(), true, false, true, $this->checkExplicitMixed, $this->checkImplicitMixed, true, false), + true, ); } @@ -283,4 +288,266 @@ public function testBug5309(): void $this->analyse([__DIR__ . '/data/bug-5309.php'], []); } + public function testBinaryMixed(): void + { + $this->checkExplicitMixed = true; + $this->checkImplicitMixed = true; + $this->analyse([__DIR__ . '/data/invalid-binary-mixed.php'], [ + [ + 'Binary operation "." between T and \'a\' results in an error.', + 11, + ], + [ + 'Binary operation ".=" between \'a\' and T results in an error.', + 13, + ], + [ + 'Binary operation "**" between T and 2 results in an error.', + 15, + ], + [ + 'Binary operation "*" between T and 2 results in an error.', + 16, + ], + [ + 'Binary operation "/" between T and 2 results in an error.', + 17, + ], + // % is not handled yet + [ + 'Binary operation "+" between T and 2 results in an error.', + 19, + ], + [ + 'Binary operation "-" between T and 2 results in an error.', + 20, + ], + [ + 'Binary operation "<<" between T and 2 results in an error.', + 21, + ], + [ + 'Binary operation ">>" between T and 2 results in an error.', + 22, + ], + [ + 'Binary operation "&" between T and 2 results in an error.', + 23, + ], + [ + 'Binary operation "|" between T and 2 results in an error.', + 24, + ], + [ + 'Binary operation "+=" between 5 and T results in an error.', + 26, + ], + [ + 'Binary operation "-=" between 5 and T results in an error.', + 29, + ], + [ + 'Binary operation "*=" between 5 and T results in an error.', + 32, + ], + [ + 'Binary operation "**=" between 5 and T results in an error.', + 35, + ], + [ + 'Binary operation "/=" between 5 and T results in an error.', + 38, + ], + // %= is not handled yet + [ + 'Binary operation "&=" between 5 and T results in an error.', + 44, + ], + [ + 'Binary operation "|=" between 5 and T results in an error.', + 47, + ], + [ + 'Binary operation "^=" between 5 and T results in an error.', + 50, + ], + [ + 'Binary operation "<<=" between 5 and T results in an error.', + 53, + ], + [ + 'Binary operation ">>=" between 5 and T results in an error.', + 56, + ], + [ + 'Binary operation "." between mixed and \'a\' results in an error.', + 61, + ], + [ + 'Binary operation ".=" between \'a\' and mixed results in an error.', + 63, + ], + [ + 'Binary operation "**" between mixed and 2 results in an error.', + 65, + ], + [ + 'Binary operation "*" between mixed and 2 results in an error.', + 66, + ], + [ + 'Binary operation "/" between mixed and 2 results in an error.', + 67, + ], + [ + 'Binary operation "+" between mixed and 2 results in an error.', + 69, + ], + [ + 'Binary operation "-" between mixed and 2 results in an error.', + 70, + ], + [ + 'Binary operation "<<" between mixed and 2 results in an error.', + 71, + ], + [ + 'Binary operation ">>" between mixed and 2 results in an error.', + 72, + ], + [ + 'Binary operation "&" between mixed and 2 results in an error.', + 73, + ], + [ + 'Binary operation "|" between mixed and 2 results in an error.', + 74, + ], + [ + 'Binary operation "+=" between 5 and mixed results in an error.', + 76, + ], + [ + 'Binary operation "-=" between 5 and mixed results in an error.', + 79, + ], + [ + 'Binary operation "*=" between 5 and mixed results in an error.', + 82, + ], + [ + 'Binary operation "**=" between 5 and mixed results in an error.', + 85, + ], + [ + 'Binary operation "/=" between 5 and mixed results in an error.', + 88, + ], + [ + 'Binary operation "&=" between 5 and mixed results in an error.', + 94, + ], + [ + 'Binary operation "|=" between 5 and mixed results in an error.', + 97, + ], + [ + 'Binary operation "^=" between 5 and mixed results in an error.', + 100, + ], + [ + 'Binary operation "<<=" between 5 and mixed results in an error.', + 103, + ], + [ + 'Binary operation ">>=" between 5 and mixed results in an error.', + 106, + ], + [ + 'Binary operation "." between mixed and \'a\' results in an error.', + 111, + ], + [ + 'Binary operation ".=" between \'a\' and mixed results in an error.', + 113, + ], + [ + 'Binary operation "**" between mixed and 2 results in an error.', + 115, + ], + [ + 'Binary operation "*" between mixed and 2 results in an error.', + 116, + ], + [ + 'Binary operation "/" between mixed and 2 results in an error.', + 117, + ], + [ + 'Binary operation "+" between mixed and 2 results in an error.', + 119, + ], + [ + 'Binary operation "-" between mixed and 2 results in an error.', + 120, + ], + [ + 'Binary operation "<<" between mixed and 2 results in an error.', + 121, + ], + [ + 'Binary operation ">>" between mixed and 2 results in an error.', + 122, + ], + [ + 'Binary operation "&" between mixed and 2 results in an error.', + 123, + ], + [ + 'Binary operation "|" between mixed and 2 results in an error.', + 124, + ], + [ + 'Binary operation "+=" between 5 and mixed results in an error.', + 126, + ], + [ + 'Binary operation "-=" between 5 and mixed results in an error.', + 129, + ], + [ + 'Binary operation "*=" between 5 and mixed results in an error.', + 132, + ], + [ + 'Binary operation "**=" between 5 and mixed results in an error.', + 135, + ], + [ + 'Binary operation "/=" between 5 and mixed results in an error.', + 138, + ], + [ + 'Binary operation "&=" between 5 and mixed results in an error.', + 144, + ], + [ + 'Binary operation "|=" between 5 and mixed results in an error.', + 147, + ], + [ + 'Binary operation "^=" between 5 and mixed results in an error.', + 150, + ], + [ + 'Binary operation "<<=" between 5 and mixed results in an error.', + 153, + ], + [ + 'Binary operation ">>=" between 5 and mixed results in an error.', + 156, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Operators/data/invalid-binary-mixed.php b/tests/PHPStan/Rules/Operators/data/invalid-binary-mixed.php new file mode 100644 index 0000000000..edd4eb52b0 --- /dev/null +++ b/tests/PHPStan/Rules/Operators/data/invalid-binary-mixed.php @@ -0,0 +1,157 @@ + 0; + var_dump($a ** 2); + var_dump($a * 2); + var_dump($a / 2); + var_dump($a % 2); + var_dump($a + 2); + var_dump($a - 2); + var_dump($a << 2); + var_dump($a >> 2); + var_dump($a & 2); + var_dump($a | 2); + $c = 5; + $c += $a; + + $c = 5; + $c -= $a; + + $c = 5; + $c *= $a; + + $c = 5; + $c **= $a; + + $c = 5; + $c /= $a; + + $c = 5; + $c %= $a; + + $c = 5; + $c &= $a; + + $c = 5; + $c |= $a; + + $c = 5; + $c ^= $a; + + $c = 5; + $c <<= $a; + + $c = 5; + $c >>= $a; +} + +function explicitMixed(mixed $a): void +{ + var_dump($a . 'a'); + $b = 'a'; + $b .= $a; + $bool = rand() > 0; + var_dump($a ** 2); + var_dump($a * 2); + var_dump($a / 2); + var_dump($a % 2); + var_dump($a + 2); + var_dump($a - 2); + var_dump($a << 2); + var_dump($a >> 2); + var_dump($a & 2); + var_dump($a | 2); + $c = 5; + $c += $a; + + $c = 5; + $c -= $a; + + $c = 5; + $c *= $a; + + $c = 5; + $c **= $a; + + $c = 5; + $c /= $a; + + $c = 5; + $c %= $a; + + $c = 5; + $c &= $a; + + $c = 5; + $c |= $a; + + $c = 5; + $c ^= $a; + + $c = 5; + $c <<= $a; + + $c = 5; + $c >>= $a; +} + +function implicitMixed($a): void +{ + var_dump($a . 'a'); + $b = 'a'; + $b .= $a; + $bool = rand() > 0; + var_dump($a ** 2); + var_dump($a * 2); + var_dump($a / 2); + var_dump($a % 2); + var_dump($a + 2); + var_dump($a - 2); + var_dump($a << 2); + var_dump($a >> 2); + var_dump($a & 2); + var_dump($a | 2); + $c = 5; + $c += $a; + + $c = 5; + $c -= $a; + + $c = 5; + $c *= $a; + + $c = 5; + $c **= $a; + + $c = 5; + $c /= $a; + + $c = 5; + $c %= $a; + + $c = 5; + $c &= $a; + + $c = 5; + $c |= $a; + + $c = 5; + $c ^= $a; + + $c = 5; + $c <<= $a; + + $c = 5; + $c >>= $a; +} From ea98962ff7b18864c1e6d6b97645e9e65c11dbc1 Mon Sep 17 00:00:00 2001 From: schlndh Date: Fri, 12 Jul 2024 13:33:35 +0200 Subject: [PATCH 2/5] report invalid % operator usage --- .../InitializerExprTypeResolver.php | 4 ++ .../InvalidBinaryOperationRuleTest.php | 42 ++++++++++++++++++- .../Rules/Operators/data/invalid-binary.php | 12 ++++++ 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/Reflection/InitializerExprTypeResolver.php b/src/Reflection/InitializerExprTypeResolver.php index 94d2fe4307..3eee9061b6 100644 --- a/src/Reflection/InitializerExprTypeResolver.php +++ b/src/Reflection/InitializerExprTypeResolver.php @@ -856,6 +856,10 @@ public function getModType(Expr $left, Expr $right, callable $getTypeCallback): return $this->getNeverType($leftType, $rightType); } + if ($leftType->toNumber() instanceof ErrorType || $rightType->toNumber() instanceof ErrorType) { + return new ErrorType(); + } + $leftTypes = $leftType->getConstantScalarTypes(); $rightTypes = $rightType->getConstantScalarTypes(); $leftTypesCount = count($leftTypes); diff --git a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php index 0fe63ac5cf..3f5f1d102f 100644 --- a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php @@ -251,6 +251,22 @@ public function testRule(): void 'Binary operation "+" between int and array{} results in an error.', 259, ], + [ + 'Binary operation "%" between array and 3 results in an error.', + 267, + ], + [ + 'Binary operation "%" between 3 and array results in an error.', + 268, + ], + [ + 'Binary operation "%" between object and 3 results in an error.', + 270, + ], + [ + 'Binary operation "%" between 3 and object results in an error.', + 271, + ], ]); } @@ -313,7 +329,10 @@ public function testBinaryMixed(): void 'Binary operation "/" between T and 2 results in an error.', 17, ], - // % is not handled yet + [ + 'Binary operation "%" between T and 2 results in an error.', + 18, + ], [ 'Binary operation "+" between T and 2 results in an error.', 19, @@ -358,7 +377,10 @@ public function testBinaryMixed(): void 'Binary operation "/=" between 5 and T results in an error.', 38, ], - // %= is not handled yet + [ + 'Binary operation "%=" between 5 and T results in an error.', + 41, + ], [ 'Binary operation "&=" between 5 and T results in an error.', 44, @@ -399,6 +421,10 @@ public function testBinaryMixed(): void 'Binary operation "/" between mixed and 2 results in an error.', 67, ], + [ + 'Binary operation "%" between mixed and 2 results in an error.', + 68, + ], [ 'Binary operation "+" between mixed and 2 results in an error.', 69, @@ -443,6 +469,10 @@ public function testBinaryMixed(): void 'Binary operation "/=" between 5 and mixed results in an error.', 88, ], + [ + 'Binary operation "%=" between 5 and mixed results in an error.', + 91, + ], [ 'Binary operation "&=" between 5 and mixed results in an error.', 94, @@ -483,6 +513,10 @@ public function testBinaryMixed(): void 'Binary operation "/" between mixed and 2 results in an error.', 117, ], + [ + 'Binary operation "%" between mixed and 2 results in an error.', + 118, + ], [ 'Binary operation "+" between mixed and 2 results in an error.', 119, @@ -527,6 +561,10 @@ public function testBinaryMixed(): void 'Binary operation "/=" between 5 and mixed results in an error.', 138, ], + [ + 'Binary operation "%=" between 5 and mixed results in an error.', + 141, + ], [ 'Binary operation "&=" between 5 and mixed results in an error.', 144, diff --git a/tests/PHPStan/Rules/Operators/data/invalid-binary.php b/tests/PHPStan/Rules/Operators/data/invalid-binary.php index ef5f6e4c58..60d71d4ba1 100644 --- a/tests/PHPStan/Rules/Operators/data/invalid-binary.php +++ b/tests/PHPStan/Rules/Operators/data/invalid-binary.php @@ -258,3 +258,15 @@ function benevolentPlus(array $a, int $i): void { function (int $int) { $int + []; }; + +function testMod(array $a, object $o): void { + echo 4 % 3; + echo '4' % 3; + echo 4 % '3'; + + echo $a % 3; + echo 3 % $a; + + echo $o % 3; + echo 3 % $o; +} From ef5a49cb04c40e65363dae683868254629accdc3 Mon Sep 17 00:00:00 2001 From: schlndh Date: Fri, 12 Jul 2024 14:05:21 +0200 Subject: [PATCH 3/5] add tests issues 7538 and 10440 --- .../InvalidBinaryOperationRuleTest.php | 20 +++++++++++++++++++ .../Rules/Operators/data/bug-10440.php | 8 ++++++++ .../PHPStan/Rules/Operators/data/bug-7538.php | 7 +++++++ 3 files changed, 35 insertions(+) create mode 100644 tests/PHPStan/Rules/Operators/data/bug-10440.php create mode 100644 tests/PHPStan/Rules/Operators/data/bug-7538.php diff --git a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php index 3f5f1d102f..001fbcd483 100644 --- a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php @@ -588,4 +588,24 @@ public function testBinaryMixed(): void ]); } + public function testBug7538(): void + { + $this->analyse([__DIR__ . '/data/bug-7538.php'], [ + [ + 'Binary operation "%" between stdClass and stdClass results in an error.', + 7, + ], + ]); + } + + public function testBug10440(): void + { + $this->analyse([__DIR__ . '/data/bug-10440.php'], [ + [ + 'Binary operation "%" between array{} and array{\'\'} results in an error.', + 8, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Operators/data/bug-10440.php b/tests/PHPStan/Rules/Operators/data/bug-10440.php new file mode 100644 index 0000000000..704cb8a695 --- /dev/null +++ b/tests/PHPStan/Rules/Operators/data/bug-10440.php @@ -0,0 +1,8 @@ + Date: Mon, 15 Jul 2024 17:55:49 +0200 Subject: [PATCH 4/5] fix binary + checking with benevolent union --- .../Operators/InvalidBinaryOperationRule.php | 2 + src/Rules/RuleLevelHelper.php | 10 +- .../InvalidBinaryOperationRuleTest.php | 190 +++++++++ .../data/binary-op-benevolent-union.php | 377 ++++++++++++++++++ 4 files changed, 578 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Operators/data/binary-op-benevolent-union.php diff --git a/src/Rules/Operators/InvalidBinaryOperationRule.php b/src/Rules/Operators/InvalidBinaryOperationRule.php index 5c687ea51d..2c42f7be1e 100644 --- a/src/Rules/Operators/InvalidBinaryOperationRule.php +++ b/src/Rules/Operators/InvalidBinaryOperationRule.php @@ -73,6 +73,8 @@ public function processNode(Node $node, Scope $scope): array if ($node instanceof Node\Expr\AssignOp\Concat || $node instanceof Node\Expr\BinaryOp\Concat) { $callback = static fn (Type $type): bool => !$type->toString() instanceof ErrorType; + } elseif ($node instanceof Node\Expr\AssignOp\Plus || $node instanceof Node\Expr\BinaryOp\Plus) { + $callback = static fn (Type $type): bool => !$type->toNumber() instanceof ErrorType || $type->isArray()->yes(); } else { $callback = static fn (Type $type): bool => !$type->toNumber() instanceof ErrorType; } diff --git a/src/Rules/RuleLevelHelper.php b/src/Rules/RuleLevelHelper.php index f3076392f6..c5a08ba5ab 100644 --- a/src/Rules/RuleLevelHelper.php +++ b/src/Rules/RuleLevelHelper.php @@ -439,7 +439,15 @@ private function findTypeToCheckImplementation( } if (count($newTypes) > 0) { - return new FoundTypeResult(TypeCombinator::union(...$newTypes), $directClassNames, [], null); + $newUnion = TypeCombinator::union(...$newTypes); + if ( + !$this->checkBenevolentUnionTypes + && $type instanceof BenevolentUnionType + ) { + $newUnion = TypeUtils::toBenevolentUnion($newUnion); + } + + return new FoundTypeResult($newUnion, $directClassNames, [], null); } } diff --git a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php index 001fbcd483..41c947e937 100644 --- a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php @@ -608,4 +608,194 @@ public function testBug10440(): void ]); } + public function testBenevolentUnion(): void + { + $this->analyse([__DIR__ . '/data/binary-op-benevolent-union.php'], [ + [ + 'Binary operation "+" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\Foo results in an error.', + 12, + ], + [ + 'Binary operation "+=" between BinaryOpBenevolentUnion\Foo and (array|bool|int|object|resource) results in an error.', + 24, + ], + [ + 'Binary operation "**" between (array|bool|int|object|resource) and array{} results in an error.', + 42, + ], + [ + 'Binary operation "**" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 43, + ], + [ + 'Binary operation "**=" between array{} and (array|bool|int|object|resource) results in an error.', + 52, + ], + [ + 'Binary operation "**=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 55, + ], + [ + 'Binary operation "*" between (array|bool|int|object|resource) and array{} results in an error.', + 73, + ], + [ + 'Binary operation "*" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 74, + ], + [ + 'Binary operation "*=" between array{} and (array|bool|int|object|resource) results in an error.', + 83, + ], + [ + 'Binary operation "*=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 86, + ], + [ + 'Binary operation "/" between (array|bool|int|object|resource) and array{} results in an error.', + 104, + ], + [ + 'Binary operation "/" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 105, + ], + [ + 'Binary operation "/=" between array{} and (array|bool|int|object|resource) results in an error.', + 114, + ], + [ + 'Binary operation "/=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 117, + ], + [ + 'Binary operation "%" between (array|bool|int|object|resource) and array{} results in an error.', + 135, + ], + [ + 'Binary operation "%" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 136, + ], + [ + 'Binary operation "%=" between array{} and (array|bool|int|object|resource) results in an error.', + 145, + ], + [ + 'Binary operation "%=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 148, + ], + [ + 'Binary operation "-" between (array|bool|int|object|resource) and array{} results in an error.', + 166, + ], + [ + 'Binary operation "-" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 167, + ], + [ + 'Binary operation "-=" between array{} and (array|bool|int|object|resource) results in an error.', + 176, + ], + [ + 'Binary operation "-=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 179, + ], + [ + 'Binary operation "." between (array|bool|int|object|resource) and array{} results in an error.', + 197, + ], + [ + 'Binary operation "." between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 198, + ], + [ + 'Binary operation ".=" between array{} and (array|bool|int|object|resource) results in an error.', + 207, + ], + [ + 'Binary operation ".=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 210, + ], + [ + 'Binary operation "<<" between (array|bool|int|object|resource) and array{} results in an error.', + 228, + ], + [ + 'Binary operation "<<" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 229, + ], + [ + 'Binary operation "<<=" between array{} and (array|bool|int|object|resource) results in an error.', + 238, + ], + [ + 'Binary operation "<<=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 241, + ], + [ + 'Binary operation ">>" between (array|bool|int|object|resource) and array{} results in an error.', + 259, + ], + [ + 'Binary operation ">>" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 260, + ], + [ + 'Binary operation ">>=" between array{} and (array|bool|int|object|resource) results in an error.', + 269, + ], + [ + 'Binary operation ">>=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 272, + ], + [ + 'Binary operation "&" between (array|bool|int|object|resource) and array{} results in an error.', + 290, + ], + [ + 'Binary operation "&" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 291, + ], + [ + 'Binary operation "&=" between array{} and (array|bool|int|object|resource) results in an error.', + 300, + ], + [ + 'Binary operation "&=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 303, + ], + [ + 'Binary operation "^" between (array|bool|int|object|resource) and array{} results in an error.', + 321, + ], + [ + 'Binary operation "^" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 322, + ], + [ + 'Binary operation "^=" between array{} and (array|bool|int|object|resource) results in an error.', + 331, + ], + [ + 'Binary operation "^=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 334, + ], + [ + 'Binary operation "|" between (array|bool|int|object|resource) and array{} results in an error.', + 352, + ], + [ + 'Binary operation "|" between (array|bool|int|object|resource) and BinaryOpBenevolentUnion\\Foo results in an error.', + 353, + ], + [ + 'Binary operation "|=" between array{} and (array|bool|int|object|resource) results in an error.', + 362, + ], + [ + 'Binary operation "|=" between BinaryOpBenevolentUnion\\Foo and (array|bool|int|object|resource) results in an error.', + 365, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Operators/data/binary-op-benevolent-union.php b/tests/PHPStan/Rules/Operators/data/binary-op-benevolent-union.php new file mode 100644 index 0000000000..1163df914a --- /dev/null +++ b/tests/PHPStan/Rules/Operators/data/binary-op-benevolent-union.php @@ -0,0 +1,377 @@ +|int|object|bool|resource> $benevolent + */ +function plus($benevolent, Foo $object): void +{ + echo $benevolent + 1; + echo $benevolent + []; + echo $benevolent + $object; + echo $benevolent + '123'; + echo $benevolent + 1.23; + echo $benevolent + $benevolent; + + $a = 1; + $a += $benevolent; + + $a = []; + $a += $benevolent; + + $a = $object; + $a += $benevolent; + + $a = '123'; + $a += $benevolent; + + $a = 1.23; + $a += $benevolent; + + $a = $benevolent; + $a += $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function exponent($benevolent, Foo $object): void +{ + echo $benevolent ** 1; + echo $benevolent ** []; + echo $benevolent ** $object; + echo $benevolent ** '123'; + echo $benevolent ** 1.23; + echo $benevolent ** $benevolent; + + $a = 1; + $a **= $benevolent; + + $a = []; + $a **= $benevolent; + + $a = $object; + $a **= $benevolent; + + $a = '123'; + $a **= $benevolent; + + $a = 1.23; + $a **= $benevolent; + + $a = $benevolent; + $a **= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function mul($benevolent, Foo $object): void +{ + echo $benevolent * 1; + echo $benevolent * []; + echo $benevolent * $object; + echo $benevolent * '123'; + echo $benevolent * 1.23; + echo $benevolent * $benevolent; + + $a = 1; + $a *= $benevolent; + + $a = []; + $a *= $benevolent; + + $a = $object; + $a *= $benevolent; + + $a = '123'; + $a *= $benevolent; + + $a = 1.23; + $a *= $benevolent; + + $a = $benevolent; + $a *= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function div($benevolent, Foo $object): void +{ + echo $benevolent / 1; + echo $benevolent / []; + echo $benevolent / $object; + echo $benevolent / '123'; + echo $benevolent / 1.23; + echo $benevolent / $benevolent; + + $a = 1; + $a /= $benevolent; + + $a = []; + $a /= $benevolent; + + $a = $object; + $a /= $benevolent; + + $a = '123'; + $a /= $benevolent; + + $a = 1.23; + $a /= $benevolent; + + $a = $benevolent; + $a /= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function mod($benevolent, Foo $object): void +{ + echo $benevolent % 1; + echo $benevolent % []; + echo $benevolent % $object; + echo $benevolent % '123'; + echo $benevolent % 1.23; + echo $benevolent % $benevolent; + + $a = 1; + $a %= $benevolent; + + $a = []; + $a %= $benevolent; + + $a = $object; + $a %= $benevolent; + + $a = '123'; + $a %= $benevolent; + + $a = 1.23; + $a %= $benevolent; + + $a = $benevolent; + $a %= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function minus($benevolent, Foo $object): void +{ + echo $benevolent - 1; + echo $benevolent - []; + echo $benevolent - $object; + echo $benevolent - '123'; + echo $benevolent - 1.23; + echo $benevolent - $benevolent; + + $a = 1; + $a -= $benevolent; + + $a = []; + $a -= $benevolent; + + $a = $object; + $a -= $benevolent; + + $a = '123'; + $a -= $benevolent; + + $a = 1.23; + $a -= $benevolent; + + $a = $benevolent; + $a -= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function concat($benevolent, Foo $object): void +{ + echo $benevolent . 1; + echo $benevolent . []; + echo $benevolent . $object; + echo $benevolent . '123'; + echo $benevolent . 1.23; + echo $benevolent . $benevolent; + + $a = 1; + $a .= $benevolent; + + $a = []; + $a .= $benevolent; + + $a = $object; + $a .= $benevolent; + + $a = '123'; + $a .= $benevolent; + + $a = 1.23; + $a .= $benevolent; + + $a = $benevolent; + $a .= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function lshift($benevolent, Foo $object): void +{ + echo $benevolent << 1; + echo $benevolent << []; + echo $benevolent << $object; + echo $benevolent << '123'; + echo $benevolent << 1.23; + echo $benevolent << $benevolent; + + $a = 1; + $a <<= $benevolent; + + $a = []; + $a <<= $benevolent; + + $a = $object; + $a <<= $benevolent; + + $a = '123'; + $a <<= $benevolent; + + $a = 1<<23; + $a <<= $benevolent; + + $a = $benevolent; + $a <<= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function rshift($benevolent, Foo $object): void +{ + echo $benevolent >> 1; + echo $benevolent >> []; + echo $benevolent >> $object; + echo $benevolent >> '123'; + echo $benevolent >> 1.23; + echo $benevolent >> $benevolent; + + $a = 1; + $a >>= $benevolent; + + $a = []; + $a >>= $benevolent; + + $a = $object; + $a >>= $benevolent; + + $a = '123'; + $a >>= $benevolent; + + $a = 1>>23; + $a >>= $benevolent; + + $a = $benevolent; + $a >>= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function bitAnd($benevolent, Foo $object): void +{ + echo $benevolent & 1; + echo $benevolent & []; + echo $benevolent & $object; + echo $benevolent & '123'; + echo $benevolent & 1.23; + echo $benevolent & $benevolent; + + $a = 1; + $a &= $benevolent; + + $a = []; + $a &= $benevolent; + + $a = $object; + $a &= $benevolent; + + $a = '123'; + $a &= $benevolent; + + $a = 1.23; + $a &= $benevolent; + + $a = $benevolent; + $a &= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function bitXor($benevolent, Foo $object): void +{ + echo $benevolent ^ 1; + echo $benevolent ^ []; + echo $benevolent ^ $object; + echo $benevolent ^ '123'; + echo $benevolent ^ 1.23; + echo $benevolent ^ $benevolent; + + $a = 1; + $a ^= $benevolent; + + $a = []; + $a ^= $benevolent; + + $a = $object; + $a ^= $benevolent; + + $a = '123'; + $a ^= $benevolent; + + $a = 1.23; + $a ^= $benevolent; + + $a = $benevolent; + $a ^= $benevolent; +} + +/** + * @param __benevolent|int|object|bool|resource> $benevolent + */ +function bitOr($benevolent, Foo $object): void +{ + echo $benevolent | 1; + echo $benevolent | []; + echo $benevolent | $object; + echo $benevolent | '123'; + echo $benevolent | 1.23; + echo $benevolent | $benevolent; + + $a = 1; + $a |= $benevolent; + + $a = []; + $a |= $benevolent; + + $a = $object; + $a |= $benevolent; + + $a = '123'; + $a |= $benevolent; + + $a = 1.23; + $a |= $benevolent; + + $a = $benevolent; + $a |= $benevolent; +} + +class Foo {} From 41b68c167bc47ee8efbf45af140cc86d9f193d79 Mon Sep 17 00:00:00 2001 From: schlndh Date: Mon, 15 Jul 2024 19:40:34 +0200 Subject: [PATCH 5/5] revert unnecessary Registry change --- src/Collectors/Registry.php | 12 +----------- src/Rules/DirectRegistry.php | 12 +----------- src/Rules/LazyRegistry.php | 12 +----------- 3 files changed, 3 insertions(+), 33 deletions(-) diff --git a/src/Collectors/Registry.php b/src/Collectors/Registry.php index 9bb7123322..65b80118b2 100644 --- a/src/Collectors/Registry.php +++ b/src/Collectors/Registry.php @@ -35,17 +35,7 @@ public function __construct(array $collectors) public function getCollectors(string $nodeType): array { if (!isset($this->cache[$nodeType])) { - $parents = class_parents($nodeType); - if ($parents === false) { - $parents = []; - } - - $implements = class_implements($nodeType); - if ($implements === false) { - $implements = []; - } - - $parentNodeTypes = [$nodeType] + $parents + $implements; + $parentNodeTypes = [$nodeType] + class_parents($nodeType) + class_implements($nodeType); $collectors = []; foreach ($parentNodeTypes as $parentNodeType) { diff --git a/src/Rules/DirectRegistry.php b/src/Rules/DirectRegistry.php index 16550a08c2..7ee53824ee 100644 --- a/src/Rules/DirectRegistry.php +++ b/src/Rules/DirectRegistry.php @@ -35,17 +35,7 @@ public function __construct(array $rules) public function getRules(string $nodeType): array { if (!isset($this->cache[$nodeType])) { - $parents = class_parents($nodeType); - if ($parents === false) { - $parents = []; - } - - $implements = class_implements($nodeType); - if ($implements === false) { - $implements = []; - } - - $parentNodeTypes = [$nodeType] + $parents + $implements; + $parentNodeTypes = [$nodeType] + class_parents($nodeType) + class_implements($nodeType); $rules = []; foreach ($parentNodeTypes as $parentNodeType) { diff --git a/src/Rules/LazyRegistry.php b/src/Rules/LazyRegistry.php index 674b05ed2b..a9a1a728ec 100644 --- a/src/Rules/LazyRegistry.php +++ b/src/Rules/LazyRegistry.php @@ -32,17 +32,7 @@ public function __construct(private Container $container) public function getRules(string $nodeType): array { if (!isset($this->cache[$nodeType])) { - $parents = class_parents($nodeType); - if ($parents === false) { - $parents = []; - } - - $implements = class_implements($nodeType); - if ($implements === false) { - $implements = []; - } - - $parentNodeTypes = [$nodeType] + $parents + $implements; + $parentNodeTypes = [$nodeType] + class_parents($nodeType) + class_implements($nodeType); $rules = []; $rulesFromContainer = $this->getRulesFromContainer();