Skip to content

Commit d84b12a

Browse files
phpstan-botondrejmirtes
phpstan-bot
authored and
ondrejmirtes
committed
Derive the native-types flag from the asking scope in ExpressionResult::getTypeOnScope()
- `ExpressionResult::getTypeOnScope()` / `answersOnScope()` now OR the explicit `$useNativeTypes` argument with `$scope->nativeTypesPromoted`, so a caller that promotes the scope instead of passing the flag no longer falls through to `resolveOwnType(false)` and the PHPDoc flavour of an untracked expression's type. - Fixes `isset()`/`empty()` over an offset of a method call, property fetch, static method call, static property fetch, plain function call, `$this->prop` and `$this->method()` ignoring `treatPhpDocTypesAsCertain: false`. The isset/empty folds (`IssetHandler`, `EmptyHandler`, `DefaultNarrowingHelper`, `CoalesceCompositionHelper`) all pass a promoted scope with `$useNativeTypes: false`. - `FuncCallHandler::resolveReturnType()`: the `$expr->name instanceof Expr` branch (`$obj()` via `__invoke`, callable strings/arrays) now returns `getNativeReturnType()` when native types are promoted, the same shortcut the named-function branch already took. This one was broken on 2.2.x too. - Probed and found already correct: `??`, `?->` chains, `array_key_exists()`, ternary, nested offsets, and the method/static-call/property native return type resolution in `MethodCallHandler`, `StaticCallHandler`, `PropertyFetchHandler` and `StaticPropertyFetchHandler`, which all already branch on `$nativeTypesPromoted`.
1 parent bc36ada commit d84b12a

7 files changed

Lines changed: 350 additions & 0 deletions

File tree

‎src/Analyser/ExprHandler/FuncCallHandler.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,13 @@ private function resolveReturnType(NodeScopeResolver $nodeScopeResolver, Mutatin
631631
: ParametersAcceptorSelector::combineAcceptors($variants);
632632
}
633633

634+
// the same shortcut the named-function branch below takes: the native
635+
// signature answers on its own, before any phpdoc-based dynamic
636+
// return type extension gets a say
637+
if ($nativeTypesPromoted && $parametersAcceptor instanceof ExtendedParametersAcceptor) {
638+
return $parametersAcceptor->getNativeReturnType();
639+
}
640+
634641
$functionName = null;
635642
if ($expr->name instanceof String_) {
636643
/** @var non-empty-string $name */

‎src/Analyser/ExpressionResult.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,11 @@ public function getCreatedTypes(Type $type, TypeSpecifierContext $context, bool
564564
*/
565565
public function getTypeOnScope(MutatingScope $scope, bool $useNativeTypes): Type
566566
{
567+
// An already-promoted asking scope selects native types on its own - a
568+
// caller that promotes the scope instead of passing the flag (the
569+
// isset/empty/?? folds) must not fall through to the phpdoc flavour of
570+
// the result's own type.
571+
$useNativeTypes = $useNativeTypes || $scope->nativeTypesPromoted;
567572
$readScope = $useNativeTypes ? $scope->doNotTreatPhpDocTypesAsCertain() : $scope;
568573
// old-world resolveType() consulted these on every ask, both flavours -
569574
// a consumer reading a call's type here (an assign filling the target's
@@ -597,6 +602,7 @@ public function answersOnScope(MutatingScope $scope, bool $useNativeTypes): bool
597602
return true;
598603
}
599604

605+
$useNativeTypes = $useNativeTypes || $scope->nativeTypesPromoted;
600606
$readScope = $useNativeTypes ? $scope->doNotTreatPhpDocTypesAsCertain() : $scope;
601607

602608
return $this->isScopeAuthoritative($readScope) || $this->askScopeVariableStateMatches($scope, $useNativeTypes);

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15169;
4+
5+
use function PHPStan\Testing\assertNativeType;
6+
use function PHPStan\Testing\assertType;
7+
8+
class C
9+
{
10+
11+
/** @var array{a: int} */
12+
public array $prop = ['a' => 1];
13+
14+
/** @return array{a: int} */
15+
public function info(): array
16+
{
17+
return ['a' => 1];
18+
}
19+
20+
}
21+
22+
/** @return array{a: int} */
23+
function info(): array
24+
{
25+
return ['a' => 1];
26+
}
27+
28+
function methodCall(C $c): void
29+
{
30+
assertType('array{a: int}', $c->info());
31+
assertNativeType('array', $c->info());
32+
33+
assertType('false', isset($c->info()['zzz']));
34+
assertNativeType('bool', isset($c->info()['zzz']));
35+
36+
assertType('true', empty($c->info()['zzz']));
37+
assertNativeType('bool', empty($c->info()['zzz']));
38+
}
39+
40+
function propertyFetch(C $c): void
41+
{
42+
assertType('false', isset($c->prop['zzz']));
43+
assertNativeType('bool', isset($c->prop['zzz']));
44+
45+
assertType('true', empty($c->prop['zzz']));
46+
assertNativeType('bool', empty($c->prop['zzz']));
47+
}
48+
49+
function funcCall(): void
50+
{
51+
assertType('false', isset(info()['zzz']));
52+
assertNativeType('bool', isset(info()['zzz']));
53+
}
54+
55+
function coalesce(C $c): void
56+
{
57+
assertType("'default'", $c->info()['zzz'] ?? 'default');
58+
assertNativeType('mixed~null', $c->info()['zzz'] ?? 'default');
59+
}

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,4 +441,24 @@ public function testBug14473(): void
441441
$this->analyse([__DIR__ . '/data/bug-14473.php'], []);
442442
}
443443

444+
public function testBug15169(): void
445+
{
446+
$this->treatPhpDocTypesAsCertain = false;
447+
448+
$this->analyse([__DIR__ . '/../DeadCode/data/bug-15169.php'], []);
449+
}
450+
451+
public function testBug15169TreatPhpDocTypesAsCertain(): void
452+
{
453+
$this->treatPhpDocTypesAsCertain = true;
454+
455+
$this->analyse([__DIR__ . '/../DeadCode/data/bug-15169.php'], [
456+
[
457+
'Result of || is always true.',
458+
50,
459+
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
460+
],
461+
]);
462+
}
463+
444464
}

‎tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,4 +434,82 @@ public function testBug11731(): void
434434
]);
435435
}
436436

437+
public function testBug15169(): void
438+
{
439+
$this->treatPhpDocTypesAsCertain = false;
440+
$this->analyse([__DIR__ . '/data/bug-15169.php'], []);
441+
}
442+
443+
public function testBug15169TreatPhpDocTypesAsCertain(): void
444+
{
445+
$this->treatPhpDocTypesAsCertain = true;
446+
$this->analyse([__DIR__ . '/data/bug-15169.php'], [
447+
[
448+
'Unreachable statement - code above always terminates.',
449+
27,
450+
],
451+
[
452+
'Unreachable statement - code above always terminates.',
453+
36,
454+
],
455+
[
456+
'Unreachable statement - code above always terminates.',
457+
45,
458+
],
459+
[
460+
'Unreachable statement - code above always terminates.',
461+
54,
462+
],
463+
[
464+
'Unreachable statement - code above always terminates.',
465+
66,
466+
],
467+
[
468+
'Unreachable statement - code above always terminates.',
469+
76,
470+
],
471+
]);
472+
}
473+
474+
public function testBug15169Analogous(): void
475+
{
476+
$this->treatPhpDocTypesAsCertain = false;
477+
$this->analyse([__DIR__ . '/data/bug-15169b.php'], []);
478+
}
479+
480+
public function testBug15169AnalogousTreatPhpDocTypesAsCertain(): void
481+
{
482+
$this->treatPhpDocTypesAsCertain = true;
483+
$this->analyse([__DIR__ . '/data/bug-15169b.php'], [
484+
[
485+
'Unreachable statement - code above always terminates.',
486+
40,
487+
],
488+
[
489+
'Unreachable statement - code above always terminates.',
490+
49,
491+
],
492+
[
493+
'Unreachable statement - code above always terminates.',
494+
66,
495+
],
496+
[
497+
'Unreachable statement - code above always terminates.',
498+
75,
499+
],
500+
[
501+
'Unreachable statement - code above always terminates.',
502+
84,
503+
],
504+
[
505+
'Unreachable statement - code above always terminates.',
506+
93,
507+
],
508+
[
509+
'Unreachable statement - code above always terminates.',
510+
102,
511+
],
512+
]);
513+
}
514+
437515
}

‎tests/PHPStan/Rules/DeadCode/data/bug-15169.php‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php declare(strict_types = 1);
2+
3+
// Analyse with treatPhpDocTypesAsCertain: false
4+
5+
namespace Bug15169DeadCode;
6+
7+
class C
8+
{
9+
10+
/** @var array{a: int} */
11+
public array $prop = ['a' => 1];
12+
13+
/** @return array{a: int} */
14+
public function info(): array
15+
{
16+
return ['a' => 1];
17+
}
18+
19+
}
20+
21+
function viaCall(C $c): string
22+
{
23+
if (!isset($c->info()['zzz'])) {
24+
return 'early';
25+
}
26+
27+
return 'A';
28+
}
29+
30+
function viaProperty(C $c): string
31+
{
32+
if (!isset($c->prop['zzz'])) {
33+
return 'early';
34+
}
35+
36+
return 'C';
37+
}
38+
39+
function viaCallEmpty(C $c): string
40+
{
41+
if (empty($c->info()['zzz'])) {
42+
return 'early';
43+
}
44+
45+
return 'E';
46+
}
47+
48+
function orChain(?C $c): string
49+
{
50+
if ($c === null || !isset($c->info()['zzz'])) {
51+
return 'early';
52+
}
53+
54+
return 'F';
55+
}
56+
57+
// --- correct on every version: the same check through a variable ---
58+
59+
function viaVariable(C $c): string
60+
{
61+
$arr = $c->info();
62+
if (!isset($arr['zzz'])) {
63+
return 'early';
64+
}
65+
66+
return 'B';
67+
}
68+
69+
/** @param array{a: int} $arr */
70+
function viaParam(array $arr): string
71+
{
72+
if (!isset($arr['zzz'])) {
73+
return 'early';
74+
}
75+
76+
return 'D';
77+
}

‎tests/PHPStan/Rules/DeadCode/data/bug-15169b.php‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
<?php declare(strict_types = 1);
2+
3+
// Analyse with treatPhpDocTypesAsCertain: false
4+
5+
namespace Bug15169b;
6+
7+
class C
8+
{
9+
10+
/** @var array{a: int} */
11+
public array $prop = ['a' => 1];
12+
13+
/** @var array{a: int} */
14+
public static array $staticProp = ['a' => 1];
15+
16+
/** @return array{a: int} */
17+
public static function staticInfo(): array
18+
{
19+
return ['a' => 1];
20+
}
21+
22+
/** @return array{a: int} */
23+
public function __invoke(): array
24+
{
25+
return ['a' => 1];
26+
}
27+
28+
/** @return array{a: int} */
29+
public function info(): array
30+
{
31+
return ['a' => 1];
32+
}
33+
34+
public function viaThisProperty(): string
35+
{
36+
if (!isset($this->prop['zzz'])) {
37+
return 'early';
38+
}
39+
40+
return 'A';
41+
}
42+
43+
public function viaThisCall(): string
44+
{
45+
if (!isset($this->info()['zzz'])) {
46+
return 'early';
47+
}
48+
49+
return 'B';
50+
}
51+
52+
}
53+
54+
/** @return array{a: int} */
55+
function info(): array
56+
{
57+
return ['a' => 1];
58+
}
59+
60+
function viaStaticCall(): string
61+
{
62+
if (!isset(C::staticInfo()['zzz'])) {
63+
return 'early';
64+
}
65+
66+
return 'C';
67+
}
68+
69+
function viaStaticProperty(): string
70+
{
71+
if (!isset(C::$staticProp['zzz'])) {
72+
return 'early';
73+
}
74+
75+
return 'D';
76+
}
77+
78+
function viaFunctionCall(): string
79+
{
80+
if (!isset(info()['zzz'])) {
81+
return 'early';
82+
}
83+
84+
return 'E';
85+
}
86+
87+
function viaInvoke(C $c): string
88+
{
89+
if (!isset($c()['zzz'])) {
90+
return 'early';
91+
}
92+
93+
return 'F';
94+
}
95+
96+
function viaStaticPropertyEmpty(): string
97+
{
98+
if (empty(C::$staticProp['zzz'])) {
99+
return 'early';
100+
}
101+
102+
return 'G';
103+
}

0 commit comments

Comments
 (0)