From a44bad9aeab32429cf586bdd3c92647338f9ce88 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 17 Mar 2020 14:06:43 +0100 Subject: [PATCH] Fix property_exists issues --- .../Comparison/ImpossibleCheckTypeHelper.php | 23 +------- .../PropertyExistsTypeSpecifyingExtension.php | 30 ++++++++++ ...mpossibleCheckTypeFunctionCallRuleTest.php | 12 ++++ .../data/check-type-function-call.php | 59 ++++++++++++++++++- 4 files changed, 100 insertions(+), 24 deletions(-) diff --git a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php index 1376769772..e9cfaab680 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeHelper.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeHelper.php @@ -9,7 +9,6 @@ use PHPStan\Analyser\Scope; use PHPStan\Analyser\TypeSpecifier; use PHPStan\Analyser\TypeSpecifierContext; -use PHPStan\Reflection\Php\UniversalObjectCratesClassReflectionExtension; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Constant\ConstantStringType; @@ -129,27 +128,7 @@ public function findSpecifiedType( } } } - } elseif ( - $functionName === 'property_exists' - && count($node->args) >= 2 - ) { - $classNames = TypeUtils::getDirectClassNames( - $scope->getType($node->args[0]->value) - ); - foreach ($classNames as $className) { - if (!$this->reflectionProvider->hasClass($className)) { - continue; - } - - if (UniversalObjectCratesClassReflectionExtension::isUniversalObjectCrate( - $this->reflectionProvider, - $this->universalObjectCratesClasses, - $this->reflectionProvider->getClass($className) - )) { - return null; - } - } - } elseif ($functionName === 'method_exists') { + } elseif ($functionName === 'method_exists' && count($node->args) >= 2) { $objectType = $scope->getType($node->args[0]->value); $methodType = $scope->getType($node->args[1]->value); diff --git a/src/Type/Php/PropertyExistsTypeSpecifyingExtension.php b/src/Type/Php/PropertyExistsTypeSpecifyingExtension.php index 194a376635..376230a03b 100644 --- a/src/Type/Php/PropertyExistsTypeSpecifyingExtension.php +++ b/src/Type/Php/PropertyExistsTypeSpecifyingExtension.php @@ -3,12 +3,15 @@ namespace PHPStan\Type\Php; use PhpParser\Node\Expr\FuncCall; +use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Identifier; use PHPStan\Analyser\Scope; use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\TypeSpecifier; use PHPStan\Analyser\TypeSpecifierAwareExtension; use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\Reflection\FunctionReflection; +use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Type\Accessory\HasPropertyType; use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\FunctionTypeSpecifyingExtension; @@ -18,9 +21,17 @@ class PropertyExistsTypeSpecifyingExtension implements FunctionTypeSpecifyingExtension, TypeSpecifierAwareExtension { + /** @var PropertyReflectionFinder */ + private $propertyReflectionFinder; + /** @var TypeSpecifier */ private $typeSpecifier; + public function __construct(PropertyReflectionFinder $propertyReflectionFinder) + { + $this->propertyReflectionFinder = $propertyReflectionFinder; + } + public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void { $this->typeSpecifier = $typeSpecifier; @@ -49,6 +60,25 @@ public function specifyTypes( return new SpecifiedTypes([], []); } + $objectType = $scope->getType($node->args[0]->value); + if ($objectType instanceof ConstantStringType) { + return new SpecifiedTypes([], []); + } elseif ((new ObjectWithoutClassType())->isSuperTypeOf($objectType)->yes()) { + $propertyNode = new PropertyFetch( + $node->args[0]->value, + new Identifier($propertyNameType->getValue()) + ); + } else { + return new SpecifiedTypes([], []); + } + + $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($propertyNode, $scope); + if ($propertyReflection !== null) { + if (!$propertyReflection->isNative()) { + return new SpecifiedTypes([], []); + } + } + return $this->typeSpecifier->create( $node->args[0]->value, new IntersectionType([ diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 9e9be07432..0a11371b75 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -222,6 +222,18 @@ public function testImpossibleCheckTypeFunctionCall(): void 'Call to function is_numeric() with 123|float will always evaluate to true.', 700, ], + [ + 'Call to function property_exists() with CheckTypeFunctionCall\Bug2221 and \'foo\' will always evaluate to true.', + 782, + ], + [ + 'Call to function assert() with bool will always evaluate to true.', + 786, + ], + [ + 'Call to function property_exists() with CheckTypeFunctionCall\Bug2221 and \'foo\' will always evaluate to true.', + 786, + ], ] ); } diff --git a/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php b/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php index be0f39e2e9..936c86f605 100644 --- a/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php +++ b/tests/PHPStan/Rules/Comparison/data/check-type-function-call.php @@ -472,9 +472,9 @@ public function doFoo( if (property_exists($std, 'foo')) { } - if (property_exists($stdOrSelf, 'foo')) { + /*if (property_exists($stdOrSelf, 'foo')) { // To solve this, we'd need FoundPropertyReflection::isNative() to return TrinaryLogic - } + }*/ if (property_exists($stdOrSelf, 'bar')) { } @@ -756,3 +756,58 @@ function doIpsum(array $data): void } } + +class Bug2221 +{ + + public $foo; + + public function doFoo(): void + { + if (property_exists(Bug2221::class, 'foo')) { + + } + + assert(property_exists(Bug2221::class, 'foo')); + + if (property_exists(Bug2221::class, 'bar')) { + + } + + assert(property_exists(Bug2221::class, 'bar')); + } + + public function doBar(self $self): void + { + if (property_exists($self, 'foo')) { + + } + + assert(property_exists($self, 'foo')); + + if (property_exists($self, 'bar')) { + + } + + assert(property_exists($self, 'bar')); + } + + public function doBaz(\stdClass $std): void + { + if (property_exists($std, 'foo')) { + + } + + assert(property_exists($std, 'foo')); + } + + public function doLorem(\SimpleXMLElement $xml): void + { + if (property_exists($xml, 'foo')) { + + } + + assert(property_exists($xml, 'foo')); + } + +}