diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 22487e357c..eda668ba6a 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -6,3 +6,4 @@ parameters: stricterFunctionMap: true reportPreciseLineForUnusedFunctionParameter: true internalTag: true + checkExtensionsForComparisonOperators: true diff --git a/conf/config.level2.neon b/conf/config.level2.neon index 51214ea554..66633be236 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -43,7 +43,6 @@ rules: - PHPStan\Rules\Methods\CallPrivateMethodThroughStaticRule - PHPStan\Rules\Methods\IncompatibleDefaultParameterTypeRule - PHPStan\Rules\Operators\InvalidBinaryOperationRule - - PHPStan\Rules\Operators\InvalidComparisonOperationRule - PHPStan\Rules\Operators\InvalidUnaryOperationRule - PHPStan\Rules\PhpDoc\FunctionConditionalReturnTypeRule - PHPStan\Rules\PhpDoc\MethodConditionalReturnTypeRule @@ -122,3 +121,9 @@ services: - class: PHPStan\Rules\InternalTag\RestrictedInternalMethodUsageExtension + - + class: PHPStan\Rules\Operators\InvalidComparisonOperationRule + arguments: + checkExtensionsForComparisonOperators: %featureToggles.checkExtensionsForComparisonOperators% + tags: + - phpstan.rules.rule diff --git a/conf/config.neon b/conf/config.neon index 26703d01ef..52056b9fcf 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -27,6 +27,7 @@ parameters: stricterFunctionMap: false reportPreciseLineForUnusedFunctionParameter: false internalTag: false + checkExtensionsForComparisonOperators: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index d18df776e3..7c8dd66b7c 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -33,6 +33,7 @@ parametersSchema: stricterFunctionMap: bool() reportPreciseLineForUnusedFunctionParameter: bool() internalTag: bool() + checkExtensionsForComparisonOperators: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Reflection/InitializerExprTypeResolver.php b/src/Reflection/InitializerExprTypeResolver.php index fb935ac630..c8539eafc4 100644 --- a/src/Reflection/InitializerExprTypeResolver.php +++ b/src/Reflection/InitializerExprTypeResolver.php @@ -872,7 +872,8 @@ public function getModType(Expr $left, Expr $right, callable $getTypeCallback): return $this->getNeverType($leftType, $rightType); } - $extensionSpecified = $this->callOperatorTypeSpecifyingExtensions(new BinaryOp\Mod($left, $right), $leftType, $rightType); + $extensionSpecified = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry() + ->callOperatorTypeSpecifyingExtensions(new BinaryOp\Mod($left, $right), $leftType, $rightType); if ($extensionSpecified !== null) { return $extensionSpecified; } @@ -1239,7 +1240,8 @@ public function getPowType(Expr $left, Expr $right, callable $getTypeCallback): $leftType = $getTypeCallback($left); $rightType = $getTypeCallback($right); - $extensionSpecified = $this->callOperatorTypeSpecifyingExtensions(new BinaryOp\Pow($left, $right), $leftType, $rightType); + $extensionSpecified = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry() + ->callOperatorTypeSpecifyingExtensions(new BinaryOp\Pow($left, $right), $leftType, $rightType); if ($extensionSpecified !== null) { return $extensionSpecified; } @@ -1492,25 +1494,6 @@ private function resolveConstantArrayTypeComparison(ConstantArrayType $leftType, return new TypeResult($resultType->toBoolean(), []); } - private function callOperatorTypeSpecifyingExtensions(Expr\BinaryOp $expr, Type $leftType, Type $rightType): ?Type - { - $operatorSigil = $expr->getOperatorSigil(); - $operatorTypeSpecifyingExtensions = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()->getOperatorTypeSpecifyingExtensions($operatorSigil, $leftType, $rightType); - - /** @var Type[] $extensionTypes */ - $extensionTypes = []; - - foreach ($operatorTypeSpecifyingExtensions as $extension) { - $extensionTypes[] = $extension->specifyType($operatorSigil, $leftType, $rightType); - } - - if (count($extensionTypes) > 0) { - return TypeCombinator::union(...$extensionTypes); - } - - return null; - } - /** * @param BinaryOp\Plus|BinaryOp\Minus|BinaryOp\Mul|BinaryOp\Div|BinaryOp\ShiftLeft|BinaryOp\ShiftRight $expr */ @@ -1555,7 +1538,8 @@ private function resolveCommonMath(Expr\BinaryOp $expr, Type $leftType, Type $ri } } - $specifiedTypes = $this->callOperatorTypeSpecifyingExtensions($expr, $leftType, $rightType); + $specifiedTypes = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry() + ->callOperatorTypeSpecifyingExtensions($expr, $leftType, $rightType); if ($specifiedTypes !== null) { return $specifiedTypes; } diff --git a/src/Rules/Operators/InvalidComparisonOperationRule.php b/src/Rules/Operators/InvalidComparisonOperationRule.php index 8dc06429e5..a7ce74b091 100644 --- a/src/Rules/Operators/InvalidComparisonOperationRule.php +++ b/src/Rules/Operators/InvalidComparisonOperationRule.php @@ -4,6 +4,8 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; +use PHPStan\DependencyInjection\Type\OperatorTypeSpecifyingExtensionRegistryProvider; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; @@ -26,7 +28,11 @@ final class InvalidComparisonOperationRule implements Rule { - public function __construct(private RuleLevelHelper $ruleLevelHelper) + public function __construct( + private RuleLevelHelper $ruleLevelHelper, + private OperatorTypeSpecifyingExtensionRegistryProvider $operatorTypeSpecifyingExtensionRegistryProvider, + private bool $checkExtensionsForComparisonOperators, + ) { } @@ -53,6 +59,22 @@ public function processNode(Node $node, Scope $scope): array return []; } + $result = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()->callOperatorTypeSpecifyingExtensions( + $node, + $scope->getType($node->left), + $scope->getType($node->right), + ); + + if ($result !== null) { + if (! $result instanceof ErrorType) { + return []; + } + + if ($this->checkExtensionsForComparisonOperators) { + return $this->createError($node, $scope); + } + } + if ( ($this->isNumberType($scope, $node->left) && ( $this->isPossiblyNullableObjectType($scope, $node->right) || $this->isPossiblyNullableArrayType($scope, $node->right) @@ -61,43 +83,7 @@ public function processNode(Node $node, Scope $scope): array $this->isPossiblyNullableObjectType($scope, $node->left) || $this->isPossiblyNullableArrayType($scope, $node->left) )) ) { - switch (get_class($node)) { - case Node\Expr\BinaryOp\Equal::class: - $nodeType = 'equal'; - break; - case Node\Expr\BinaryOp\NotEqual::class: - $nodeType = 'notEqual'; - break; - case Node\Expr\BinaryOp\Greater::class: - $nodeType = 'greater'; - break; - case Node\Expr\BinaryOp\GreaterOrEqual::class: - $nodeType = 'greaterOrEqual'; - break; - case Node\Expr\BinaryOp\Smaller::class: - $nodeType = 'smaller'; - break; - case Node\Expr\BinaryOp\SmallerOrEqual::class: - $nodeType = 'smallerOrEqual'; - break; - case Node\Expr\BinaryOp\Spaceship::class: - $nodeType = 'spaceship'; - break; - default: - throw new ShouldNotHappenException(); - } - - return [ - RuleErrorBuilder::message(sprintf( - 'Comparison operation "%s" between %s and %s results in an error.', - $node->getOperatorSigil(), - $scope->getType($node->left)->describe(VerbosityLevel::value()), - $scope->getType($node->right)->describe(VerbosityLevel::value()), - )) - ->line($node->left->getStartLine()) - ->identifier(sprintf('%s.invalid', $nodeType)) - ->build(), - ]; + return $this->createError($node, $scope); } return []; @@ -164,4 +150,46 @@ private function isPossiblyNullableArrayType(Scope $scope, Node\Expr $expr): boo return !($type instanceof ErrorType) && $type->isArray()->yes(); } + /** @return list */ + private function createError(Node\Expr\BinaryOp $node, Scope $scope): array + { + switch (get_class($node)) { + case Node\Expr\BinaryOp\Equal::class: + $nodeType = 'equal'; + break; + case Node\Expr\BinaryOp\NotEqual::class: + $nodeType = 'notEqual'; + break; + case Node\Expr\BinaryOp\Greater::class: + $nodeType = 'greater'; + break; + case Node\Expr\BinaryOp\GreaterOrEqual::class: + $nodeType = 'greaterOrEqual'; + break; + case Node\Expr\BinaryOp\Smaller::class: + $nodeType = 'smaller'; + break; + case Node\Expr\BinaryOp\SmallerOrEqual::class: + $nodeType = 'smallerOrEqual'; + break; + case Node\Expr\BinaryOp\Spaceship::class: + $nodeType = 'spaceship'; + break; + default: + throw new ShouldNotHappenException(); + } + + return [ + RuleErrorBuilder::message(sprintf( + 'Comparison operation "%s" between %s and %s results in an error.', + $node->getOperatorSigil(), + $scope->getType($node->left)->describe(VerbosityLevel::value()), + $scope->getType($node->right)->describe(VerbosityLevel::value()), + )) + ->line($node->left->getStartLine()) + ->identifier(sprintf('%s.invalid', $nodeType)) + ->build(), + ]; + } + } diff --git a/src/Type/OperatorTypeSpecifyingExtensionRegistry.php b/src/Type/OperatorTypeSpecifyingExtensionRegistry.php index 7b84648f28..11f65849b7 100644 --- a/src/Type/OperatorTypeSpecifyingExtensionRegistry.php +++ b/src/Type/OperatorTypeSpecifyingExtensionRegistry.php @@ -2,8 +2,10 @@ namespace PHPStan\Type; +use PhpParser\Node\Expr; use function array_filter; use function array_values; +use function count; final class OperatorTypeSpecifyingExtensionRegistry { @@ -25,4 +27,23 @@ public function getOperatorTypeSpecifyingExtensions(string $operator, Type $left return array_values(array_filter($this->extensions, static fn (OperatorTypeSpecifyingExtension $extension): bool => $extension->isOperatorSupported($operator, $leftType, $rightType))); } + public function callOperatorTypeSpecifyingExtensions(Expr\BinaryOp $expr, Type $leftType, Type $rightType): ?Type + { + $operatorSigil = $expr->getOperatorSigil(); + $operatorTypeSpecifyingExtensions = $this->getOperatorTypeSpecifyingExtensions($operatorSigil, $leftType, $rightType); + + /** @var Type[] $extensionTypes */ + $extensionTypes = []; + + foreach ($operatorTypeSpecifyingExtensions as $extension) { + $extensionTypes[] = $extension->specifyType($operatorSigil, $leftType, $rightType); + } + + if (count($extensionTypes) > 0) { + return TypeCombinator::union(...$extensionTypes); + } + + return null; + } + } diff --git a/src/Type/Php/BcMathNumberOperatorTypeSpecifyingExtension.php b/src/Type/Php/BcMathNumberOperatorTypeSpecifyingExtension.php index 3e0d793052..24a3731c56 100644 --- a/src/Type/Php/BcMathNumberOperatorTypeSpecifyingExtension.php +++ b/src/Type/Php/BcMathNumberOperatorTypeSpecifyingExtension.php @@ -3,7 +3,9 @@ namespace PHPStan\Type\Php; use PHPStan\Php\PhpVersion; +use PHPStan\Type\BooleanType; use PHPStan\Type\ErrorType; +use PHPStan\Type\IntegerRangeType; use PHPStan\Type\NeverType; use PHPStan\Type\ObjectType; use PHPStan\Type\OperatorTypeSpecifyingExtension; @@ -25,7 +27,7 @@ public function isOperatorSupported(string $operatorSigil, Type $leftSide, Type $bcMathNumberType = new ObjectType('BcMath\Number'); - return in_array($operatorSigil, ['-', '+', '*', '/', '**', '%'], true) + return in_array($operatorSigil, ['-', '+', '*', '/', '**', '%', '<', '<=', '>', '>=', '==', '!=', '<=>'], true) && ( $bcMathNumberType->isSuperTypeOf($leftSide)->yes() || $bcMathNumberType->isSuperTypeOf($rightSide)->yes() @@ -39,6 +41,18 @@ public function specifyType(string $operatorSigil, Type $leftSide, Type $rightSi ? $rightSide : $leftSide; + if ($otherSide->isFloat()->yes()) { + return new ErrorType(); + } + + if (in_array($operatorSigil, ['<', '<=', '>', '>=', '==', '!='], true)) { + return new BooleanType(); + } + + if ($operatorSigil === '<=>') { + return IntegerRangeType::fromInterval(-1, 1); + } + if ( $otherSide->isInteger()->yes() || $otherSide->isNumericString()->yes() diff --git a/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php index 3305d725c4..2e653f54ab 100644 --- a/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Operators; +use PHPStan\DependencyInjection\Type\OperatorTypeSpecifyingExtensionRegistryProvider; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; @@ -17,6 +18,8 @@ protected function getRule(): Rule { return new InvalidComparisonOperationRule( new RuleLevelHelper($this->createReflectionProvider(), true, false, true, false, false, false, true), + $this->getContainer()->getByType(OperatorTypeSpecifyingExtensionRegistryProvider::class), + true, ); } @@ -173,4 +176,22 @@ public function testBug11119(): void $this->analyse([__DIR__ . '/data/bug-11119.php'], []); } + public function testBug13001(): void + { + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4.'); + } + + $this->analyse([__DIR__ . '/data/bug-13001.php'], [ + [ + 'Comparison operation ">" between BcMath\\Number and 0.2 results in an error.', + 10, + ], + [ + 'Comparison operation "<=>" between 0.2 and BcMath\\Number results in an error.', + 11, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Operators/data/bug-13001.php b/tests/PHPStan/Rules/Operators/data/bug-13001.php new file mode 100644 index 0000000000..bd8ac1b2f5 --- /dev/null +++ b/tests/PHPStan/Rules/Operators/data/bug-13001.php @@ -0,0 +1,87 @@ += 8.4 + +namespace Bug13001; + +$bcNumber = new \BcMath\Number(-1); + +// invalid + +var_dump( + $bcNumber > 0.2, + 0.2 <=> $bcNumber, +); + +// probably invalid, but PHPStan currently allows these comparisons: + +var_dump( + $bcNumber < true, + null <= $bcNumber, + fopen('php://stdin', 'r') >= $bcNumber, + new \stdClass() == $bcNumber, +); + +// valid + +var_dump( + $bcNumber < 0, + $bcNumber < '0.2', + $bcNumber < new \BcMath\Number(3), +); +var_dump( + $bcNumber <= 0, + $bcNumber <= '0.2', + $bcNumber <= new \BcMath\Number(3), +); +var_dump( + $bcNumber > 0, + $bcNumber > '0.2', + $bcNumber > new \BcMath\Number(3), +); +var_dump( + $bcNumber >= 0, + $bcNumber >= '0.2', + $bcNumber >= new \BcMath\Number(3), +); +var_dump( + $bcNumber == 0, + $bcNumber == '0.2', + $bcNumber == new \BcMath\Number(3), +); +var_dump( + $bcNumber != 0, + $bcNumber != '0.2', + $bcNumber != new \BcMath\Number(3), +); +var_dump( + $bcNumber <=> 0, + $bcNumber <=> '0.2', + $bcNumber <=> new \BcMath\Number(3), +); +var_dump( + 0 < $bcNumber, + '0.2' < $bcNumber, +); +var_dump( + 0 <= $bcNumber, + '0.2' <= $bcNumber, +); +var_dump( + 0 > $bcNumber, + '0.2' > $bcNumber, +); +var_dump( + 0 >= $bcNumber, + '0.2' >= $bcNumber, +); +var_dump( + 0 == $bcNumber, + '0.2' == $bcNumber, +); +var_dump( + 0 != $bcNumber, + '0.2' != $bcNumber, +); +var_dump( + 0 <=> $bcNumber, + '0.2' <=> $bcNumber, +);