Skip to content

Commit 058b077

Browse files
github-actions[bot]phpstan-bot
authored andcommitted
Fix phpstan/phpstan#13853: skip readOnlyAssignNotInConstructor when isset guard is present
- Track PropertyInitializationExpr with NeverType in the falsy branch of isset($this->prop) in TypeSpecifier - Skip readOnlyAssignNotInConstructor error in ReadOnlyPropertyAssignRule when PropertyInitializationExpr indicates property is known uninitialized - Adjust IssetCheck and ClassPropertiesNode to treat NeverType PropertyInitializationExpr as "not initialized" - New regression test in tests/PHPStan/Rules/Properties/data/bug-13853.php
1 parent d8f5be7 commit 058b077

6 files changed

Lines changed: 116 additions & 6 deletions

File tree

src/Analyser/TypeSpecifier.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use PhpParser\Node\Name;
2222
use PHPStan\DependencyInjection\AutowiredService;
2323
use PHPStan\Node\Expr\AlwaysRememberedExpr;
24+
use PHPStan\Node\Expr\PropertyInitializationExpr;
2425
use PHPStan\Node\Expr\TypeExpr;
2526
use PHPStan\Node\IssetExpr;
2627
use PHPStan\Node\Printer\ExprPrinter;
@@ -1028,7 +1029,21 @@ public function specifyTypesInCondition(
10281029
}
10291030
}
10301031

1031-
return new SpecifiedTypes();
1032+
if (
1033+
$issetExpr instanceof PropertyFetch
1034+
&& $issetExpr->var instanceof Expr\Variable
1035+
&& $issetExpr->var->name === 'this'
1036+
&& $issetExpr->name instanceof Node\Identifier
1037+
) {
1038+
return $this->create(
1039+
new PropertyInitializationExpr($issetExpr->name->toString()),
1040+
new NeverType(),
1041+
TypeSpecifierContext::createTrue(),
1042+
$scope,
1043+
)->setRootExpr($expr);
1044+
}
1045+
1046+
return new SpecifiedTypes();
10321047
}
10331048

10341049
$tmpVars = [$issetExpr];

src/Node/ClassPropertiesNode.php

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ public function getUninitializedProperties(
212212
if (array_key_exists($propertyName, $initializedPropertiesMap)) {
213213
$hasInitialization = $initializedPropertiesMap[$propertyName];
214214
if (in_array($function->getName(), $constructors, true)) {
215-
$hasInitialization = $hasInitialization->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName)));
215+
$hasInitialization = $hasInitialization->or(self::hasPropertyInitialization($usageScope, $propertyName));
216216
}
217217
if (
218218
!$hasInitialization->no()
@@ -234,9 +234,9 @@ public function getUninitializedProperties(
234234
) {
235235
continue;
236236
}
237-
$hasInitialization = $initializedPropertiesMap[$propertyName]->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName)));
237+
$hasInitialization = $initializedPropertiesMap[$propertyName]->or(self::hasPropertyInitialization($usageScope, $propertyName));
238238
if (!$hasInitialization->yes() && $usageScope->isInAnonymousFunction() && $usageScope->getParentScope() !== null) {
239-
$hasInitialization = $hasInitialization->or($usageScope->getParentScope()->hasExpressionType(new PropertyInitializationExpr($propertyName)));
239+
$hasInitialization = $hasInitialization->or(self::hasPropertyInitialization($usageScope->getParentScope(), $propertyName));
240240
}
241241
if (!$hasInitialization->yes()) {
242242
$prematureAccess[] = [
@@ -304,7 +304,7 @@ private function collectUninitializedProperties(array $constructors, array $unin
304304
}
305305

306306
foreach (array_keys($uninitializedProperties) as $propertyName) {
307-
if (!$methodScope->hasExpressionType(new PropertyInitializationExpr($propertyName))->yes()) {
307+
if (!self::hasPropertyInitialization($methodScope, $propertyName)->yes()) {
308308
continue;
309309
}
310310

@@ -405,12 +405,23 @@ private function getMethodsCalledFromConstructor(
405405
private function getInitializedProperties(Scope $scope, array $initialInitializedProperties): array
406406
{
407407
foreach ($initialInitializedProperties as $propertyName => $isInitialized) {
408-
$initialInitializedProperties[$propertyName] = $isInitialized->or($scope->hasExpressionType(new PropertyInitializationExpr($propertyName)));
408+
$initialInitializedProperties[$propertyName] = $isInitialized->or(self::hasPropertyInitialization($scope, $propertyName));
409409
}
410410

411411
return $initialInitializedProperties;
412412
}
413413

414+
private static function hasPropertyInitialization(Scope $scope, string $propertyName): TrinaryLogic
415+
{
416+
$initExpr = new PropertyInitializationExpr($propertyName);
417+
$has = $scope->hasExpressionType($initExpr);
418+
if ($has->yes() && $scope->getType($initExpr) instanceof NeverType) {
419+
return TrinaryLogic::createNo();
420+
}
421+
422+
return $has;
423+
}
424+
414425
/**
415426
* @return list<PropertyAssign>
416427
*/

src/Rules/IssetCheck.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str
156156
&& $expr->var instanceof Expr\Variable
157157
&& $expr->var->name === 'this'
158158
&& $scope->hasExpressionType(new PropertyInitializationExpr($propertyReflection->getName()))->yes()
159+
&& !$scope->getType(new PropertyInitializationExpr($propertyReflection->getName())) instanceof NeverType
159160
) {
160161
return $this->generateError(
161162
$propertyReflection->getNativeType(),

src/Rules/Properties/ReadOnlyPropertyAssignRule.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use PhpParser\Node;
77
use PHPStan\Analyser\Scope;
88
use PHPStan\DependencyInjection\RegisteredRule;
9+
use PHPStan\Node\Expr\PropertyInitializationExpr;
910
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
1011
use PHPStan\Node\Expr\UnsetOffsetExpr;
1112
use PHPStan\Node\PropertyAssignNode;
@@ -14,6 +15,7 @@
1415
use PHPStan\Rules\Rule;
1516
use PHPStan\Rules\RuleErrorBuilder;
1617
use PHPStan\ShouldNotHappenException;
18+
use PHPStan\Type\NeverType;
1719
use PHPStan\Type\ObjectType;
1820
use PHPStan\Type\TypeUtils;
1921
use function in_array;
@@ -111,6 +113,16 @@ public function processNode(Node $node, Scope $scope): array
111113
continue;
112114
}
113115

116+
if (
117+
$propertyFetch->var instanceof Node\Expr\Variable
118+
&& $propertyFetch->var->name === 'this'
119+
&& $propertyFetch->name instanceof Node\Identifier
120+
&& $scope->hasExpressionType(new PropertyInitializationExpr($propertyFetch->name->toString()))->yes()
121+
&& $scope->getType(new PropertyInitializationExpr($propertyFetch->name->toString())) instanceof NeverType
122+
) {
123+
continue;
124+
}
125+
114126
$errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is assigned outside of the constructor.', $declaringClass->getDisplayName(), $propertyReflection->getName()))
115127
->line($propertyFetch->name->getStartLine())
116128
->identifier('property.readOnlyAssignNotInConstructor')

tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,4 +180,15 @@ public function testCloneWith(): void
180180
$this->analyse([__DIR__ . '/data/readonly-property-assign-clone-with.php'], []);
181181
}
182182

183+
#[RequiresPhp('>= 8.1')]
184+
public function testBug13853(): void
185+
{
186+
$this->analyse([__DIR__ . '/data/bug-13853.php'], [
187+
[
188+
'Readonly property Bug13853\NoIssetGuard::$prop is assigned outside of the constructor.',
189+
58,
190+
],
191+
]);
192+
}
193+
183194
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php // lint >= 8.1
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug13853;
6+
7+
abstract class BaseReportLocator
8+
{
9+
private readonly string $report;
10+
11+
public function __construct(
12+
private readonly string $defaultPathname,
13+
) {
14+
}
15+
16+
final public function locate(): string
17+
{
18+
if (isset($this->report)) {
19+
return $this->report;
20+
}
21+
22+
$this->report = is_file($this->defaultPathname)
23+
? $this->defaultPathname
24+
: $this->lookup();
25+
26+
return $this->report;
27+
}
28+
29+
public abstract function lookup(): string;
30+
}
31+
32+
class AnotherExample
33+
{
34+
private readonly int $value;
35+
36+
public function getValue(): int
37+
{
38+
if (!isset($this->value)) {
39+
$this->value = $this->compute();
40+
}
41+
42+
return $this->value;
43+
}
44+
45+
private function compute(): int
46+
{
47+
return 42;
48+
}
49+
}
50+
51+
class NoIssetGuard
52+
{
53+
private readonly string $prop;
54+
55+
public function setProp(): void
56+
{
57+
// no isset guard - should still report error
58+
$this->prop = 'foo';
59+
}
60+
}

0 commit comments

Comments
 (0)