Skip to content

Commit

Permalink
Fix Equal handling in TypeSpecifier
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Mar 9, 2022
1 parent 316f269 commit 10b295f
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/Analyser/TypeSpecifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,14 @@ public function specifyTypesInCondition(
return $this->specifyTypesInCondition($scope, new Expr\BinaryOp\Identical($expr->left, $expr->right), $context);
}

$leftExprString = $this->printer->prettyPrintExpr($expr->left);
$rightExprString = $this->printer->prettyPrintExpr($expr->right);
if ($leftExprString === $rightExprString) {
if (!$expr->left instanceof Expr\Variable || !$expr->right instanceof Expr\Variable) {
return new SpecifiedTypes();
}
}

$leftTypes = $this->create($expr->left, $leftType, $context, false, $scope);
$rightTypes = $this->create($expr->right, $rightType, $context, false, $scope);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Comparison;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;

/**
* @extends RuleTestCase<ImpossibleCheckTypeMethodCallRule>
*/
class ImpossibleCheckTypeMethodCallRuleEqualsTest extends RuleTestCase
{

public function getRule(): Rule
{
return new ImpossibleCheckTypeMethodCallRule(
new ImpossibleCheckTypeHelper(
$this->createReflectionProvider(),
$this->getTypeSpecifier(),
[],
true,
),
true,
true,
);
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/impossible-method-call.php'], [
[
'Call to method PHPStan\Tests\AssertionClass::assertString() with string will always evaluate to true.',
14,
],
[
'Call to method PHPStan\Tests\AssertionClass::assertString() with int will always evaluate to false.',
15,
],
[
'Call to method PHPStan\Tests\AssertionClass::assertNotInt() with int will always evaluate to false.',
30,
],
[
'Call to method PHPStan\Tests\AssertionClass::assertNotInt() with string will always evaluate to true.',
36,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with 1 and 1 will always evaluate to true.',
60,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with 1 and 2 will always evaluate to false.',
63,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with 1 and 1 will always evaluate to false.',
66,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with 1 and 2 will always evaluate to true.',
69,
],
[
'Call to method ImpossibleMethodCall\Foo::isSame() with stdClass and stdClass will always evaluate to true.',
78,
],
[
'Call to method ImpossibleMethodCall\Foo::isNotSame() with stdClass and stdClass will always evaluate to false.',
81,
],
]);
}

public static function getAdditionalConfigFiles(): array
{
return [
__DIR__ . '/impossible-check-type-method-call-equals.neon',
];
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,93 @@ public function specifyTypes(
}

}

class FooIsEqual implements MethodTypeSpecifyingExtension, TypeSpecifierAwareExtension
{

/** @var TypeSpecifier */
private $typeSpecifier;

public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void
{
$this->typeSpecifier = $typeSpecifier;
}

public function getClass(): string
{
return \ImpossibleMethodCall\Foo::class;
}

public function isMethodSupported(
MethodReflection $methodReflection,
MethodCall $node,
TypeSpecifierContext $context
): bool
{
return $methodReflection->getName() === 'isSame'
&& count($node->args) >= 2;
}

public function specifyTypes(
MethodReflection $methodReflection,
MethodCall $node,
Scope $scope,
TypeSpecifierContext $context
): SpecifiedTypes
{
return $this->typeSpecifier->specifyTypesInCondition(
$scope,
new \PhpParser\Node\Expr\BinaryOp\Equal(
$node->args[0]->value,
$node->args[1]->value
),
$context
);
}

}

class FooIsNotEqual implements MethodTypeSpecifyingExtension,
TypeSpecifierAwareExtension {

/** @var TypeSpecifier */
private $typeSpecifier;

public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void
{
$this->typeSpecifier = $typeSpecifier;
}

public function getClass(): string
{
return \ImpossibleMethodCall\Foo::class;
}

public function isMethodSupported(
MethodReflection $methodReflection,
MethodCall $node,
TypeSpecifierContext $context
): bool
{
return $methodReflection->getName() === 'isNotSame'
&& count($node->args) >= 2;
}

public function specifyTypes(
MethodReflection $methodReflection,
MethodCall $node,
Scope $scope,
TypeSpecifierContext $context
): SpecifiedTypes
{
return $this->typeSpecifier->specifyTypesInCondition(
$scope,
new \PhpParser\Node\Expr\BinaryOp\NotEqual(
$node->args[0]->value,
$node->args[1]->value
),
$context
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
services:
-
class: PHPStan\Tests\AssertionClassMethodTypeSpecifyingExtension
arguments:
nullContext: null
tags:
- phpstan.typeSpecifier.methodTypeSpecifyingExtension
-
class: PHPStan\Rules\Comparison\AssertNotInt
tags:
- phpstan.typeSpecifier.methodTypeSpecifyingExtension
-
class: PHPStan\Rules\Comparison\FooIsEqual
tags:
- phpstan.typeSpecifier.methodTypeSpecifyingExtension
-
class: PHPStan\Rules\Comparison\FooIsNotEqual
tags:
- phpstan.typeSpecifier.methodTypeSpecifyingExtension

0 comments on commit 10b295f

Please sign in to comment.