Skip to content
Merged
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
71 changes: 39 additions & 32 deletions src/Analyser/TypeSpecifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -758,10 +758,10 @@ public function specifyTypesInCondition(
$result = $result->setAlwaysOverwriteTypes();
}
return $result->setNewConditionalExpressionHolders($this->mergeConditionalHolders([
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, false, true, $rightScope),
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, false, true, $scope),
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, true, true, $rightScope),
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, true, true, $scope),
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, false, true, $rightScope, $expr->right),
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, false, true, $scope, $expr->left),
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, true, true, $rightScope, $expr->right),
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, true, true, $scope, $expr->left),
]))->setRootExpr($expr);
}

Expand Down Expand Up @@ -811,10 +811,10 @@ public function specifyTypesInCondition(
$result = $result->setAlwaysOverwriteTypes();
}
return $result->setNewConditionalExpressionHolders($this->mergeConditionalHolders([
$this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, false, false, $rightScope),
$this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, false, false, $scope),
$this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, true, false, $rightScope),
$this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, true, false, $scope),
$this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, false, false, $rightScope, $expr->right),
$this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, false, false, $scope, $expr->left),
$this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, true, false, $rightScope, $expr->right),
$this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, true, false, $scope, $expr->left),
]))->setRootExpr($expr);
}

Expand Down Expand Up @@ -2106,7 +2106,7 @@ private function mergeConditionalHolders(array $holderLists): array
/**
* @return array<string, ConditionalExpressionHolder[]>
*/
private function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $conditionSpecifiedTypes, SpecifiedTypes $holderSpecifiedTypes, bool $holdersFromSureTypes, bool $holderSideIsNegated, Scope $rightScope): array
private function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $conditionSpecifiedTypes, SpecifiedTypes $holderSpecifiedTypes, bool $holdersFromSureTypes, bool $holderSideIsNegated, Scope $rightScope, ?Expr $holderSideExpr = null): array
{
// The condition side asserts that its sub-expression evaluates truthy.
// When that sub-expression is itself a compound boolean (e.g. `$a && $b`),
Expand Down Expand Up @@ -2147,29 +2147,17 @@ private function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $co
$holders = [];
$holderTypes = $holdersFromSureTypes ? $holderSpecifiedTypes->getSureTypes() : $holderSpecifiedTypes->getSureNotTypes();

// In the `BooleanAnd` false context a true condition implies the other
// side is false. When that other side is a plain conjunction
// (`$a === null && $b === null`), its false narrowing is empty, so the
// holder narrowings are re-derived by swapping the truthy sure/sureNot
// lists — landing in the sureNot list here. The implied negation is then a
// disjunction (`$a !== null || $b !== null`) that cannot be expressed as
// independent per-expression narrowings: splitting it into one holder per
// expression would let each fire on its own and over-narrow, so no holder
// is produced. A genuine negated conjunction (`!($a instanceof X && ...)`)
// instead asserts every conjunct true through the sure list, which is sound
// to split; likewise the `BooleanOr` true context. Only the swapped
// sureNot multi-target case is suppressed.
if ($holderSideIsNegated && !$holdersFromSureTypes) {
$trackableHolderCount = 0;
foreach ($holderTypes as [$holderExpr]) {
if (!$this->isTrackableExpression($holderExpr)) {
continue;
}
$trackableHolderCount++;
}
if ($trackableHolderCount > 1) {
return [];
}
// A holder side that is itself a compound boolean cannot always be split
// into independent per-expression holders. In the `BooleanAnd` false
// context the holder asserts its side is false: when that side is a
// conjunction (`$a && $b`), its negation is the disjunction `!$a || !$b`,
// which has no per-expression narrowing — narrowing each conjunct
// independently would drop a reachable value (e.g. `$a = false, $b = true`).
// Symmetrically, in the `BooleanOr` true context the holder asserts its
// side is true, and a disjunction side (`$a || $b`) is itself a disjunction.
// Such a side is left whole rather than split into over-narrowing holders.
if ($this->isUnsplittableCompoundHolderSide($holderSideExpr, $holderSideIsNegated)) {
return [];
}

foreach ($holderTypes as $exprString => [$expr, $type]) {
Expand Down Expand Up @@ -2217,6 +2205,25 @@ private function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $co
return [];
}

/**
* A holder side whose truth value is asserted as a disjunction cannot be
* decomposed into independent per-expression holders. That happens for a
* conjunction (`&&`) asserted false (negated context) and for a disjunction
* (`||`) asserted true.
*/
private function isUnsplittableCompoundHolderSide(?Expr $holderSideExpr, bool $holderSideIsNegated): bool
{
if ($holderSideExpr === null) {
return false;
}

if ($holderSideIsNegated) {
return $holderSideExpr instanceof BooleanAnd || $holderSideExpr instanceof LogicalAnd;
}

return $holderSideExpr instanceof BooleanOr || $holderSideExpr instanceof LogicalOr;
}

private function isTrackableExpression(Expr $expr): bool
{
if ($expr instanceof Expr\Variable) {
Expand Down
56 changes: 56 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-14787.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php declare(strict_types = 1);

namespace Bug14787;

use function PHPStan\Testing\assertType;

function reported(bool $a, bool $b, bool $c): void
{
// After `!($a && $b && $c)` only the disjunction `!$a || !$b || !$c` is known.
// Knowing `$c` is true does not make `$b` false (e.g. $a=false, $b=true, $c=true).
if ($a && $b && $c) {
return;
}

if ($c) {
assertType('bool', $a);
assertType('bool', $b);
}

// Both operand orders must behave the same.
if ($c && $b) {
assertType('true', $c);
assertType('true', $b);
}
if ($b && $c) {
assertType('true', $b);
assertType('true', $c);
}
}

function twoOperandsStillNarrow(bool $b, bool $c): void
{
// A non-compound side keeps its sound narrowing: `!($b && $c)` with `$c` true
// does imply `$b` is false.
if ($b && $c) {
return;
}

if ($c) {
assertType('false', $b);
}
}

function disjunctionOperandNarrows(bool $p, bool $q, bool $r): void
{
// `!(($p || $q) && $r)` with `$r` true implies `!($p || $q)`, i.e. both false.
// The negation of a disjunction is a conjunction, so this split is sound.
if (($p || $q) && $r) {
return;
}

if ($r) {
assertType('false', $p);
assertType('false', $q);
}
}
Loading