diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 0ba5e1a7a5..3ea9223f7c 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -26,6 +26,7 @@ use PHPStan\Type\ErrorType; use PHPStan\Type\Generic\TemplateType; use PHPStan\Type\IntegerRangeType; +use PHPStan\Type\MixedType; use PHPStan\Type\NeverType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; @@ -412,6 +413,32 @@ public function check( ->line($argumentLine) ->acceptsReasonsTip($accepts->reasons) ->build(); + } elseif ($argumentValue instanceof Expr\Ternary && $argumentValueType instanceof MixedType) { + // Type normalization can collapse a ternary's resulting type to + // mixed (e.g. mixed|string becomes mixed), hiding a branch whose + // type is not accepted. When the whole argument collapsed to mixed, + // inspect the branch types separately so such a passed value is + // still reported. A non-mixed resulting type keeps enough + // information for the regular check above, so branch inspection + // would only introduce false positives there. + foreach ($this->getTernaryBranchTypes($argumentValue, $scope) as $branchType) { + $branchAccepts = $this->ruleLevelHelper->accepts($parameterType, $branchType, $isStrictTypes); + if ($branchAccepts->result) { + continue; + } + + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType, $branchType); + $errors[] = RuleErrorBuilder::message(sprintf( + $wrongArgumentTypeMessage, + $this->describeParameter($parameter, $argumentName ?? $i + 1), + $parameterType->describe($verbosityLevel), + $branchType->describe($verbosityLevel), + )) + ->identifier('argument.type') + ->line($argumentLine) + ->acceptsReasonsTip($branchAccepts->reasons) + ->build(); + } } } @@ -801,6 +828,42 @@ private function describeParameter(ParameterReflection $parameter, int|string|nu return implode(' ', $parts); } + /** + * Collects the leaf types of a ternary's branches, each resolved in the scope + * narrowed by the controlling condition. Nested ternaries are flattened so every + * value the expression can produce is represented by its own (un-normalized) type. + * + * The else branch is narrowed by the negated condition (`filterByTruthyValue` of + * `!cond`) rather than `filterByFalseyValue($cond)`, mirroring how + * TernaryHandler::specifyTypes models the else branch. Some conditions (e.g. + * `is_resource()`, whose stub only declares `@phpstan-assert-if-true`) narrow + * asymmetrically, so the falsey scope would otherwise diverge from the type the + * ternary actually produces and report spurious branch types. + * + * @return list + */ + private function getTernaryBranchTypes(Expr\Ternary $ternary, Scope $scope): array + { + $truthyScope = $scope->filterByTruthyValue($ternary->cond); + $falseyScope = $scope->filterByTruthyValue(new Expr\BooleanNot($ternary->cond)); + + if ($ternary->if === null) { + $ifTypes = [TypeCombinator::removeFalsey($truthyScope->getType($ternary->cond))]; + } elseif ($ternary->if instanceof Expr\Ternary) { + $ifTypes = $this->getTernaryBranchTypes($ternary->if, $truthyScope); + } else { + $ifTypes = [$truthyScope->getType($ternary->if)]; + } + + if ($ternary->else instanceof Expr\Ternary) { + $elseTypes = $this->getTernaryBranchTypes($ternary->else, $falseyScope); + } else { + $elseTypes = [$falseyScope->getType($ternary->else)]; + } + + return array_merge($ifTypes, $elseTypes); + } + /** * @return list|null Null when the expression is not a constant or bitmask of constants */ diff --git a/tests/PHPStan/Analyser/nsrt/bug-11281.php b/tests/PHPStan/Analyser/nsrt/bug-11281.php index fa9d543098..3d9fc067dd 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11281.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11281.php @@ -25,6 +25,48 @@ function hello2(string $values): void } } +/** + * The merged subtype-absorbed variable must survive as a conditional target + * regardless of which control-flow form reads the guard afterwards. + */ +function positiveGuard(string $values): void +{ + $ok = false; + try { + $values = array_map(static fn ($item) => Hello::fromObject($item), json_decode($values)); + $ok = true; + } catch (\Throwable) { + } + if ($ok) { + assertType('array', $values); + } +} + +function nestedGuard(string $values, bool $other): void +{ + $ok = false; + try { + $values = array_map(static fn ($item) => Hello::fromObject($item), json_decode($values)); + $ok = true; + } catch (\Throwable) { + } + if ($other && $ok) { + assertType('array', $values); + } +} + +function ternaryGuard(string $values): void +{ + $ok = false; + try { + $values = array_map(static fn ($item) => Hello::fromObject($item), json_decode($values)); + $ok = true; + } catch (\Throwable) { + } + $result = $ok ? $values : []; + assertType('array', $result); +} + final class Hello { diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 1d4dd576ae..7df47077e5 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -2978,4 +2978,22 @@ public function testBug11494(): void ]); } + public function testBug11281(): void + { + $this->analyse([__DIR__ . '/data/bug-11281.php'], [ + [ + 'Parameter #1 $i of function Bug11281Functions\sayHello expects int, string given.', + 18, + ], + [ + 'Parameter #1 $i of function Bug11281Functions\sayHello expects int, string given.', + 35, + ], + [ + 'Parameter #1 $s of function Bug11281Functions\expectsString expects string, string|false given.', + 64, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-11281.php b/tests/PHPStan/Rules/Functions/data/bug-11281.php new file mode 100644 index 0000000000..78cd18e10e --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-11281.php @@ -0,0 +1,68 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug11281Functions; + +function sayHello(int $i): void +{ +} + +/** + * @param array $values + */ +function test(array $values): void +{ + // The ternary's resulting type normalizes to mixed (mixed|string), + // but the else branch is definitely a string passed to an int parameter. + sayHello(array_key_exists('key', $values) ? $values['key'] : ' a string'); +} + +/** + * @param array $values + */ +function noError(array $values): void +{ + // Numeric-ish coercible branches must not be flagged. + sayHello(array_key_exists('key', $values) ? $values['key'] : 5); +} + +/** + * @param array $values + */ +function nested(array $values, bool $other, bool $another): void +{ + sayHello($other ? $values['key'] : ($another ? 1 : ' nested string')); +} + +function expectsString(string $s): void +{ +} + +function benevolentBranchNotReported(mixed $value): void +{ + // stream_get_contents() returns a *benevolent* string|false. PHPStan intentionally + // accepts benevolent unions for a string parameter, so the equivalent direct call + // expectsString(stream_get_contents($r)) is error-free too — reporting it here would + // re-introduce the pg_escape_bytea false positive this branch inspection guards + // against. is_resource() also narrows asymmetrically (@phpstan-assert-if-true only), + // so the else branch must keep the type the ternary actually produces (mixed, + // accepted) instead of a spurious narrowing. No error should be reported here. + expectsString( + is_resource($value) + ? stream_get_contents($value) + : $value, + ); +} + +function strictFalseBranchReported(mixed $value, string|false $sf): void +{ + // A *strict* (non-benevolent) string|false branch is not accepted by the string + // parameter, so branch inspection reports it even though is_resource() narrows + // asymmetrically and the ternary's resulting type normalizes to mixed. + expectsString( + is_resource($value) + ? $sf + : $value, + ); +}