Conversation
An invalid step makes `range()` throw a ValueError, so a call where all
constant argument combinations throw is now `*NEVER*` instead of an array
type. The constant arguments are folded by calling `range()` on the PHP
version PHPStan itself runs on, so a caught ValueError is only turned into
`never` when the analysed PHP version is known to reject the step as well:
a step of 0 or one wider than the range has been a ValueError since PHP 8.0,
while a negative step on an increasing range and a non-finite step only
became one in PHP 8.3.
Ranges longer than `RANGE_LENGTH_THRESHOLD` are generalized from the values
the range consists of instead of from the argument types, so `range('A', 'z')`
is a list of single byte strings and `range(1, 200, 1.0)` is a list of ints -
the latter only since PHP 8.3, where an integral float step stopped producing
floats.
Ref phpstan/phpstan#10022
| assertType('*NEVER*', range(5, 6, 3)); | ||
| assertType('*NEVER*', range(2, 5, -1)); | ||
| assertType('*NEVER*', range('a', 'z', -1)); | ||
| assertType('*NEVER*', range(1, 10, INF)); |
There was a problem hiding this comment.
we need a test that range(1, 10, INF) will be false not *NEVER* on PHP7
There was a problem hiding this comment.
Added in 4f4f737. PHP 7 reported an invalid step with a warning and a false return value, so on PHP 7 the extension now returns false for such a call instead of an array type. RangePhp74Test runs with phpVersion: 70400 and covers range(1, 10, INF), range(2, 5, 0), range(5, 6, 3), and array{6}|false for a union where only one combination fails. PHP 7.4, 8.0–8.2 and 8.3+ all reject these steps, so the test runs on every CI runtime without a version gate.
5b68933 fixes the failing CI jobs: PHPUnit 11 treats an empty data provider as an error, so RangePhp82Test now skips through #[RequiresPhp('>= 8.3.0')].
…ider PHPUnit 11 reports an empty data set as an error.
Before PHP 8.0 an invalid step was reported by a warning and a false return value instead of a ValueError.
|
This pull request has been marked as ready for review. |
| // PHP 7 reports an invalid step by returning false instead of throwing a ValueError | ||
| if ($rangeValues === false) { | ||
| if ($this->failsOnAnalysedVersion($startConstant->getValue(), $endConstant->getValue(), $stepConstant->getValue())) { | ||
| if ($this->phpVersion->throwsValueErrorForInternalFunctions()) { |
There was a problem hiding this comment.
if you use $scope->getPhpVersion() in the extension instead of the DI wide PhpVersion, type inference gets more precise and also will support narrowing via conditions like if (PHP_VERSION_ID < 80000) { .
this means you can put your tests into the nsrt/ folder and describe php-version dependent types like
if (PHP_VERSION_ID < 80000) {
// PHP 7 reports an invalid step with a warning and returns false instead of throwing a ValueError
assertType('false', range(1, 10, INF));
assertType('false', range(2, 5, 0));
...
}
without the need for adding a class RangePhp74Test extends TypeInferenceTestCase and the corresponding NEON will also be obsolete -> less files to fiddle with
There was a problem hiding this comment.
Done in 596668a. The extension reads $scope->getPhpVersion() now. I added throwsValueErrorForInternalFunctions() and hasStricterRangeFunction() to PhpVersions and dropped the PhpVersion method this PR had added. I moved the asserts to nsrt/range-invalid-step.php and nsrt/bug-10022.php, split by PHP_VERSION_ID, so RangePhp74Test, RangePhp82Test and the NEON file are gone.
With a version range, the runtime can reject a step that some analysed version accepts. The extension falls back to the general type in that case. Before, range($flag ? 2 : 5, 5, -1) analysed for 8.2 gave array{5} and missed [2, 3, 4, 5].
`Scope::getPhpVersion()` follows conditions like `PHP_VERSION_ID < 80000`, so the tests move into `nsrt/` and no longer need separate test classes with a fixed `phpVersion`. When a combination fails at runtime but the analysed PHP versions might accept its step, the other constant combinations are not returned on their own anymore.
|
The Mutation Testing jobs fail on |
|
This pull request has been marked as ready for review. |
3 participants
Ref phpstan/phpstan#10022
Two inaccuracies in the return type of
range():$stepmakesrange()throw aValueError, but the extension still returned an array type. It now returnsneverwhen every combination of the constant arguments throws.RANGE_LENGTH_THRESHOLDthe extension generalized the argument types, which predates the PHP 8.3 changes. It now generalizes the values the range consists of:// range('A', 'z') -non-empty-list<int|(literal-string&lowercase-string&non-falsy-string)|(literal-string&non-falsy-string&uppercase-string)> +non-empty-list<literal-string&non-empty-string> // range(1, 200, 1.0) -non-empty-list<float> +non-empty-list<int<1, 200>>The extension folds the constant arguments by calling the native
range(), so a caughtValueErroronly says that the runtime rejects the step. I gated both changes onPhpVersionwherever PHP 8.3 changed the behaviour. A$stepof0, or one wider than the range, has been aValueErrorsince PHP 8.0, while a negative$stepon an increasing range and a non-finite$steponly became one in 8.3. An integral float$stepproduces ints since 8.3 and floats before that. Analysing for PHP 8.2 still givesnon-empty-list<int>forrange(2, 5, -1)andnon-empty-list<float>forrange(1, 200, 1.0).RangePhp82Testcovers that axis: it setsphpVersionto 8.2 and runs on 8.3+.This does not close phpstan/phpstan#10022. PHPStan already infers the PHP 8.3 result for the reproducer in that issue,
range('1', 'a'), because the extension folds the constant arguments through the nativerange(). For the same reason the folded values follow the PHP version PHPStan runs on rather than the configuredphpVersion: withphpVersion: 80200on a PHPStan running on 8.5,range('1', 'a')still comes out as the 49 element character range instead ofarray{1, 0}. Making the values followphpVersionmeans reimplementingrange()down to the rounding ofstart + i * step, and I did not want to put that in this PR. This PR fixes what you can decide without knowing the values: which steps PHP rejects, and how to generalize a range past the threshold.