Skip to content

Commit 569c936

Browse files
schlndhschlndh
committed
Bleeding edge - improve comparison against BcMath\Number
1 parent a758cf3 commit 569c936

9 files changed

+217
-61
lines changed

‎conf/bleedingEdge.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ parameters:
88
reportPreciseLineForUnusedFunctionParameter: true
99
internalTag: true
1010
newStaticInAbstractClassStaticMethod: true
11+
checkExtensionsForComparisonOperators: true

‎conf/config.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ parameters:
3131
reportPreciseLineForUnusedFunctionParameter: false
3232
internalTag: false
3333
newStaticInAbstractClassStaticMethod: false
34+
checkExtensionsForComparisonOperators: false
3435
fileExtensions:
3536
- php
3637
checkAdvancedIsset: false

‎conf/parametersSchema.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ parametersSchema:
3535
reportPreciseLineForUnusedFunctionParameter: bool()
3636
internalTag: bool()
3737
newStaticInAbstractClassStaticMethod: bool()
38+
checkExtensionsForComparisonOperators: bool()
3839
])
3940
fileExtensions: listOf(string())
4041
checkAdvancedIsset: bool()

‎src/Reflection/InitializerExprTypeResolver.php‎

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

887-
$extensionSpecified = $this->callOperatorTypeSpecifyingExtensions(new BinaryOp\Mod($left, $right), $leftType, $rightType);
887+
$extensionSpecified = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()
888+
->callOperatorTypeSpecifyingExtensions(new BinaryOp\Mod($left, $right), $leftType, $rightType);
888889
if ($extensionSpecified !== null) {
889890
return $extensionSpecified;
890891
}
@@ -1259,7 +1260,8 @@ public function getPowType(Expr $left, Expr $right, callable $getTypeCallback):
12591260
$leftType = $getTypeCallback($left);
12601261
$rightType = $getTypeCallback($right);
12611262

1262-
$extensionSpecified = $this->callOperatorTypeSpecifyingExtensions(new BinaryOp\Pow($left, $right), $leftType, $rightType);
1263+
$extensionSpecified = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()
1264+
->callOperatorTypeSpecifyingExtensions(new BinaryOp\Pow($left, $right), $leftType, $rightType);
12631265
if ($extensionSpecified !== null) {
12641266
return $extensionSpecified;
12651267
}
@@ -1543,25 +1545,6 @@ private function resolveConstantArrayTypeComparison(ConstantArrayType $leftType,
15431545
return new TypeResult($resultType->toBoolean(), []);
15441546
}
15451547

1546-
private function callOperatorTypeSpecifyingExtensions(Expr\BinaryOp $expr, Type $leftType, Type $rightType): ?Type
1547-
{
1548-
$operatorSigil = $expr->getOperatorSigil();
1549-
$operatorTypeSpecifyingExtensions = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()->getOperatorTypeSpecifyingExtensions($operatorSigil, $leftType, $rightType);
1550-
1551-
/** @var Type[] $extensionTypes */
1552-
$extensionTypes = [];
1553-
1554-
foreach ($operatorTypeSpecifyingExtensions as $extension) {
1555-
$extensionTypes[] = $extension->specifyType($operatorSigil, $leftType, $rightType);
1556-
}
1557-
1558-
if (count($extensionTypes) > 0) {
1559-
return TypeCombinator::union(...$extensionTypes);
1560-
}
1561-
1562-
return null;
1563-
}
1564-
15651548
/**
15661549
* @param BinaryOp\Plus|BinaryOp\Minus|BinaryOp\Mul|BinaryOp\Div|BinaryOp\ShiftLeft|BinaryOp\ShiftRight $expr
15671550
*/
@@ -1606,7 +1589,8 @@ private function resolveCommonMath(Expr\BinaryOp $expr, Type $leftType, Type $ri
16061589
}
16071590
}
16081591

1609-
$specifiedTypes = $this->callOperatorTypeSpecifyingExtensions($expr, $leftType, $rightType);
1592+
$specifiedTypes = $this->operatorTypeSpecifyingExtensionRegistryProvider->getRegistry()
1593+
->callOperatorTypeSpecifyingExtensions($expr, $leftType, $rightType);
16101594
if ($specifiedTypes !== null) {
16111595
return $specifiedTypes;
16121596
}

‎src/Rules/Operators/InvalidComparisonOperationRule.php‎

Lines changed: 68 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
7+
use PHPStan\DependencyInjection\AutowiredParameter;
78
use PHPStan\DependencyInjection\RegisteredRule;
9+
use PHPStan\DependencyInjection\Type\OperatorTypeSpecifyingExtensionRegistryProvider;
10+
use PHPStan\Rules\IdentifierRuleError;
811
use PHPStan\Rules\Rule;
912
use PHPStan\Rules\RuleErrorBuilder;
1013
use PHPStan\Rules\RuleLevelHelper;
@@ -28,7 +31,12 @@
2831
final class InvalidComparisonOperationRule implements Rule
2932
{
3033

31-
public function __construct(private RuleLevelHelper $ruleLevelHelper)
34+
public function __construct(
35+
private RuleLevelHelper $ruleLevelHelper,
36+
private OperatorTypeSpecifyingExtensionRegistryProvider $operatorTypeSpecifyingExtensionRegistryProvider,
37+
#[AutowiredParameter(ref: '%featureToggles.checkExtensionsForComparisonOperators%')]
38+
private bool $checkExtensionsForComparisonOperators,
39+
)
3240
{
3341
}
3442

@@ -55,6 +63,22 @@ public function processNode(Node $node, Scope $scope): array
5563
return [];
5664
}
5765

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

10593
return [];
@@ -166,4 +154,46 @@ private function isPossiblyNullableArrayType(Scope $scope, Node\Expr $expr): boo
166154
return !($type instanceof ErrorType) && $type->isArray()->yes();
167155
}
168156

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

‎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
@@ -4,7 +4,9 @@
44

55
use PHPStan\DependencyInjection\AutowiredService;
66
use PHPStan\Php\PhpVersion;
7+
use PHPStan\Type\BooleanType;
78
use PHPStan\Type\ErrorType;
9+
use PHPStan\Type\IntegerRangeType;
810
use PHPStan\Type\NeverType;
911
use PHPStan\Type\ObjectType;
1012
use PHPStan\Type\OperatorTypeSpecifyingExtension;
@@ -27,7 +29,7 @@ public function isOperatorSupported(string $operatorSigil, Type $leftSide, Type
2729

2830
$bcMathNumberType = new ObjectType('BcMath\Number');
2931

30-
return in_array($operatorSigil, ['-', '+', '*', '/', '**', '%'], true)
32+
return in_array($operatorSigil, ['-', '+', '*', '/', '**', '%', '<', '<=', '>', '>=', '==', '!=', '<=>'], true)
3133
&& (
3234
$bcMathNumberType->isSuperTypeOf($leftSide)->yes()
3335
|| $bcMathNumberType->isSuperTypeOf($rightSide)->yes()
@@ -41,6 +43,18 @@ public function specifyType(string $operatorSigil, Type $leftSide, Type $rightSi
4143
? $rightSide
4244
: $leftSide;
4345

46+
if ($otherSide->isFloat()->yes()) {
47+
return new ErrorType();
48+
}
49+
50+
if (in_array($operatorSigil, ['<', '<=', '>', '>=', '==', '!='], true)) {
51+
return new BooleanType();
52+
}
53+
54+
if ($operatorSigil === '<=>') {
55+
return IntegerRangeType::fromInterval(-1, 1);
56+
}
57+
4458
if (
4559
$otherSide->isInteger()->yes()
4660
|| $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(self::createReflectionProvider(), true, false, true, false, false, false, true),
21+
$this->getContainer()->getByType(OperatorTypeSpecifyingExtensionRegistryProvider::class),
22+
true,
2023
);
2124
}
2225

@@ -170,4 +173,18 @@ public function testBug11119(): void
170173
$this->analyse([__DIR__ . '/data/bug-11119.php'], []);
171174
}
172175

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

0 commit comments

Comments
 (0)