diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index cceef0e7c0..9c1849aa59 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -21,6 +21,7 @@ use PhpParser\Node\Name; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Node\Expr\AlwaysRememberedExpr; +use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\TypeExpr; use PHPStan\Node\IssetExpr; use PHPStan\Node\Printer\ExprPrinter; @@ -1028,7 +1029,21 @@ public function specifyTypesInCondition( } } - return new SpecifiedTypes(); + if ( + $issetExpr instanceof PropertyFetch + && $issetExpr->var instanceof Expr\Variable + && $issetExpr->var->name === 'this' + && $issetExpr->name instanceof Node\Identifier + ) { + return $this->create( + new PropertyInitializationExpr($issetExpr->name->toString()), + new NeverType(), + TypeSpecifierContext::createTrue(), + $scope, + )->setRootExpr($expr); + } + + return new SpecifiedTypes(); } $tmpVars = [$issetExpr]; diff --git a/src/Node/ClassPropertiesNode.php b/src/Node/ClassPropertiesNode.php index f29a7cd962..ba4571db34 100644 --- a/src/Node/ClassPropertiesNode.php +++ b/src/Node/ClassPropertiesNode.php @@ -212,7 +212,7 @@ public function getUninitializedProperties( if (array_key_exists($propertyName, $initializedPropertiesMap)) { $hasInitialization = $initializedPropertiesMap[$propertyName]; if (in_array($function->getName(), $constructors, true)) { - $hasInitialization = $hasInitialization->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName))); + $hasInitialization = $hasInitialization->or(self::hasPropertyInitialization($usageScope, $propertyName)); } if ( !$hasInitialization->no() @@ -234,9 +234,9 @@ public function getUninitializedProperties( ) { continue; } - $hasInitialization = $initializedPropertiesMap[$propertyName]->or($usageScope->hasExpressionType(new PropertyInitializationExpr($propertyName))); + $hasInitialization = $initializedPropertiesMap[$propertyName]->or(self::hasPropertyInitialization($usageScope, $propertyName)); if (!$hasInitialization->yes() && $usageScope->isInAnonymousFunction() && $usageScope->getParentScope() !== null) { - $hasInitialization = $hasInitialization->or($usageScope->getParentScope()->hasExpressionType(new PropertyInitializationExpr($propertyName))); + $hasInitialization = $hasInitialization->or(self::hasPropertyInitialization($usageScope->getParentScope(), $propertyName)); } if (!$hasInitialization->yes()) { $prematureAccess[] = [ @@ -304,7 +304,7 @@ private function collectUninitializedProperties(array $constructors, array $unin } foreach (array_keys($uninitializedProperties) as $propertyName) { - if (!$methodScope->hasExpressionType(new PropertyInitializationExpr($propertyName))->yes()) { + if (!self::hasPropertyInitialization($methodScope, $propertyName)->yes()) { continue; } @@ -405,12 +405,23 @@ private function getMethodsCalledFromConstructor( private function getInitializedProperties(Scope $scope, array $initialInitializedProperties): array { foreach ($initialInitializedProperties as $propertyName => $isInitialized) { - $initialInitializedProperties[$propertyName] = $isInitialized->or($scope->hasExpressionType(new PropertyInitializationExpr($propertyName))); + $initialInitializedProperties[$propertyName] = $isInitialized->or(self::hasPropertyInitialization($scope, $propertyName)); } return $initialInitializedProperties; } + private static function hasPropertyInitialization(Scope $scope, string $propertyName): TrinaryLogic + { + $initExpr = new PropertyInitializationExpr($propertyName); + $has = $scope->hasExpressionType($initExpr); + if ($has->yes() && $scope->getType($initExpr) instanceof NeverType) { + return TrinaryLogic::createNo(); + } + + return $has; + } + /** * @return list */ diff --git a/src/Rules/IssetCheck.php b/src/Rules/IssetCheck.php index 2e1adcf1c6..5b4f47b68f 100644 --- a/src/Rules/IssetCheck.php +++ b/src/Rules/IssetCheck.php @@ -156,6 +156,7 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str && $expr->var instanceof Expr\Variable && $expr->var->name === 'this' && $scope->hasExpressionType(new PropertyInitializationExpr($propertyReflection->getName()))->yes() + && !$scope->getType(new PropertyInitializationExpr($propertyReflection->getName())) instanceof NeverType ) { return $this->generateError( $propertyReflection->getNativeType(), diff --git a/src/Rules/Properties/ReadOnlyPropertyAssignRule.php b/src/Rules/Properties/ReadOnlyPropertyAssignRule.php index 987e000256..eb60327617 100644 --- a/src/Rules/Properties/ReadOnlyPropertyAssignRule.php +++ b/src/Rules/Properties/ReadOnlyPropertyAssignRule.php @@ -6,6 +6,7 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; +use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\SetOffsetValueTypeExpr; use PHPStan\Node\Expr\UnsetOffsetExpr; use PHPStan\Node\PropertyAssignNode; @@ -14,6 +15,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\ShouldNotHappenException; +use PHPStan\Type\NeverType; use PHPStan\Type\ObjectType; use PHPStan\Type\TypeUtils; use function in_array; @@ -111,6 +113,16 @@ public function processNode(Node $node, Scope $scope): array continue; } + if ( + $propertyFetch->var instanceof Node\Expr\Variable + && $propertyFetch->var->name === 'this' + && $propertyFetch->name instanceof Node\Identifier + && $scope->hasExpressionType(new PropertyInitializationExpr($propertyFetch->name->toString()))->yes() + && $scope->getType(new PropertyInitializationExpr($propertyFetch->name->toString())) instanceof NeverType + ) { + continue; + } + $errors[] = RuleErrorBuilder::message(sprintf('Readonly property %s::$%s is assigned outside of the constructor.', $declaringClass->getDisplayName(), $propertyReflection->getName())) ->line($propertyFetch->name->getStartLine()) ->identifier('property.readOnlyAssignNotInConstructor') diff --git a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php index 40bbb2d5c3..fce215bc30 100644 --- a/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/ReadOnlyPropertyAssignRuleTest.php @@ -180,4 +180,15 @@ public function testCloneWith(): void $this->analyse([__DIR__ . '/data/readonly-property-assign-clone-with.php'], []); } + #[RequiresPhp('>= 8.1')] + public function testBug13853(): void + { + $this->analyse([__DIR__ . '/data/bug-13853.php'], [ + [ + 'Readonly property Bug13853\NoIssetGuard::$prop is assigned outside of the constructor.', + 58, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-13853.php b/tests/PHPStan/Rules/Properties/data/bug-13853.php new file mode 100644 index 0000000000..1e9d852754 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-13853.php @@ -0,0 +1,60 @@ += 8.1 + +declare(strict_types = 1); + +namespace Bug13853; + +abstract class BaseReportLocator +{ + private readonly string $report; + + public function __construct( + private readonly string $defaultPathname, + ) { + } + + final public function locate(): string + { + if (isset($this->report)) { + return $this->report; + } + + $this->report = is_file($this->defaultPathname) + ? $this->defaultPathname + : $this->lookup(); + + return $this->report; + } + + public abstract function lookup(): string; +} + +class AnotherExample +{ + private readonly int $value; + + public function getValue(): int + { + if (!isset($this->value)) { + $this->value = $this->compute(); + } + + return $this->value; + } + + private function compute(): int + { + return 42; + } +} + +class NoIssetGuard +{ + private readonly string $prop; + + public function setProp(): void + { + // no isset guard - should still report error + $this->prop = 'foo'; + } +}