Skip to content

Commit 9bfe49c

Browse files
phpstan-botondrejmirtes
phpstan-bot
authored and
ondrejmirtes
committed
Consult ExpressionTypeResolverExtensions in MutatingScope::resolveScopeStateType()
- The state-type read (getStateType()) used by the non-nullability ensure of `?->`, `??`, isset()/empty() and by narrowing skipped expression type resolver extensions, unlike resolveType(). A receiver typed only by an extension (e.g. a universal object crate property) was read as mixed, so `$crate->holder->day` was never ensured non-null and the plain twin of `$crate->holder->day?->toDateString()` was checked against `Day|null`. - Non-variable expressions now ask the extensions first (on the native- promoted scope for the native flavour), mirroring resolveType(). - Ported the same change to the turbo-ext mirror (MutatingScope.cpp). - The same root cause also broke, and this fixes: chained `?->` over an extension-typed receiver, `?->` property fetch on an offset of it, `$crate->holder->day->date ?? null` (was mixed), and `!== null` narrowing of `$crate->holder->day` (was mixed~null).
1 parent 4ef9ab7 commit 9bfe49c

8 files changed

Lines changed: 241 additions & 1 deletion

‎src/Analyser/MutatingScope.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3423,6 +3423,18 @@ private function getScopeStateNativeType(Expr $expr): Type
34233423
*/
34243424
private function resolveScopeStateType(Expr $expr, bool $native): Type
34253425
{
3426+
if (!$expr instanceof Variable) {
3427+
// mirror resolveType(): an extension's answer wins over both the
3428+
// tracked holder and the reflection-derived read below
3429+
$askScope = $native ? $this->doNotTreatPhpDocTypesAsCertain() : $this;
3430+
foreach ($this->services->expressionTypeResolverExtensions->getAll() as $extension) {
3431+
$type = $extension->getType($expr, $askScope);
3432+
if ($type !== null) {
3433+
return $type;
3434+
}
3435+
}
3436+
}
3437+
34263438
if (!$expr instanceof Variable && $this->hasExpressionType($expr)->yes()) {
34273439
// mirror resolveType()'s tracked-holder lookup without pricing the
34283440
// node - the tracked type IS scope state (the extension hook is

‎tests/PHPStan/Analyser/ExpressionTypeResolverExtensionTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ class ExpressionTypeResolverExtensionTest extends TypeInferenceTestCase
1111
public static function dataFileAsserts(): iterable
1212
{
1313
yield from self::gatherAssertTypes(__DIR__ . '/data/expression-type-resolver-extension.php');
14+
yield from self::gatherAssertTypes(__DIR__ . '/data/bug-15421.php');
1415
}
1516

1617
/**

‎tests/PHPStan/Analyser/data/Bug15421ExpressionTypeResolverExtension.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
namespace ExpressionTypeResolverExtension;
4+
5+
use PhpParser\Node\Expr;
6+
use PhpParser\Node\Expr\PropertyFetch;
7+
use PhpParser\Node\Identifier;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Type\ExpressionTypeResolverExtension;
10+
use PHPStan\Type\ObjectType;
11+
use PHPStan\Type\Type;
12+
13+
class Bug15421ExpressionTypeResolverExtension implements ExpressionTypeResolverExtension
14+
{
15+
16+
public function getType(Expr $expr, Scope $scope): ?Type
17+
{
18+
if ($expr instanceof PropertyFetch
19+
&& $expr->name instanceof Identifier
20+
&& $expr->name->name === 'holder'
21+
&& (new ObjectType('Bug15421\Crate'))->isSuperTypeOf($scope->getType($expr->var))->yes()) {
22+
return new ObjectType('Bug15421\Holder');
23+
}
24+
25+
return null;
26+
}
27+
28+
}

‎tests/PHPStan/Analyser/data/bug-15421.php‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15421;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
final class Day {
8+
public string $date = 'x';
9+
public function toDateString(): string { return 'x'; }
10+
public function next(): ?Day { return null; }
11+
}
12+
final class Holder {
13+
public ?Day $day = null;
14+
/** @var array{day?: Day|null} */
15+
public array $days = [];
16+
public function getDay(): ?Day { return $this->day; }
17+
}
18+
final class Crate {}
19+
20+
function test(Crate $crate): void
21+
{
22+
$a = $crate->holder->day?->toDateString(); // reported on 2.3.0
23+
$h = $crate->holder;
24+
$b = $h->day?->toDateString(); // not reported
25+
}
26+
27+
function analogous(Crate $crate): void
28+
{
29+
assertType('Bug15421\Holder', $crate->holder);
30+
assertType('Bug15421\Day|null', $crate->holder->day);
31+
assertType('string|null', $crate->holder->day?->toDateString());
32+
assertType('string|null', $crate->holder->day?->date);
33+
assertType('Bug15421\Day|null', $crate->holder->day?->next()?->next());
34+
assertType('string|null', $crate->holder->getDay()?->toDateString());
35+
assertType('string|null', $crate->holder->getDay()?->date);
36+
assertType('string|null', $crate->holder->day->date ?? null);
37+
assertType('string|null', $crate->holder->days['day']?->date);
38+
assertType('bool', isset($crate->holder->day->date));
39+
assertType('bool', empty($crate->holder->day->date));
40+
if ($crate->holder->day !== null) {
41+
assertType('Bug15421\Day', $crate->holder->day);
42+
assertType('string', $crate->holder->day->toDateString());
43+
}
44+
}

‎tests/PHPStan/Analyser/expression-type-resolver-extension.neon‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
# config for ExpressionTypeResolverExtensionTest
2+
parameters:
3+
universalObjectCratesClasses:
4+
- Bug15421\Crate
5+
26
services:
37
-
48
class: ExpressionTypeResolverExtension\MethodCallReturnsBoolExpressionTypeResolverExtension
59
tags:
610
- phpstan.broker.expressionTypeResolverExtension
7-
11+
-
12+
class: ExpressionTypeResolverExtension\Bug15421ExpressionTypeResolverExtension
13+
tags:
14+
- phpstan.broker.expressionTypeResolverExtension

‎tests/PHPStan/Rules/Methods/CallMethodsRuleExpressionTypeResolverExtensionTest.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Methods;
4+
5+
use PHPStan\Rules\FunctionCallParametersCheck;
6+
use PHPStan\Rules\NonStringableDynamicAccessCheck;
7+
use PHPStan\Rules\NullsafeCheck;
8+
use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper;
9+
use PHPStan\Rules\Properties\PropertyReflectionFinder;
10+
use PHPStan\Rules\Rule;
11+
use PHPStan\Rules\RuleLevelHelper;
12+
use PHPStan\Testing\RuleTestCase;
13+
14+
/**
15+
* @extends RuleTestCase<CallMethodsRule>
16+
*/
17+
class CallMethodsRuleExpressionTypeResolverExtensionTest extends RuleTestCase
18+
{
19+
20+
protected function getRule(): Rule
21+
{
22+
$reflectionProvider = self::createReflectionProvider();
23+
$ruleLevelHelper = new RuleLevelHelper(
24+
$reflectionProvider,
25+
checkNullables: true,
26+
checkThisOnly: false,
27+
checkUnionTypes: true,
28+
checkExplicitMixed: false,
29+
checkImplicitMixed: false,
30+
checkBenevolentUnionTypes: false,
31+
discoveringSymbolsTip: true,
32+
);
33+
return new CallMethodsRule(
34+
new MethodCallCheck(
35+
$reflectionProvider,
36+
$ruleLevelHelper,
37+
checkFunctionNameCase: true,
38+
reportMagicMethods: true,
39+
),
40+
new FunctionCallParametersCheck(
41+
$ruleLevelHelper,
42+
new NullsafeCheck(),
43+
new UnresolvableTypeHelper(),
44+
new PropertyReflectionFinder(),
45+
$reflectionProvider,
46+
checkArgumentTypes: true,
47+
checkArgumentsPassedByReference: true,
48+
checkExtraArguments: true,
49+
checkMissingTypehints: true,
50+
),
51+
new NonStringableDynamicAccessCheck($ruleLevelHelper, true),
52+
);
53+
}
54+
55+
public function testBug15421(): void
56+
{
57+
$this->analyse([__DIR__ . '/../../Analyser/data/bug-15421.php'], []);
58+
}
59+
60+
public static function getAdditionalConfigFiles(): array
61+
{
62+
return [
63+
__DIR__ . '/../../Analyser/expression-type-resolver-extension.neon',
64+
];
65+
}
66+
67+
}

‎tests/PHPStan/Rules/Properties/AccessPropertiesRuleExpressionTypeResolverExtensionTest.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Properties;
4+
5+
use PHPStan\Php\PhpVersion;
6+
use PHPStan\Rules\NonStringableDynamicAccessCheck;
7+
use PHPStan\Rules\Rule;
8+
use PHPStan\Rules\RuleLevelHelper;
9+
use PHPStan\Testing\RuleTestCase;
10+
use const PHP_VERSION_ID;
11+
12+
/**
13+
* @extends RuleTestCase<AccessPropertiesRule>
14+
*/
15+
class AccessPropertiesRuleExpressionTypeResolverExtensionTest extends RuleTestCase
16+
{
17+
18+
protected function getRule(): Rule
19+
{
20+
$reflectionProvider = self::createReflectionProvider();
21+
$ruleLevelHelper = new RuleLevelHelper(
22+
$reflectionProvider,
23+
checkNullables: true,
24+
checkThisOnly: false,
25+
checkUnionTypes: true,
26+
checkExplicitMixed: false,
27+
checkImplicitMixed: false,
28+
checkBenevolentUnionTypes: false,
29+
discoveringSymbolsTip: true,
30+
);
31+
return new AccessPropertiesRule(
32+
new AccessPropertiesCheck(
33+
$reflectionProvider,
34+
$ruleLevelHelper,
35+
new PhpVersion(PHP_VERSION_ID),
36+
new NonStringableDynamicAccessCheck($ruleLevelHelper, true),
37+
reportMagicProperties: true,
38+
checkDynamicProperties: false,
39+
),
40+
);
41+
}
42+
43+
public function testBug15421(): void
44+
{
45+
$this->analyse([__DIR__ . '/../../Analyser/data/bug-15421.php'], []);
46+
}
47+
48+
public static function getAdditionalConfigFiles(): array
49+
{
50+
return [
51+
__DIR__ . '/../../Analyser/expression-type-resolver-extension.neon',
52+
];
53+
}
54+
55+
}

‎turbo-ext/src/MutatingScope.cpp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10549,6 +10549,32 @@ class MutatingScope
1054910549
ZVAL_OBJ(&exprZv, expr);
1055010550
bool isVariable;
1055110551
if (UNEXPECTED(!isInstance(zv::Ref(&exprZv), PT_CLASS_VARIABLE, isVariable))) return zv::Val();
10552+
if (!isVariable) {
10553+
/* mirror resolveType(): an extension's answer wins over both the
10554+
* tracked holder and the reflection-derived read below */
10555+
zv::Val askScope = native ? thisDoNotTreatPhpDocTypesAsCertain() : self_();
10556+
if (UNEXPECTED(askScope.isUndef())) return zv::Val();
10557+
zv::Ref extensions = service(PT_MSS_EXPRESSION_TYPE_RESOLVER_EXTENSIONS);
10558+
if (UNEXPECTED(!extensions.isObject())) return uninitializedProperty("expressionTypeResolverExtensions");
10559+
zv::Val all = pt_extensions_collection_get_all(extensions.asObject());
10560+
if (UNEXPECTED(all.isUndef())) return zv::Val();
10561+
if (EXPECTED(Z_TYPE_P(all.raw()) == IS_ARRAY)) {
10562+
zv::Args args{expr, askScope.raw()};
10563+
for (auto entry : zv::ArrRef(all.raw())) {
10564+
zv::Ref extension = entry.value().deref();
10565+
if (UNEXPECTED(!extension.isObject())) {
10566+
zend_throw_error(NULL, "Call to a member function getType() on %s", zend_zval_value_name(extension.raw()));
10567+
return zv::Val();
10568+
}
10569+
zv::Val type = pt_type_call(extension.asObject(), PT_LC("gettype"), 2, args);
10570+
if (UNEXPECTED(type.isUndef())) return zv::Val();
10571+
if (!type.isNull()) return type;
10572+
}
10573+
} else {
10574+
zend_error(E_WARNING, "foreach() argument must be of type array|object, %s given", zend_zval_value_name(all.raw()));
10575+
if (UNEXPECTED(EG(exception))) return zv::Val();
10576+
}
10577+
}
1055210578
if (!isVariable) {
1055310579
zv::Val has = thisHasExpressionType(&exprZv);
1055410580
if (UNEXPECTED(has.isUndef())) return zv::Val();

0 commit comments

Comments
 (0)