diff --git a/conf/config.level4.neon b/conf/config.level4.neon index 093d95a686..099db6da81 100644 --- a/conf/config.level4.neon +++ b/conf/config.level4.neon @@ -9,6 +9,8 @@ services: - class: PHPStan\Rules\Classes\ImpossibleInstanceOfRule + arguments: + checkAlwaysTrueInstanceof: %checkAlwaysTrueInstanceof% tags: - phpstan.rules.rule diff --git a/conf/config.neon b/conf/config.neon index 613eceea09..a368c57884 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -5,6 +5,7 @@ parameters: autoload_files: [] fileExtensions: - php + checkAlwaysTrueInstanceof: false checkFunctionArgumentTypes: false checkArgumentsPassedByReference: false checkMaybeUndefinedVariables: false diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index e455315ebe..7e68693c20 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -145,6 +145,7 @@ public function processNodes( Scope $closureBindScope = null ) { + /** @var \PhpParser\Node|string $node */ foreach ($nodes as $i => $node) { if (!($node instanceof \PhpParser\Node)) { continue; @@ -189,8 +190,7 @@ public function processNodes( } elseif ($node instanceof Node\Stmt\Declare_) { foreach ($node->declares as $declare) { if ( - $declare instanceof Node\Stmt\DeclareDeclare - && $declare->key === 'strict_types' + $declare->key === 'strict_types' && $declare->value instanceof Node\Scalar\LNumber && $declare->value->value === 1 ) { diff --git a/src/Analyser/Scope.php b/src/Analyser/Scope.php index 9efd4c1d9c..4f5d470e7f 100644 --- a/src/Analyser/Scope.php +++ b/src/Analyser/Scope.php @@ -548,7 +548,7 @@ function (Expr\ArrayItem $item): Type { if ($constantClass === 'self') { $constantClass = $this->getClassReflection()->getName(); } - } elseif ($node->class instanceof Expr) { + } else { $constantClassType = $this->getType($node->class); if ($constantClassType->getClass() !== null) { $constantClass = $constantClassType->getClass(); @@ -1090,7 +1090,7 @@ public function enterAnonymousFunction( private function isParameterValueNullable(Node\Param $parameter): bool { - if ($parameter->default instanceof ConstFetch && $parameter->default->name instanceof Name) { + if ($parameter->default instanceof ConstFetch) { return strtolower((string) $parameter->default->name) === 'null'; } diff --git a/src/Reflection/Php/PhpParameterFromParserNodeReflection.php b/src/Reflection/Php/PhpParameterFromParserNodeReflection.php index 00f1b2342d..5f77ea13e7 100644 --- a/src/Reflection/Php/PhpParameterFromParserNodeReflection.php +++ b/src/Reflection/Php/PhpParameterFromParserNodeReflection.php @@ -4,7 +4,6 @@ use PhpParser\Node\Expr; use PhpParser\Node\Expr\ConstFetch; -use PhpParser\Node\Name; use PHPStan\Type\Type; use PHPStan\Type\TypehintHelper; @@ -71,7 +70,6 @@ public function getType(): Type if ($phpDocType !== null && $this->defaultValue !== null) { if ( $this->defaultValue instanceof ConstFetch - && $this->defaultValue->name instanceof Name && strtolower((string) $this->defaultValue->name) === 'null' ) { $phpDocType = \PHPStan\Type\TypeCombinator::addNull($phpDocType); diff --git a/src/Rules/Classes/ImpossibleInstanceOfRule.php b/src/Rules/Classes/ImpossibleInstanceOfRule.php index befe29f669..8f4add6dd4 100644 --- a/src/Rules/Classes/ImpossibleInstanceOfRule.php +++ b/src/Rules/Classes/ImpossibleInstanceOfRule.php @@ -4,20 +4,17 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; -use PHPStan\Broker\Broker; use PHPStan\Type\ObjectType; class ImpossibleInstanceOfRule implements \PHPStan\Rules\Rule { - /** - * @var \PHPStan\Broker\Broker - */ - private $broker; + /** @var bool */ + private $checkAlwaysTrueInstanceof; - public function __construct(Broker $broker) + public function __construct(bool $checkAlwaysTrueInstanceof) { - $this->broker = $broker; + $this->checkAlwaysTrueInstanceof = $checkAlwaysTrueInstanceof; } public function getNodeType(): string @@ -39,43 +36,21 @@ public function processNode(Node $node, Scope $scope): array $type = $scope->getType($node->class); } - if ($type->getClass() === null) { - return []; - } - $expressionType = $scope->getType($node->expr); - if ($expressionType->getClass() === null) { - return []; - } - - if (!$this->broker->hasClass($expressionType->getClass())) { - return []; - } - - $expressionClassReflection = $this->broker->getClass($expressionType->getClass()); - if (!$this->broker->hasClass($type->getClass())) { - return []; - } + $isSuperset = $type->isSupersetOf($expressionType); - if ($expressionClassReflection->isSubclassOf($type->getClass())) { + if ($isSuperset->no()) { return [ sprintf( - 'Instanceof between %s and %s will always evaluate to true.', + 'Instanceof between %s and %s will always evaluate to false.', $expressionType->describe(), $type->describe() ), ]; - } - - $classReflection = $this->broker->getClass($type->getClass()); - if ($classReflection->isInterface() || $expressionClassReflection->isInterface()) { - return []; - } - - if (!$expressionType->accepts($type)) { + } elseif ($isSuperset->yes() && $this->checkAlwaysTrueInstanceof) { return [ sprintf( - 'Instanceof between %s and %s will always evaluate to false.', + 'Instanceof between %s and %s will always evaluate to true.', $expressionType->describe(), $type->describe() ), diff --git a/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php b/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php index 3f02d72e26..3dfceb4a81 100644 --- a/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php @@ -5,20 +5,32 @@ class ImpossibleInstanceOfRuleTest extends \PHPStan\Rules\AbstractRuleTest { + /** @var bool */ + private $checkAlwaysTrueInstanceOf; + protected function getRule(): \PHPStan\Rules\Rule { - return new ImpossibleInstanceOfRule($this->createBroker()); + return new ImpossibleInstanceOfRule($this->checkAlwaysTrueInstanceOf); } - public function testInstantiation() + public function testInstanceof() { + $this->checkAlwaysTrueInstanceOf = true; $this->analyse( [__DIR__ . '/data/impossible-instanceof.php'], [ + [ + 'Instanceof between ImpossibleInstanceOf\Lorem and ImpossibleInstanceOf\Lorem will always evaluate to true.', + 59, + ], [ 'Instanceof between ImpossibleInstanceOf\Ipsum and ImpossibleInstanceOf\Lorem will always evaluate to true.', 65, ], + [ + 'Instanceof between ImpossibleInstanceOf\Ipsum and ImpossibleInstanceOf\Ipsum will always evaluate to true.', + 68, + ], [ 'Instanceof between ImpossibleInstanceOf\Dolor and ImpossibleInstanceOf\Lorem will always evaluate to false.', 71, @@ -35,4 +47,18 @@ public function testInstantiation() ); } + public function testInstanceofWithoutAlwaysTrue() + { + $this->checkAlwaysTrueInstanceOf = false; + $this->analyse( + [__DIR__ . '/data/impossible-instanceof.php'], + [ + [ + 'Instanceof between ImpossibleInstanceOf\Dolor and ImpossibleInstanceOf\Lorem will always evaluate to false.', + 71, + ], + ] + ); + } + }