Skip to content

Commit da45cc1

Browse files
phpstan-botondrejmirtes
phpstan-bot
authored and
ondrejmirtes
committed
Require every receiver union member to have the property when resolving native type of PropertyFetch and StaticPropertyFetch
- PropertyFetchHandler: the native-type path used MutatingScope::getInstancePropertyReflection(), which keeps only the union members that have the property, so `?Foo` / `Foo|Bar` receivers got the property's native type. Now it returns ErrorType unless the whole receiver type (with null removed only on a nullsafe chain) has the property, as 2.2 did via PropertyReflectionFinder. - StaticPropertyFetchHandler: same for `$expr::$prop` - a nullable class expression outside a nullsafe chain, or a union with members lacking the static property, gives ErrorType. - Ported both changes to the turbo-ext C++ mirrors (PropertyFetchHandler.cpp, StaticPropertyFetchHandler.cpp); analysis output is identical with the extension on and off. - Probed method calls, static method calls, class constants, array dim fetch, nullsafe and dynamic-name property fetches against 2.2.17: they already match, so no changes there.
1 parent 4ef9ab7 commit da45cc1

7 files changed

Lines changed: 169 additions & 9 deletions

File tree

‎src/Analyser/ExprHandler/PropertyFetchHandler.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,16 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, PropertyFetc
150150
// comes from beforeScope; the scope-dependent receiver type is read
151151
// from the operand result above.
152152
$reflectionScope = $nativeTypesPromoted ? $beforeScope->doNotTreatPhpDocTypesAsCertain() : $beforeScope;
153-
$resolveProperty = function (string $propertyName) use ($nativeTypesPromoted, $reflectionScope, $receiverType, $expr): Type {
153+
$resolveProperty = function (string $propertyName) use ($nativeTypesPromoted, $reflectionScope, $receiverType, $varResult, $expr): Type {
154154
if ($nativeTypesPromoted) {
155-
$propertyReflection = $reflectionScope->getInstancePropertyReflection($receiverType, $propertyName);
155+
// unlike getInstancePropertyReflection(), do not pick only the union
156+
// members that have the property - a fetch on e.g. a null member
157+
// does not produce the property's native type
158+
$nativeReceiverType = $varResult->containsNullsafe() ? TypeCombinator::removeNull($receiverType) : $receiverType;
159+
if (!$nativeReceiverType->hasInstanceProperty($propertyName)->yes()) {
160+
return new ErrorType();
161+
}
162+
$propertyReflection = $reflectionScope->getInstancePropertyReflection($nativeReceiverType, $propertyName);
156163
if ($propertyReflection === null) {
157164
return new ErrorType();
158165
}

‎src/Analyser/ExprHandler/StaticPropertyFetchHandler.php‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,17 @@ public function composeResult(StaticPropertyFetch $expr, ?ExpressionResult $clas
163163
$staticPropertyFetchedOnType = TypeCombinator::removeNull($classType)->getObjectTypeOrClassStringObjectType();
164164
}
165165

166-
$resolveProperty = function (string $propertyName) use ($nativeTypesPromoted, $reflectionScope, $staticPropertyFetchedOnType, $expr): Type {
166+
$resolveProperty = function (string $propertyName) use ($nativeTypesPromoted, $reflectionScope, $staticPropertyFetchedOnType, $classType, $classResult, $expr): Type {
167167
if ($nativeTypesPromoted) {
168+
// unlike getStaticPropertyReflection(), do not pick only the union
169+
// members that have the property - a fetch on e.g. a null member
170+
// does not produce the property's native type
171+
if ($classType !== null && !$classResult->containsNullsafe() && TypeCombinator::containsNull($classType)) {
172+
return new ErrorType();
173+
}
174+
if (!$staticPropertyFetchedOnType->hasStaticProperty($propertyName)->yes()) {
175+
return new ErrorType();
176+
}
168177
$propertyReflection = $reflectionScope->getStaticPropertyReflection($staticPropertyFetchedOnType, $propertyName);
169178
if ($propertyReflection === null) {
170179
return new ErrorType();

‎tests/PHPStan/Analyser/nsrt/bug-15424.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug15424Types;
6+
7+
use function PHPStan\Testing\assertNativeType;
8+
use function PHPStan\Testing\assertType;
9+
10+
final class Dekking
11+
{
12+
public static float $staticAmount = 1.0;
13+
14+
public function __construct(public float $amount)
15+
{
16+
}
17+
}
18+
19+
final class Other
20+
{
21+
public float $amount = 1.0;
22+
}
23+
24+
final class WithoutAmount
25+
{
26+
}
27+
28+
final class Holder
29+
{
30+
public Dekking $dekking;
31+
32+
public ?Dekking $nullableDekking = null;
33+
}
34+
35+
function nullableReceiver(?Dekking $data, ?Holder $holder): void
36+
{
37+
assertType('float', $data->amount);
38+
assertNativeType('*ERROR*', $data->amount);
39+
assertType('float', $data::$staticAmount);
40+
assertNativeType('*ERROR*', $data::$staticAmount);
41+
42+
assertType('float|null', $holder?->dekking->amount);
43+
assertNativeType('float|null', $holder?->dekking->amount);
44+
assertType('float|null', $holder?->dekking::$staticAmount);
45+
assertNativeType('float|null', $holder?->dekking::$staticAmount);
46+
}
47+
48+
function unionReceiver(Dekking|WithoutAmount $data, Dekking|Other $both): void
49+
{
50+
assertType('float', $data->amount);
51+
assertNativeType('*ERROR*', $data->amount);
52+
assertType('float', $data::$staticAmount);
53+
assertNativeType('*ERROR*', $data::$staticAmount);
54+
55+
assertType('float', $both->amount);
56+
assertNativeType('float', $both->amount);
57+
}
58+
59+
function nonNullableReceiver(Dekking $data): void
60+
{
61+
assertType('float', $data->amount);
62+
assertNativeType('float', $data->amount);
63+
assertType('float', $data::$staticAmount);
64+
assertNativeType('float', $data::$staticAmount);
65+
}

‎tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,4 +1287,16 @@ public function testBug15432(): void
12871287
]);
12881288
}
12891289

1290+
#[RequiresPhp('>= 8.0.0')]
1291+
public function testBug15424(): void
1292+
{
1293+
$this->treatPhpDocTypesAsCertain = false;
1294+
$this->analyse([__DIR__ . '/data/bug-15424.php'], [
1295+
[
1296+
'Strict comparison using === between float and null will always evaluate to false.',
1297+
25,
1298+
],
1299+
]);
1300+
}
1301+
12901302
}

‎tests/PHPStan/Rules/Comparison/data/bug-15424.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug15424;
6+
7+
final class Dekking
8+
{
9+
public function __construct(public float $amount)
10+
{
11+
}
12+
}
13+
14+
function nullableReceiver(?Dekking $data = null): ?string
15+
{
16+
if ($data->amount === null) {
17+
return null;
18+
}
19+
20+
return 'x';
21+
}
22+
23+
function nonNullableReceiver(Dekking $data): ?string
24+
{
25+
if ($data->amount === null) {
26+
return null;
27+
}
28+
29+
return 'x';
30+
}

‎turbo-ext/src/PropertyFetchHandler.cpp‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -337,8 +337,26 @@ class PropertyFetchHandler
337337
}
338338

339339
/* $resolveProperty($propertyName) of the typeCallback */
340-
zv::Val resolveProperty(bool nativeTypesPromoted, zval *reflectionScope, zval *receiverType, zval *expr, zend_string *propertyName) const
340+
zv::Val resolveProperty(bool nativeTypesPromoted, zval *reflectionScope, zval *receiverType, zval *varResult, zval *expr, zend_string *propertyName) const
341341
{
342+
zv::Val nativeReceiverType;
343+
if (nativeTypesPromoted) {
344+
// unlike getInstancePropertyReflection(), do not pick only the union
345+
// members that have the property - a fetch on e.g. a null member
346+
// does not produce the property's native type
347+
bool containsNullsafe;
348+
if (UNEXPECTED(!pt_expression_result_contains_nullsafe(varResult, containsNullsafe))) return zv::Val();
349+
if (containsNullsafe) {
350+
nativeReceiverType = pt_type_combinator_remove_null(receiverType);
351+
if (UNEXPECTED(nativeReceiverType.isUndef())) return zv::Val();
352+
receiverType = nativeReceiverType.raw();
353+
}
354+
zval propertyNameZv;
355+
ZVAL_STR(&propertyNameZv, propertyName);
356+
zend_long hasInstanceProperty = pt_type_call_trinary(Z_OBJ_P(receiverType), PT_LC("hasinstanceproperty"), 1, &propertyNameZv);
357+
if (UNEXPECTED(hasInstanceProperty < 0)) return zv::Val();
358+
if (hasInstanceProperty != PT_TRI_YES) return pt_type_new_error_type();
359+
}
342360
zv::Val propertyReflection = pt_mutating_scope_get_instance_property_reflection(Z_OBJ_P(reflectionScope), receiverType, propertyName);
343361
if (UNEXPECTED(propertyReflection.isUndef())) return zv::Val();
344362
if (propertyReflection.isNull()) return pt_type_new_error_type();
@@ -383,7 +401,7 @@ class PropertyFetchHandler
383401
zend_type_error("PhpParser\\Node\\Identifier::toString(): Return value must be of type string, %s returned", zend_zval_value_name(propertyName));
384402
return zv::Val();
385403
}
386-
zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, receiverType.raw(), expr, Z_STR_P(propertyName));
404+
zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, receiverType.raw(), varResult, expr, Z_STR_P(propertyName));
387405
if (UNEXPECTED(type.isUndef())) return zv::Val();
388406
// $shortCircuit($type)
389407
bool containsNullsafe;
@@ -438,7 +456,7 @@ class PropertyFetchHandler
438456
zend_type_error("%s(): Argument #1 ($propertyName) must be of type string, %s given", pt_pfh_closure_name, zend_zval_value_name(value.raw()));
439457
return zv::Val();
440458
}
441-
zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, receiverType.raw(), expr, Z_STR_P(value.raw()));
459+
zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, receiverType.raw(), varResult, expr, Z_STR_P(value.raw()));
442460
if (UNEXPECTED(type.isUndef())) return zv::Val();
443461
types.push(std::move(type));
444462
}

‎turbo-ext/src/StaticPropertyFetchHandler.cpp‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,27 @@ class StaticPropertyFetchHandler
342342
}
343343

344344
/* $resolveProperty($propertyName) of the typeCallback */
345-
static zv::Val resolveProperty(bool nativeTypesPromoted, zval *reflectionScope, zval *fetchedOnType, zval *expr, zend_string *propertyName)
345+
static zv::Val resolveProperty(bool nativeTypesPromoted, zval *reflectionScope, zval *fetchedOnType, zval *classType, zval *classResult, zval *expr, zend_string *propertyName)
346346
{
347+
if (nativeTypesPromoted) {
348+
// unlike getStaticPropertyReflection(), do not pick only the union
349+
// members that have the property - a fetch on e.g. a null member
350+
// does not produce the property's native type
351+
if (classType != NULL && Z_TYPE_P(classResult) != IS_NULL) {
352+
bool containsNullsafe;
353+
if (UNEXPECTED(!pt_expression_result_contains_nullsafe(classResult, containsNullsafe))) return zv::Val();
354+
if (!containsNullsafe) {
355+
bool containsNull;
356+
if (UNEXPECTED(!pt_type_combinator_contains_null(classType, containsNull))) return zv::Val();
357+
if (containsNull) return pt_type_new_error_type();
358+
}
359+
}
360+
zval propertyNameZv;
361+
ZVAL_STR(&propertyNameZv, propertyName);
362+
zend_long hasStaticProperty = pt_type_call_trinary(Z_OBJ_P(fetchedOnType), PT_LC("hasstaticproperty"), 1, &propertyNameZv);
363+
if (UNEXPECTED(hasStaticProperty < 0)) return zv::Val();
364+
if (hasStaticProperty != PT_TRI_YES) return pt_type_new_error_type();
365+
}
347366
zv::Val propertyReflection = pt_mutating_scope_get_static_property_reflection(Z_OBJ_P(reflectionScope), fetchedOnType, propertyName);
348367
if (UNEXPECTED(propertyReflection.isUndef())) return zv::Val();
349368
if (propertyReflection.isNull()) return pt_type_new_error_type();
@@ -406,7 +425,7 @@ class StaticPropertyFetchHandler
406425
zend_type_error("PhpParser\\Node\\Identifier::toString(): Return value must be of type string, %s returned", zend_zval_value_name(propertyName));
407426
return zv::Val();
408427
}
409-
zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, staticPropertyFetchedOnType.raw(), expr, Z_STR_P(propertyName));
428+
zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, staticPropertyFetchedOnType.raw(), classType.isUndef() ? NULL : classType.raw(), classResult, expr, Z_STR_P(propertyName));
410429
if (UNEXPECTED(type.isUndef())) return zv::Val();
411430
// $shortCircuit($type)
412431
if (Z_TYPE_P(classResult) != IS_NULL) {
@@ -463,7 +482,7 @@ class StaticPropertyFetchHandler
463482
zend_type_error("%s(): Argument #1 ($propertyName) must be of type string, %s given", pt_spfh_closure_name, zend_zval_value_name(value.raw()));
464483
return zv::Val();
465484
}
466-
zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, staticPropertyFetchedOnType.raw(), expr, Z_STR_P(value.raw()));
485+
zv::Val type = resolveProperty(nativeTypesPromoted, reflectionScope, staticPropertyFetchedOnType.raw(), classType.isUndef() ? NULL : classType.raw(), classResult, expr, Z_STR_P(value.raw()));
467486
if (UNEXPECTED(type.isUndef())) return zv::Val();
468487
types.push(std::move(type));
469488
}

0 commit comments

Comments
 (0)