Skip to content

Commit 5c0bbad

Browse files
phpstan-botclaude
authored andcommitted
Do not split a swapped negated-conjunction holder side into per-expression holders
When a `BooleanAnd` guard side is itself a plain conjunction (e.g. `$a === null && $b === null`), the false-context holder narrowings are re-derived by swapping the truthy sure/sureNot lists, landing in the sureNot list. The negation those holders must encode is a disjunction (`$a !== null || $b !== null`), which cannot be represented as independent per-expression narrowings. Splitting it let each holder fire on its own and over-narrow, dropping a reachable value. Suppress holder creation only for that swapped sureNot multi-target case. Genuine negated conjunctions (`!($x instanceof X && ...)`) assert every conjunct true through the sure list and the `BooleanOr` true context remain sound to split, so they are untouched. Closes phpstan/phpstan#14780 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d937244 commit 5c0bbad

2 files changed

Lines changed: 54 additions & 9 deletions

File tree

src/Analyser/TypeSpecifier.php

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -758,10 +758,10 @@ public function specifyTypesInCondition(
758758
$result = $result->setAlwaysOverwriteTypes();
759759
}
760760
return $result->setNewConditionalExpressionHolders($this->mergeConditionalHolders([
761-
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, false, $rightScope),
762-
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, false, $scope),
763-
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, true, $rightScope),
764-
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, true, $scope),
761+
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, false, true, $rightScope),
762+
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, false, true, $scope),
763+
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, true, true, $rightScope),
764+
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, true, true, $scope),
765765
]))->setRootExpr($expr);
766766
}
767767

@@ -811,10 +811,10 @@ public function specifyTypesInCondition(
811811
$result = $result->setAlwaysOverwriteTypes();
812812
}
813813
return $result->setNewConditionalExpressionHolders($this->mergeConditionalHolders([
814-
$this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, false, $rightScope),
815-
$this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, false, $scope),
816-
$this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, true, $rightScope),
817-
$this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, true, $scope),
814+
$this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, false, false, $rightScope),
815+
$this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, false, false, $scope),
816+
$this->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, true, false, $rightScope),
817+
$this->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, true, false, $scope),
818818
]))->setRootExpr($expr);
819819
}
820820

@@ -2106,7 +2106,7 @@ private function mergeConditionalHolders(array $holderLists): array
21062106
/**
21072107
* @return array<string, ConditionalExpressionHolder[]>
21082108
*/
2109-
private function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $conditionSpecifiedTypes, SpecifiedTypes $holderSpecifiedTypes, bool $holdersFromSureTypes, Scope $rightScope): array
2109+
private function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $conditionSpecifiedTypes, SpecifiedTypes $holderSpecifiedTypes, bool $holdersFromSureTypes, bool $holderSideIsNegated, Scope $rightScope): array
21102110
{
21112111
// The condition side asserts that its sub-expression evaluates truthy.
21122112
// When that sub-expression is itself a compound boolean (e.g. `$a && $b`),
@@ -2146,6 +2146,32 @@ private function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $co
21462146
if (count($conditionExpressionTypes) > 0) {
21472147
$holders = [];
21482148
$holderTypes = $holdersFromSureTypes ? $holderSpecifiedTypes->getSureTypes() : $holderSpecifiedTypes->getSureNotTypes();
2149+
2150+
// In the `BooleanAnd` false context a true condition implies the other
2151+
// side is false. When that other side is a plain conjunction
2152+
// (`$a === null && $b === null`), its false narrowing is empty, so the
2153+
// holder narrowings are re-derived by swapping the truthy sure/sureNot
2154+
// lists — landing in the sureNot list here. The implied negation is then a
2155+
// disjunction (`$a !== null || $b !== null`) that cannot be expressed as
2156+
// independent per-expression narrowings: splitting it into one holder per
2157+
// expression would let each fire on its own and over-narrow, so no holder
2158+
// is produced. A genuine negated conjunction (`!($a instanceof X && ...)`)
2159+
// instead asserts every conjunct true through the sure list, which is sound
2160+
// to split; likewise the `BooleanOr` true context. Only the swapped
2161+
// sureNot multi-target case is suppressed.
2162+
if ($holderSideIsNegated && !$holdersFromSureTypes) {
2163+
$trackableHolderCount = 0;
2164+
foreach ($holderTypes as [$holderExpr]) {
2165+
if (!$this->isTrackableExpression($holderExpr)) {
2166+
continue;
2167+
}
2168+
$trackableHolderCount++;
2169+
}
2170+
if ($trackableHolderCount > 1) {
2171+
return [];
2172+
}
2173+
}
2174+
21492175
foreach ($holderTypes as $exprString => [$expr, $type]) {
21502176
if (!$this->isTrackableExpression($expr)) {
21512177
continue;

tests/PHPStan/Analyser/nsrt/bug-14780.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,22 @@ function fourConditions(bool $a, bool $b, ?int $expiration, ?int $auto): void
4848
assertType('int|null', $auto);
4949
}
5050
}
51+
52+
function compoundHolderSide(?int $a, ?object $b, bool $mock): void
53+
{
54+
// The guarded side `$a === null && $b === null` is itself a conjunction.
55+
// Its negation `$a !== null || $b !== null` is a disjunction and must not be
56+
// split into independent `$mock => $a !== null` / `$mock => $b !== null`
57+
// holders that would each over-narrow.
58+
if ($a === null && $b === null && !$mock) {
59+
throw new \LogicException();
60+
}
61+
62+
if ($mock) {
63+
return;
64+
}
65+
66+
// only (a !== null || b !== null) is known here
67+
assertType('int|null', $a);
68+
assertType('object|null', $b);
69+
}

0 commit comments

Comments
 (0)