[Amoifr]

Closes phpstan/phpstan#15366

PHP 8.6 adds clamp(mixed $value, mixed $min, mixed $max): mixed. php_math_clamp() returns a copy of one of its three arguments: $max when $max < $value, otherwise $min when $value < $min, otherwise $value.

The new extension rewrites the call into that ternary, with AlwaysRememberedExpr, the way MinMaxFunctionReturnTypeExtension handles min($a, $b) and max($a, $b). So clamp($int, 1, 10) is int<1, 10>, clamp($int, 1.0, 10.0) is 1.0|10.0|int<1, 10>, and constant arguments give a constant result. Arrays, which PHP compares by size first, and unpacked arguments keep the native mixed.

The type inference test needs clamp() at runtime, so it is gated with // lint >= 8.6.

[VincentLanglet]

i don't remember having such check in other dynamic extension. Is it really needed ? (or can't it be supported ?)

This doesn't seems to be tested

[staabm]

I guess its inspired by MinMaxFunctionReturnTypeExtension. agree it needs more tests

[Amoifr]

Good question, it can be supported. I checked every case against what PHP 8.6 actually returns: arrays compare by count and then element by element, and an array is greater than any scalar. PHPStan keeps those comparisons as bool, so the result is the union of the three arguments, which always contains the real value. Removed in bf9e838; int|array<int> now gives array<int>|int<1, 10> instead of mixed.

[staabm]

the above comment was about the if ($arg->unpack) { which means we need tests for using this function with splat operator.

I don't think we need precise types for clamp(...$x) now but at least a test which covers the current result would be good

[Amoifr]

Thanks for clarifying, and sorry @VincentLanglet, I answered about the wrong line. The check is needed: without it, the extension takes an unpacked array for the argument itself, so clamp(...[$i], ...[1], ...[10]) gives array{int} and clamp($i, 1, ...[10]) gives array{10}|int<1, max>. Added doSplat() in 8362760; it pins the current mixed result and fails without the check.

[staabm]

looking at this condition makes me feel we miss tests for values which are maybe an array, like int|array or something

[Amoifr]

The condition is gone (see the thread above), and int|array<int>, list<int> and constant arrays are now tested in doArrays().

[staabm]

I guess its inspired by MinMaxFunctionReturnTypeExtension. agree it needs more tests

[staabm]

why do we need AlwaysRememberedExpr. there is no test failling if we strip this to

		// same comparisons as php_math_clamp(): $max < $value ? $max : ($value < $min ? $min : $value)
		return $scope->getType(new Ternary(
			new Smaller($maxExpr, $valueExpr),
			$maxExpr,
			new Ternary(
				new Smaller($valueExpr, $minExpr),
				$minExpr,
				$valueExpr,
			),
		));

see cc57957 in which it was added to MinMax

[Amoifr]

It's needed for impure arguments, for the same reason as cc57957: without it, clamp(impureInt(), 1, 10) is int instead of int<1, 10>, because the call is evaluated again in each branch of the ternary. I added that case, and it fails without the wrapper.

[staabm]

please add tests and asserts for how the return type is inferered when min/max is

• a union of const integers
• a integer range

[Amoifr]

Added in doBounds(): unions of constant integers and integer ranges, for both bounds.

[staabm]

https://www.php.net/clamp contains a example with datetime objects. please also test that

[Amoifr]

Added the php.net example in doDates(), plus a mixed DateTime/DateTimeImmutable case.

[staabm]

Thank you 🙏