diff --git a/src/Analyser/ExprHandler/PropertyFetchHandler.php b/src/Analyser/ExprHandler/PropertyFetchHandler.php index 2095791b11c..b2f80325811 100644 --- a/src/Analyser/ExprHandler/PropertyFetchHandler.php +++ b/src/Analyser/ExprHandler/PropertyFetchHandler.php @@ -150,9 +150,16 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, PropertyFetc // comes from beforeScope; the scope-dependent receiver type is read // from the operand result above. $reflectionScope = $nativeTypesPromoted ? $beforeScope->doNotTreatPhpDocTypesAsCertain() : $beforeScope; - $resolveProperty = function (string $propertyName) use ($nativeTypesPromoted, $reflectionScope, $receiverType, $expr): Type { + $resolveProperty = function (string $propertyName) use ($nativeTypesPromoted, $reflectionScope, $receiverType, $varResult, $expr): Type { if ($nativeTypesPromoted) { - $propertyReflection = $reflectionScope->getInstancePropertyReflection($receiverType, $propertyName); + // unlike getInstancePropertyReflection(), do not pick only the union + // members that have the property - a fetch on e.g. a null member + // does not produce the property's native type + $nativeReceiverType = $varResult->containsNullsafe() ? TypeCombinator::removeNull($receiverType) : $receiverType; + if (!$nativeReceiverType->hasInstanceProperty($propertyName)->yes()) { + return new ErrorType(); + } + $propertyReflection = $reflectionScope->getInstancePropertyReflection($nativeReceiverType, $propertyName); if ($propertyReflection === null) { return new ErrorType(); } diff --git a/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php b/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php index c3742ccd17d..b1cde0a64e5 100644 --- a/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php +++ b/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php @@ -163,8 +163,17 @@ public function composeResult(StaticPropertyFetch $expr, ?ExpressionResult $clas $staticPropertyFetchedOnType = TypeCombinator::removeNull($classType)->getObjectTypeOrClassStringObjectType(); } - $resolveProperty = function (string $propertyName) use ($nativeTypesPromoted, $reflectionScope, $staticPropertyFetchedOnType, $expr): Type { + $resolveProperty = function (string $propertyName) use ($nativeTypesPromoted, $reflectionScope, $staticPropertyFetchedOnType, $classType, $classResult, $expr): Type { if ($nativeTypesPromoted) { + // unlike getStaticPropertyReflection(), do not pick only the union + // members that have the property - a fetch on e.g. a null member + // does not produce the property's native type + if ($classType !== null && !$classResult->containsNullsafe() && TypeCombinator::containsNull($classType)) { + return new ErrorType(); + } + if (!$staticPropertyFetchedOnType->hasStaticProperty($propertyName)->yes()) { + return new ErrorType(); + } $propertyReflection = $reflectionScope->getStaticPropertyReflection($staticPropertyFetchedOnType, $propertyName); if ($propertyReflection === null) { return new ErrorType(); diff --git a/tests/PHPStan/Analyser/nsrt/bug-15424.php b/tests/PHPStan/Analyser/nsrt/bug-15424.php new file mode 100644 index 00000000000..ea0175577fb --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15424.php @@ -0,0 +1,65 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug15424Types; + +use function PHPStan\Testing\assertNativeType; +use function PHPStan\Testing\assertType; + +final class Dekking +{ + public static float $staticAmount = 1.0; + + public function __construct(public float $amount) + { + } +} + +final class Other +{ + public float $amount = 1.0; +} + +final class WithoutAmount +{ +} + +final class Holder +{ + public Dekking $dekking; + + public ?Dekking $nullableDekking = null; +} + +function nullableReceiver(?Dekking $data, ?Holder $holder): void +{ + assertType('float', $data->amount); + assertNativeType('*ERROR*', $data->amount); + assertType('float', $data::$staticAmount); + assertNativeType('*ERROR*', $data::$staticAmount); + + assertType('float|null', $holder?->dekking->amount); + assertNativeType('float|null', $holder?->dekking->amount); + assertType('float|null', $holder?->dekking::$staticAmount); + assertNativeType('float|null', $holder?->dekking::$staticAmount); +} + +function unionReceiver(Dekking|WithoutAmount $data, Dekking|Other $both): void +{ + assertType('float', $data->amount); + assertNativeType('*ERROR*', $data->amount); + assertType('float', $data::$staticAmount); + assertNativeType('*ERROR*', $data::$staticAmount); + + assertType('float', $both->amount); + assertNativeType('float', $both->amount); +} + +function nonNullableReceiver(Dekking $data): void +{ + assertType('float', $data->amount); + assertNativeType('float', $data->amount); + assertType('float', $data::$staticAmount); + assertNativeType('float', $data::$staticAmount); +} diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index 3875567895f..dae69da0b83 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php @@ -1287,4 +1287,16 @@ public function testBug15432(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testBug15424(): void + { + $this->treatPhpDocTypesAsCertain = false; + $this->analyse([__DIR__ . '/data/bug-15424.php'], [ + [ + 'Strict comparison using === between float and null will always evaluate to false.', + 25, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-15424.php b/tests/PHPStan/Rules/Comparison/data/bug-15424.php new file mode 100644 index 00000000000..c4de1174c77 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-15424.php @@ -0,0 +1,30 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug15424; + +final class Dekking +{ + public function __construct(public float $amount) + { + } +} + +function nullableReceiver(?Dekking $data = null): ?string +{ + if ($data->amount === null) { + return null; + } + + return 'x'; +} + +function nonNullableReceiver(Dekking $data): ?string +{ + if ($data->amount === null) { + return null; + } + + return 'x'; +} diff --git a/turbo-ext/src/PropertyFetchHandler.cpp b/turbo-ext/src/PropertyFetchHandler.cpp index ea2ef96f8b2..e0596af01dd 100644 --- a/turbo-ext/src/PropertyFetchHandler.cpp +++ b/turbo-ext/src/PropertyFetchHandler.cpp @@ -337,8 +337,26 @@ class PropertyFetchHandler } /* $resolveProperty($propertyName) of the typeCallback */ - zv::Val resolveProperty(bool nativeTypesPromoted, zval *reflectionScope, zval *receiverType, zval *expr, zend_string *propertyName) const + zv::Val resolveProperty(bool nativeTypesPromoted, zval *reflectionScope, zval *receiverType, zval *varResult, zval *expr, zend_string *propertyName) const { + zv::Val nativeReceiverType; + if (nativeTypesPromoted) { + // unlike getInstancePropertyReflection(), do not pick only the union + // members that have the property - a fetch on e.g. a null member + // does not produce the property's native type + bool containsNullsafe; + if (UNEXPECTED(!pt_expression_result_contains_nullsafe(varResult, containsNullsafe))) return zv::Val(); + if (containsNullsafe) { + nativeReceiverType = pt_type_combinator_remove_null(receiverType); + if (UNEXPECTED(nativeReceiverType.isUndef())) return zv::Val(); + receiverType = nativeReceiverType.raw(); + } + zval propertyNameZv; + ZVAL_STR(&propertyNameZv, propertyName); + zend_long hasInstanceProperty = pt_type_call_trinary(Z_OBJ_P(receiverType), PT_LC("hasinstanceproperty"), 1, &propertyNameZv); + if (UNEXPECTED(hasInstanceProperty < 0)) return zv::Val(); + if (hasInstanceProperty != PT_TRI_YES) return pt_type_new_error_type(); + } zv::Val propertyReflection = pt_mutating_scope_get_instance_property_reflection(Z_OBJ_P(reflectionScope), receiverType, propertyName); if (UNEXPECTED(propertyReflection.isUndef())) return zv::Val(); if (propertyReflection.isNull()) return pt_type_new_error_type(); @@ -383,7 +401,7 @@ class PropertyFetchHandler zend_type_error("PhpParser\\Node\\Identifier::toString(): Return value must be of type string, %s returned", zend_zval_value_name(propertyName)); return zv::Val(); } - zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, receiverType.raw(), expr, Z_STR_P(propertyName)); + zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, receiverType.raw(), varResult, expr, Z_STR_P(propertyName)); if (UNEXPECTED(type.isUndef())) return zv::Val(); // $shortCircuit($type) bool containsNullsafe; @@ -438,7 +456,7 @@ class PropertyFetchHandler zend_type_error("%s(): Argument #1 ($propertyName) must be of type string, %s given", pt_pfh_closure_name, zend_zval_value_name(value.raw())); return zv::Val(); } - zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, receiverType.raw(), expr, Z_STR_P(value.raw())); + zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, receiverType.raw(), varResult, expr, Z_STR_P(value.raw())); if (UNEXPECTED(type.isUndef())) return zv::Val(); types.push(std::move(type)); } diff --git a/turbo-ext/src/StaticPropertyFetchHandler.cpp b/turbo-ext/src/StaticPropertyFetchHandler.cpp index 09a05dba11a..c1fa3531e78 100644 --- a/turbo-ext/src/StaticPropertyFetchHandler.cpp +++ b/turbo-ext/src/StaticPropertyFetchHandler.cpp @@ -342,8 +342,27 @@ class StaticPropertyFetchHandler } /* $resolveProperty($propertyName) of the typeCallback */ - static zv::Val resolveProperty(bool nativeTypesPromoted, zval *reflectionScope, zval *fetchedOnType, zval *expr, zend_string *propertyName) + static zv::Val resolveProperty(bool nativeTypesPromoted, zval *reflectionScope, zval *fetchedOnType, zval *classType, zval *classResult, zval *expr, zend_string *propertyName) { + if (nativeTypesPromoted) { + // unlike getStaticPropertyReflection(), do not pick only the union + // members that have the property - a fetch on e.g. a null member + // does not produce the property's native type + if (classType != NULL && Z_TYPE_P(classResult) != IS_NULL) { + bool containsNullsafe; + if (UNEXPECTED(!pt_expression_result_contains_nullsafe(classResult, containsNullsafe))) return zv::Val(); + if (!containsNullsafe) { + bool containsNull; + if (UNEXPECTED(!pt_type_combinator_contains_null(classType, containsNull))) return zv::Val(); + if (containsNull) return pt_type_new_error_type(); + } + } + zval propertyNameZv; + ZVAL_STR(&propertyNameZv, propertyName); + zend_long hasStaticProperty = pt_type_call_trinary(Z_OBJ_P(fetchedOnType), PT_LC("hasstaticproperty"), 1, &propertyNameZv); + if (UNEXPECTED(hasStaticProperty < 0)) return zv::Val(); + if (hasStaticProperty != PT_TRI_YES) return pt_type_new_error_type(); + } zv::Val propertyReflection = pt_mutating_scope_get_static_property_reflection(Z_OBJ_P(reflectionScope), fetchedOnType, propertyName); if (UNEXPECTED(propertyReflection.isUndef())) return zv::Val(); if (propertyReflection.isNull()) return pt_type_new_error_type(); @@ -406,7 +425,7 @@ class StaticPropertyFetchHandler zend_type_error("PhpParser\\Node\\Identifier::toString(): Return value must be of type string, %s returned", zend_zval_value_name(propertyName)); return zv::Val(); } - zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, staticPropertyFetchedOnType.raw(), expr, Z_STR_P(propertyName)); + zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, staticPropertyFetchedOnType.raw(), classType.isUndef() ? NULL : classType.raw(), classResult, expr, Z_STR_P(propertyName)); if (UNEXPECTED(type.isUndef())) return zv::Val(); // $shortCircuit($type) if (Z_TYPE_P(classResult) != IS_NULL) { @@ -463,7 +482,7 @@ class StaticPropertyFetchHandler zend_type_error("%s(): Argument #1 ($propertyName) must be of type string, %s given", pt_spfh_closure_name, zend_zval_value_name(value.raw())); return zv::Val(); } - zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, staticPropertyFetchedOnType.raw(), expr, Z_STR_P(value.raw())); + zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, staticPropertyFetchedOnType.raw(), classType.isUndef() ? NULL : classType.raw(), classResult, expr, Z_STR_P(value.raw())); if (UNEXPECTED(type.isUndef())) return zv::Val(); types.push(std::move(type)); }