Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 44 additions & 14 deletions src/Analyser/ExprHandler/AssignHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
use PHPStan\Node\Expr\TypeExpr;
use PHPStan\Node\Printer\ExprPrinter;
use PHPStan\Node\PropertyAssignNode;
use PHPStan\Node\VariableAssignNode;
use PHPStan\Php\PhpVersion;
Expand Down Expand Up @@ -84,6 +85,7 @@ final class AssignHandler implements ExprHandler
public function __construct(
private TypeSpecifier $typeSpecifier,
private PhpVersion $phpVersion,
private ExprPrinter $exprPrinter,
)
{
}
Expand Down Expand Up @@ -242,6 +244,7 @@ public function processAssignVar(
$type = $scopeBeforeAssignEval->getType($assignedExpr);

$conditionalExpressions = [];
$nullsafeProtectedExprStrings = $this->collectNullsafeProtectedExprStrings($assignedExpr);
if ($assignedExpr instanceof Ternary) {
$if = $assignedExpr->if;
if ($if === null) {
Expand All @@ -259,23 +262,23 @@ public function processAssignVar(
$truthyType->isSuperTypeOf($falseyType)->no()
&& $falseyType->isSuperTypeOf($truthyType)->no()
) {
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType);
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $nullsafeProtectedExprStrings);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $nullsafeProtectedExprStrings);
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $nullsafeProtectedExprStrings);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $nullsafeProtectedExprStrings);
}
}

$truthyType = TypeCombinator::removeFalsey($type);
if ($truthyType !== $type) {
$truthySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createTruthy());
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType);
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $nullsafeProtectedExprStrings);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $nullsafeProtectedExprStrings);

$falseyType = TypeCombinator::intersect($type, StaticTypeFactory::falsey());
$falseySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createFalsey());
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType);
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $nullsafeProtectedExprStrings);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $nullsafeProtectedExprStrings);
}

foreach ([null, false, 0, 0.0, '', '0', []] as $falseyScalar) {
Expand Down Expand Up @@ -304,13 +307,13 @@ public function processAssignVar(

$notIdenticalConditionExpr = new Expr\BinaryOp\NotIdentical($assignedExpr, $astNode);
$notIdenticalSpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $notIdenticalConditionExpr, TypeSpecifierContext::createTrue());
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $notIdenticalSpecifiedTypes, $withoutFalseyType);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $notIdenticalSpecifiedTypes, $withoutFalseyType);
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $notIdenticalSpecifiedTypes, $withoutFalseyType, $nullsafeProtectedExprStrings);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $notIdenticalSpecifiedTypes, $withoutFalseyType, $nullsafeProtectedExprStrings);

$identicalConditionExpr = new Expr\BinaryOp\Identical($assignedExpr, $astNode);
$identicalSpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $identicalConditionExpr, TypeSpecifierContext::createTrue());
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType);
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType, $nullsafeProtectedExprStrings);
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType, $nullsafeProtectedExprStrings);
}

$nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $assignedExpr), $scopeBeforeAssignEval, $storage);
Expand Down Expand Up @@ -848,9 +851,10 @@ private function unwrapAssign(Expr $expr): Expr

/**
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
* @param array<string, true> $nullsafeProtectedExprStrings
* @return array<string, ConditionalExpressionHolder[]>
*/
private function processSureTypesForConditionalExpressionsAfterAssign(Scope $scope, string $variableName, array $conditionalExpressions, SpecifiedTypes $specifiedTypes, Type $variableType): array
private function processSureTypesForConditionalExpressionsAfterAssign(Scope $scope, string $variableName, array $conditionalExpressions, SpecifiedTypes $specifiedTypes, Type $variableType, array $nullsafeProtectedExprStrings = []): array
{
foreach ($specifiedTypes->getSureTypes() as $exprString => [$expr, $exprType]) {
if ($expr instanceof Variable) {
Expand All @@ -871,6 +875,10 @@ private function processSureTypesForConditionalExpressionsAfterAssign(Scope $sco
continue;
}

if (isset($nullsafeProtectedExprStrings[$exprString])) {
continue;
}

if (!isset($conditionalExpressions[$exprString])) {
$conditionalExpressions[$exprString] = [];
}
Expand All @@ -889,9 +897,10 @@ private function processSureTypesForConditionalExpressionsAfterAssign(Scope $sco

/**
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
* @param array<string, true> $nullsafeProtectedExprStrings
* @return array<string, ConditionalExpressionHolder[]>
*/
private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $scope, string $variableName, array $conditionalExpressions, SpecifiedTypes $specifiedTypes, Type $variableType): array
private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $scope, string $variableName, array $conditionalExpressions, SpecifiedTypes $specifiedTypes, Type $variableType, array $nullsafeProtectedExprStrings = []): array
{
foreach ($specifiedTypes->getSureNotTypes() as $exprString => [$expr, $exprType]) {
if ($expr instanceof Variable) {
Expand All @@ -912,6 +921,10 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $
continue;
}

if (isset($nullsafeProtectedExprStrings[$exprString])) {
continue;
}

if (!isset($conditionalExpressions[$exprString])) {
$conditionalExpressions[$exprString] = [];
}
Expand All @@ -928,6 +941,23 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $
return $conditionalExpressions;
}

/**
* @return array<string, true>
*/
private function collectNullsafeProtectedExprStrings(Expr $expr): array
{
$result = [];
$current = $expr;
while ($current instanceof Expr\NullsafeMethodCall || $current instanceof Expr\NullsafePropertyFetch) {
$var = $current->var;
if ($var instanceof MethodCall || $var instanceof Expr\StaticCall) {
$result[$this->exprPrinter->printExpr($var)] = true;
}
$current = $var;
}
return $result;
}

/**
* @param list<ArrayDimFetch> $dimFetchStack
*/
Expand Down
63 changes: 63 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-14493.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php // lint >= 8.0

declare(strict_types = 1);

namespace Bug14493Nsrt;

use function PHPStan\Testing\assertType;

class OrderEntity {
static public function getStaticOrderCustomer(): ?OrderCustomerEntity
{
return new OrderCustomerEntity();
}

public function getOrderCustomer(): ?OrderCustomerEntity
{
return new OrderCustomerEntity();
}
}

class OrderCustomerEntity {
public function getCustomer(): ?CustomerEntity { return null; }
/** @return array<string>|null */
public function getVatIds(): ?array { return null; }
}

class CustomerEntity {
final public const ACCOUNT_TYPE_BUSINESS = 'business';

public function getAccountType(): string { return ''; }
}

class TypeTest
{
public function doFoo(OrderEntity $order): void
{
$customerType = $order->getOrderCustomer()?->getCustomer()?->getAccountType();
assertType('string|null', $customerType);

if ($customerType !== CustomerEntity::ACCOUNT_TYPE_BUSINESS) {
return;
}

assertType("'business'", $customerType);

// The method still returns nullable - the nullsafe chain narrowing
// should not leak to the broader scope for subsequent calls
assertType('Bug14493Nsrt\OrderCustomerEntity|null', $order->getOrderCustomer());
}

public function doBar(OrderEntity $order): void
{
$customerType = $order::getStaticOrderCustomer()?->getCustomer()?->getAccountType();
assertType('string|null', $customerType);

if ($customerType !== CustomerEntity::ACCOUNT_TYPE_BUSINESS) {
return;
}

assertType("'business'", $customerType);
assertType('Bug14493Nsrt\OrderCustomerEntity|null', $order::getStaticOrderCustomer());
}
}
6 changes: 6 additions & 0 deletions tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,10 @@ public function testBug12222(): void
$this->analyse([__DIR__ . '/data/bug-12222.php'], []);
}

#[RequiresPhp('>= 8.0.0')]
public function testBug14493(): void
{
$this->analyse([__DIR__ . '/data/bug-14493.php'], []);
}

}
63 changes: 63 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-14493.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php // lint >= 8.0

declare(strict_types=1);

namespace Bug14493;

class OrderEntity {
static public function getStaticOrderCustomer(): ?OrderCustomerEntity
{
return new OrderCustomerEntity();
}

public function getOrderCustomer(): ?OrderCustomerEntity
{
return new OrderCustomerEntity();
}
}

class OrderCustomerEntity {
public function getCustomer(): ?CustomerEntity { return null; }
/** @return array<string>|null */
public function getVatIds(): ?array { return null; }
}

class CustomerEntity {
final public const ACCOUNT_TYPE_BUSINESS = 'business';

public function getAccountType(): string { return ''; }
}


abstract class AbstractDocumentRenderer
{
protected function doFoo(OrderEntity $order): bool
{
$customerType = $order->getOrderCustomer()?->getCustomer()?->getAccountType();
if ($customerType !== CustomerEntity::ACCOUNT_TYPE_BUSINESS) {
return false;
}

$vatIds = $order->getOrderCustomer()?->getVatIds();
if (!is_array($vatIds)) {
return false;
}

return true;
}

protected function doBar(OrderEntity $order): bool
{
$customerType = $order::getStaticOrderCustomer()?->getCustomer()?->getAccountType();
if ($customerType !== CustomerEntity::ACCOUNT_TYPE_BUSINESS) {
return false;
}

$vatIds = $order::getStaticOrderCustomer()?->getVatIds();
if (!is_array($vatIds)) {
return false;
}

return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,10 @@ public function testBug6922(): void
$this->analyse([__DIR__ . '/data/bug-6922.php'], []);
}

#[RequiresPhp('>= 8.0.0')]
public function testBug14493(): void
{
$this->analyse([__DIR__ . '/data/bug-14493.php'], []);
}

}
38 changes: 38 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-14493.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php // lint >= 8.0

declare(strict_types=1);

namespace Bug14493NullsafeProperty;

class Inner {
public string $value = '';
public ?string $nullableValue = null;
}

class Middle {
public ?Inner $inner = null;
public function getInner(): ?Inner { return $this->inner; }
}

class Outer {
public function getMiddle(): ?Middle { return null; }
}

class TestPropertyFetch
{
public function doFoo(Outer $outer): bool
{
$value = $outer->getMiddle()?->getInner()?->value;
if ($value !== 'expected') {
return false;
}

// After narrowing, $outer->getMiddle() should NOT be flagged
$nullableValue = $outer->getMiddle()?->inner?->nullableValue;
if ($nullableValue === null) {
return false;
}

return true;
}
}
Loading