Skip to content

Commit aeb85e6

Browse files
schlndhschlndh
committed
Bleeding edge - improve comparison against BcMath\Number
1 parent f61439f commit aeb85e6

10 files changed

+221
-62
lines changed

‎conf/bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ parameters:
66
stricterFunctionMap: true
77
reportPreciseLineForUnusedFunctionParameter: true
88
internalTag: true
9+
checkExtensionsForComparisonOperators: true

‎conf/config.level2.neon

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ rules:
4343
- PHPStan\Rules\Methods\CallPrivateMethodThroughStaticRule
4444
- PHPStan\Rules\Methods\IncompatibleDefaultParameterTypeRule
4545
- PHPStan\Rules\Operators\InvalidBinaryOperationRule
46-
- PHPStan\Rules\Operators\InvalidComparisonOperationRule
4746
- PHPStan\Rules\Operators\InvalidUnaryOperationRule
4847
- PHPStan\Rules\PhpDoc\FunctionConditionalReturnTypeRule
4948
- PHPStan\Rules\PhpDoc\MethodConditionalReturnTypeRule
@@ -122,3 +121,9 @@ services:
122121

123122
-
124123
class: PHPStan\Rules\InternalTag\RestrictedInternalMethodUsageExtension
124+
-
125+
class: PHPStan\Rules\Operators\InvalidComparisonOperationRule
126+
arguments:
127+
checkExtensionsForComparisonOperators: %featureToggles.checkExtensionsForComparisonOperators%
128+
tags:
129+
- phpstan.rules.rule

‎conf/config.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ parameters:
2727
stricterFunctionMap: false
2828
reportPreciseLineForUnusedFunctionParameter: false
2929
internalTag: false
30+
checkExtensionsForComparisonOperators: false
3031
fileExtensions:
3132
- php
3233
checkAdvancedIsset: false

‎conf/parametersSchema.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ parametersSchema:
3333
stricterFunctionMap: bool()
3434
reportPreciseLineForUnusedFunctionParameter: bool()
3535
internalTag: bool()
36+
checkExtensionsForComparisonOperators: bool()
3637
])
3738
fileExtensions: listOf(string())
3839
checkAdvancedIsset: bool()

‎src/Reflection/InitializerExprTypeResolver.php

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -872,7 +872,8 @@ public function getModType(Expr $left, Expr $right, callable $getTypeCallback):
872872
return $this->getNeverType($leftType, $rightType);
873873
}
874874

875-
$extensionSpecified = $this->callOperatorTypeSpecifyingExtensions(new BinaryOp\Mod($left, $right), $leftType, $rightType);
875+
$extensionSpecified = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()
876+
->callOperatorTypeSpecifyingExtensions(new BinaryOp\Mod($left, $right), $leftType, $rightType);
876877
if ($extensionSpecified !== null) {
877878
return $extensionSpecified;
878879
}
@@ -1239,7 +1240,8 @@ public function getPowType(Expr $left, Expr $right, callable $getTypeCallback):
12391240
$leftType = $getTypeCallback($left);
12401241
$rightType = $getTypeCallback($right);
12411242

1242-
$extensionSpecified = $this->callOperatorTypeSpecifyingExtensions(new BinaryOp\Pow($left, $right), $leftType, $rightType);
1243+
$extensionSpecified = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()
1244+
->callOperatorTypeSpecifyingExtensions(new BinaryOp\Pow($left, $right), $leftType, $rightType);
12431245
if ($extensionSpecified !== null) {
12441246
return $extensionSpecified;
12451247
}
@@ -1492,25 +1494,6 @@ private function resolveConstantArrayTypeComparison(ConstantArrayType $leftType,
14921494
return new TypeResult($resultType->toBoolean(), []);
14931495
}
14941496

1495-
private function callOperatorTypeSpecifyingExtensions(Expr\BinaryOp $expr, Type $leftType, Type $rightType): ?Type
1496-
{
1497-
$operatorSigil = $expr->getOperatorSigil();
1498-
$operatorTypeSpecifyingExtensions = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()->getOperatorTypeSpecifyingExtensions($operatorSigil, $leftType, $rightType);
1499-
1500-
/** @var Type[] $extensionTypes */
1501-
$extensionTypes = [];
1502-
1503-
foreach ($operatorTypeSpecifyingExtensions as $extension) {
1504-
$extensionTypes[] = $extension->specifyType($operatorSigil, $leftType, $rightType);
1505-
}
1506-
1507-
if (count($extensionTypes) > 0) {
1508-
return TypeCombinator::union(...$extensionTypes);
1509-
}
1510-
1511-
return null;
1512-
}
1513-
15141497
/**
15151498
* @param BinaryOp\Plus|BinaryOp\Minus|BinaryOp\Mul|BinaryOp\Div|BinaryOp\ShiftLeft|BinaryOp\ShiftRight $expr
15161499
*/
@@ -1555,7 +1538,8 @@ private function resolveCommonMath(Expr\BinaryOp $expr, Type $leftType, Type $ri
15551538
}
15561539
}
15571540

1558-
$specifiedTypes = $this->callOperatorTypeSpecifyingExtensions($expr, $leftType, $rightType);
1541+
$specifiedTypes = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()
1542+
->callOperatorTypeSpecifyingExtensions($expr, $leftType, $rightType);
15591543
if ($specifiedTypes !== null) {
15601544
return $specifiedTypes;
15611545
}

‎src/Rules/Operators/InvalidComparisonOperationRule.php

Lines changed: 66 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
7+
use PHPStan\DependencyInjection\Type\OperatorTypeSpecifyingExtensionRegistryProvider;
8+
use PHPStan\Rules\IdentifierRuleError;
79
use PHPStan\Rules\Rule;
810
use PHPStan\Rules\RuleErrorBuilder;
911
use PHPStan\Rules\RuleLevelHelper;
@@ -26,7 +28,11 @@
2628
final class InvalidComparisonOperationRule implements Rule
2729
{
2830

29-
public function __construct(private RuleLevelHelper $ruleLevelHelper)
31+
public function __construct(
32+
private RuleLevelHelper $ruleLevelHelper,
33+
private OperatorTypeSpecifyingExtensionRegistryProvider $operatorTypeSpecifyingExtensionRegistryProvider,
34+
private bool $checkExtensionsForComparisonOperators,
35+
)
3036
{
3137
}
3238

@@ -53,6 +59,22 @@ public function processNode(Node $node, Scope $scope): array
5359
return [];
5460
}
5561

62+
$result = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()->callOperatorTypeSpecifyingExtensions(
63+
$node,
64+
$scope->getType($node->left),
65+
$scope->getType($node->right),
66+
);
67+
68+
if ($result !== null) {
69+
if (! $result instanceof ErrorType) {
70+
return [];
71+
}
72+
73+
if ($this->checkExtensionsForComparisonOperators) {
74+
return $this->createError($node, $scope);
75+
}
76+
}
77+
5678
if (
5779
($this->isNumberType($scope, $node->left) && (
5880
$this->isPossiblyNullableObjectType($scope, $node->right) || $this->isPossiblyNullableArrayType($scope, $node->right)
@@ -61,43 +83,7 @@ public function processNode(Node $node, Scope $scope): array
6183
$this->isPossiblyNullableObjectType($scope, $node->left) || $this->isPossiblyNullableArrayType($scope, $node->left)
6284
))
6385
) {
64-
switch (get_class($node)) {
65-
case Node\Expr\BinaryOp\Equal::class:
66-
$nodeType = 'equal';
67-
break;
68-
case Node\Expr\BinaryOp\NotEqual::class:
69-
$nodeType = 'notEqual';
70-
break;
71-
case Node\Expr\BinaryOp\Greater::class:
72-
$nodeType = 'greater';
73-
break;
74-
case Node\Expr\BinaryOp\GreaterOrEqual::class:
75-
$nodeType = 'greaterOrEqual';
76-
break;
77-
case Node\Expr\BinaryOp\Smaller::class:
78-
$nodeType = 'smaller';
79-
break;
80-
case Node\Expr\BinaryOp\SmallerOrEqual::class:
81-
$nodeType = 'smallerOrEqual';
82-
break;
83-
case Node\Expr\BinaryOp\Spaceship::class:
84-
$nodeType = 'spaceship';
85-
break;
86-
default:
87-
throw new ShouldNotHappenException();
88-
}
89-
90-
return [
91-
RuleErrorBuilder::message(sprintf(
92-
'Comparison operation "%s" between %s and %s results in an error.',
93-
$node->getOperatorSigil(),
94-
$scope->getType($node->left)->describe(VerbosityLevel::value()),
95-
$scope->getType($node->right)->describe(VerbosityLevel::value()),
96-
))
97-
->line($node->left->getStartLine())
98-
->identifier(sprintf('%s.invalid', $nodeType))
99-
->build(),
100-
];
86+
return $this->createError($node, $scope);
10187
}
10288

10389
return [];
@@ -164,4 +150,46 @@ private function isPossiblyNullableArrayType(Scope $scope, Node\Expr $expr): boo
164150
return !($type instanceof ErrorType) && $type->isArray()->yes();
165151
}
166152

153+
/** @return list<IdentifierRuleError> */
154+
private function createError(Node\Expr\BinaryOp $node, Scope $scope): array
155+
{
156+
switch (get_class($node)) {
157+
case Node\Expr\BinaryOp\Equal::class:
158+
$nodeType = 'equal';
159+
break;
160+
case Node\Expr\BinaryOp\NotEqual::class:
161+
$nodeType = 'notEqual';
162+
break;
163+
case Node\Expr\BinaryOp\Greater::class:
164+
$nodeType = 'greater';
165+
break;
166+
case Node\Expr\BinaryOp\GreaterOrEqual::class:
167+
$nodeType = 'greaterOrEqual';
168+
break;
169+
case Node\Expr\BinaryOp\Smaller::class:
170+
$nodeType = 'smaller';
171+
break;
172+
case Node\Expr\BinaryOp\SmallerOrEqual::class:
173+
$nodeType = 'smallerOrEqual';
174+
break;
175+
case Node\Expr\BinaryOp\Spaceship::class:
176+
$nodeType = 'spaceship';
177+
break;
178+
default:
179+
throw new ShouldNotHappenException();
180+
}
181+
182+
return [
183+
RuleErrorBuilder::message(sprintf(
184+
'Comparison operation "%s" between %s and %s results in an error.',
185+
$node->getOperatorSigil(),
186+
$scope->getType($node->left)->describe(VerbosityLevel::value()),
187+
$scope->getType($node->right)->describe(VerbosityLevel::value()),
188+
))
189+
->line($node->left->getStartLine())
190+
->identifier(sprintf('%s.invalid', $nodeType))
191+
->build(),
192+
];
193+
}
194+
167195
}

‎src/Type/OperatorTypeSpecifyingExtensionRegistry.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22

33
namespace PHPStan\Type;
44

5+
use PhpParser\Node\Expr;
56
use function array_filter;
67
use function array_values;
8+
use function count;
79

810
final class OperatorTypeSpecifyingExtensionRegistry
911
{
@@ -25,4 +27,23 @@ public function getOperatorTypeSpecifyingExtensions(string $operator, Type $left
2527
return array_values(array_filter($this->extensions, static fn (OperatorTypeSpecifyingExtension $extension): bool => $extension->isOperatorSupported($operator, $leftType, $rightType)));
2628
}
2729

30+
public function callOperatorTypeSpecifyingExtensions(Expr\BinaryOp $expr, Type $leftType, Type $rightType): ?Type
31+
{
32+
$operatorSigil = $expr->getOperatorSigil();
33+
$operatorTypeSpecifyingExtensions = $this->getOperatorTypeSpecifyingExtensions($operatorSigil, $leftType, $rightType);
34+
35+
/** @var Type[] $extensionTypes */
36+
$extensionTypes = [];
37+
38+
foreach ($operatorTypeSpecifyingExtensions as $extension) {
39+
$extensionTypes[] = $extension->specifyType($operatorSigil, $leftType, $rightType);
40+
}
41+
42+
if (count($extensionTypes) > 0) {
43+
return TypeCombinator::union(...$extensionTypes);
44+
}
45+
46+
return null;
47+
}
48+
2849
}

‎src/Type/Php/BcMathNumberOperatorTypeSpecifyingExtension.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
namespace PHPStan\Type\Php;
44

55
use PHPStan\Php\PhpVersion;
6+
use PHPStan\Type\BooleanType;
67
use PHPStan\Type\ErrorType;
8+
use PHPStan\Type\IntegerRangeType;
79
use PHPStan\Type\NeverType;
810
use PHPStan\Type\ObjectType;
911
use PHPStan\Type\OperatorTypeSpecifyingExtension;
@@ -25,7 +27,7 @@ public function isOperatorSupported(string $operatorSigil, Type $leftSide, Type
2527

2628
$bcMathNumberType = new ObjectType('BcMath\Number');
2729

28-
return in_array($operatorSigil, ['-', '+', '*', '/', '**', '%'], true)
30+
return in_array($operatorSigil, ['-', '+', '*', '/', '**', '%', '<', '<=', '>', '>=', '==', '!=', '<=>'], true)
2931
&& (
3032
$bcMathNumberType->isSuperTypeOf($leftSide)->yes()
3133
|| $bcMathNumberType->isSuperTypeOf($rightSide)->yes()
@@ -39,6 +41,18 @@ public function specifyType(string $operatorSigil, Type $leftSide, Type $rightSi
3941
? $rightSide
4042
: $leftSide;
4143

44+
if ($otherSide->isFloat()->yes()) {
45+
return new ErrorType();
46+
}
47+
48+
if (in_array($operatorSigil, ['<', '<=', '>', '>=', '==', '!='], true)) {
49+
return new BooleanType();
50+
}
51+
52+
if ($operatorSigil === '<=>') {
53+
return IntegerRangeType::fromInterval(-1, 1);
54+
}
55+
4256
if (
4357
$otherSide->isInteger()->yes()
4458
|| $otherSide->isNumericString()->yes()

‎tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Operators;
44

5+
use PHPStan\DependencyInjection\Type\OperatorTypeSpecifyingExtensionRegistryProvider;
56
use PHPStan\Rules\Rule;
67
use PHPStan\Rules\RuleLevelHelper;
78
use PHPStan\Testing\RuleTestCase;
@@ -17,6 +18,8 @@ protected function getRule(): Rule
1718
{
1819
return new InvalidComparisonOperationRule(
1920
new RuleLevelHelper($this->createReflectionProvider(), true, false, true, false, false, false, true),
21+
$this->getContainer()->getByType(OperatorTypeSpecifyingExtensionRegistryProvider::class),
22+
true,
2023
);
2124
}
2225

@@ -173,4 +176,18 @@ public function testBug11119(): void
173176
$this->analyse([__DIR__ . '/data/bug-11119.php'], []);
174177
}
175178

179+
public function testBug13001(): void
180+
{
181+
$this->analyse([__DIR__ . '/data/bug-13001.php'], [
182+
[
183+
'Comparison operation ">" between BcMath\\Number and 0.2 results in an error.',
184+
10,
185+
],
186+
[
187+
'Comparison operation "<=>" between 0.2 and BcMath\\Number results in an error.',
188+
11,
189+
],
190+
]);
191+
}
192+
176193
}

0 commit comments

Comments
 (0)