This specific distinction seems to be very important for Psalm, as `explode()` and
`lowercase-string` are used aggressively across the codebase.
Also, this change expands the baseline by a few entries, since some of the code locations
instide Psalm itself have un-checked list destructuring operations, as well as array
access calls on potentially undefined array keys produced by `explode()`, which were
previously just `list<string>`, and are now `array{0: string, 1?: string}`, which is
a bit more precise.
* in PHP 8.0, `ReflectionUnionType` is composed on `ReflectionNamedType`s
* in PHP 8.1, `ReflectionIntersectionType` is composed of `ReflectionNamedType`s
* in PHP 8.2, `ReflectionUnionType` is composed of `ReflectionIntersectionType|ReflectionNamedType`s
Slight variations for each PHP version.
As per local testing, this doesn't work yet.
## Local testing setup:
I did some digging to make sure that the stubs work as expected.
Here's what I did to validate this patch locally (since I don't think it can really be fully automated)
## Create a dummy file to verify used symbols
```php
<?php
namespace Testing;
/** @return \ReflectionClass<\stdClass> */
function getAClass(): \ReflectionClass { throw new \Exception('irrelevant'); }
function getAnUnionType(): \ReflectionUnionType { throw new \Exception('irrelevant'); }
function getAnIntersectionType(): \ReflectionIntersectionType { throw new \Exception('irrelevant'); }
// verifying that `getName()` is stubbed in all versions: this should always be a `class-string<\stdClass>`
$name = getAClass()->getName();
// union types should appear starting with PHP 8.0. Starting with PHP 8.2, they allow for intersections.
$unionTypes = getAnUnionType()->getTypes();
// intersection types should appear starting with PHP 8.1
$intersectionTypes = getAnIntersectionType()->getTypes();
$results = [$name, $unionTypes, $intersectionTypes];
/** @psalm-trace $results */ // tracing this will show us the differences between versions
return $results;
```
## Run the script against various `vimeo/psalm` versions
```sh
docker run --rm -ti -v $(pwd):/app -w /app php:7.4 ./psalm --php-version=7.4 --no-cache reflection-test.php | grep Trace
docker run --rm -ti -v $(pwd):/app -w /app php:8.0 ./psalm --php-version=8.0 --no-cache reflection-test.php | grep Trace
docker run --rm -ti -v $(pwd):/app -w /app php:8.1 ./psalm --php-version=8.1 --no-cache reflection-test.php | grep Trace
docker run --rm -ti -v $(pwd):/app -w /app php:8.2.0RC7-cli ./psalm --php-version=8.2 --no-cache reflection-test.php | grep Trace
```
## Evaluate output
```
❯ docker run --rm -ti -v $(pwd):/app -w /app php:7.4 ./psalm --php-version=7.4 --no-cache reflection-test.php | grep Trace
ERROR: Trace - reflection-test.php:20:1 - $results: list{class-string<stdClass>, mixed, mixed} (see https://psalm.dev/224)
❯ docker run --rm -ti -v $(pwd):/app -w /app php:8.0 ./psalm --php-version=8.0 --no-cache reflection-test.php | grep Trace
ERROR: Trace - reflection-test.php:20:1 - $results: list{class-string<stdClass>, non-empty-list<ReflectionNamedType>, mixed} (see https://psalm.dev/224)
❯ docker run --rm -ti -v $(pwd):/app -w /app php:8.1 ./psalm --php-version=8.1 --no-cache reflection-test.php | grep Trace
ERROR: Trace - reflection-test.php:20:1 - $results: list{class-string<stdClass>, non-empty-list<ReflectionNamedType>, non-empty-list<ReflectionNamedType>} (see https://psalm.dev/224)
psalm on feature/#8720-improve-types-and-purity-for-reflection-symbols [!?] via 🐘 v8.1.13 via ❄️ impure (nix-shell) took 4s
❯ docker run --rm -ti -v $(pwd):/app -w /app php:8.2.0RC7-cli ./psalm --php-version=8.2 --no-cache reflection-test.php | grep Trace
ERROR: Trace - reflection-test.php:20:1 - $results: list{class-string<stdClass>, non-empty-list<ReflectionNamedType>, non-empty-list<ReflectionNamedType>} (see https://psalm.dev/224)
```
These templates were leading to false positives: assuming
an `object` is given as input, the inferred return
type would always have been `true`, which is obviously
not valid.
Removing them is the healthier alternative, for now.
Ref: https://github.com/vimeo/psalm/pull/8722#discussion_r1027102713
As per @weirdan's feedback, we can prevent
the usage of `object` instances that
implement `__invoke()`, as well as `array`
callables, by declaring the ctor argument of
`ReflectionFunction` to be either a real `Closure`,
or a `callable-string`.
While this may not be 100% of scenarios, it is a
healthy way to identify errors in userland.
Ref: https://github.com/vimeo/psalm/pull/8722#discussion_r1027151421
Also:
* added PHP 8.2 stubs
* refined types to make impossible scenarios more clear (like `ReflectionIntersectionType#allowsNull()`)
This is a first attempt at refining these types: the structure of these stubs is quite confusing to me,
so I don't know if this approach is correct, and if the stubs are merged together, or if entire symbols
need to be completely re-declared for each PHP version.
Those functions should not return a string when they receive a
non-empty-string in input.
The following example is expected to work:
```php
<?php
/**
* @param non-empty-string $i
*/
function takesNonEmptyString(string $i): void {
echo $i;
}
takesNonEmptyString(bin2hex("a"));
takesNonEmptyString(base64_encode("a"));
```
While there is value in declaring `DateTimeImmutable::createFromInterface()` as mutation-free in
a stub, this method was introduced in PHP 8.0, so we cannot really declare it in a stub.
For now, we drop it, as the value of its stub declaration is much lower than the problems it
introduces through its conditional existence.
Also:
* a non-empty format string will always lead to `non-empty-string`
* it seems that you can throw **everything** at `DateTimeInterface#format()`, even null bytes,
and it will always produce a `string`
Also simplified the return type from `static|false` to `static`, since
the method throws at all times, on failure.
On PHP 7.x, it could only fail if an invalid type was passed in, which is
not really valid anyway, from a type perspective.
Ref (PHP 8.1.x): 32d55f7422/ext/date/php_date.c (L3353-L3369)
Ref (PHP 7.0.33): bf574c2b67/ext/date/php_date.c (L3596-L3612)
Also simplified the return type from `static|false` to `static`, since
the method throws at all times, on failure.
On PHP 7.x, it could only fail if an invalid type was passed in, which is
not really valid anyway, from a type perspective.
Ref (PHP 8.1.x): 32d55f7422/ext/date/php_date.c (L3308-L3324)
Ref (PHP 7.0.33): bf574c2b67/ext/date/php_date.c (L3549-L3565)
Also simplified the return type from `static|false` to `static`, since
the method throws at all times, on failure.
On PHP 7.x, it could only fail if an invalid type was passed in, which is
not really valid anyway, from a type perspective.
Ref (PHP 8.1.x): 32d55f7422/ext/date/php_date.c (L3258-L3274)
Ref (PHP 7.0.33): bf574c2b67/ext/date/php_date.c (L3496-L3512)
Also simplified the return type from `static|false` to `static`, since
the method throws at all times, on failure.
On PHP 7.x, it could only fail if an invalid type was passed in, which is
not really valid anyway, from a type perspective.
Ref (PHP 8.1.x): 32d55f7422/ext/date/php_date.c (L3212-L3228)
Ref (PHP 7.0.33): bf574c2b67/ext/date/php_date.c (L3447-L3463)
Also expanded the return type from `static` to `static|false`, since the
operation can fail (with a warning too), such as in following example:
https://3v4l.org/Xrjlc
```php
<?php
var_dump(
(new DateTimeImmutable())
->modify('potato')
);
```
Produces
```
Warning: DateTimeImmutable::modify(): Failed to parse time string (potato) at position 0 (p): The timezone could not be found in the database in /in/Xrjlc on line 6
bool(false)
```
Ref: 534127d3b2/ext/date/php_date.stub.php (L508-L509)
Ref: c205d652d1 (r934032422)
The idea is that `@psalm-pure` disallows `$this` usage in child classes,
which is not wanted, while `@psalm-mutation-free` allows it.
By using `@psalm-mutation-free`, we don't completely destroy inheritance
use-cases based on internal (immutable) state.
`DateTimeImmutable` is **almost** immutable: `DateTimeImmutable::__construct()` is in fact not a pure
method, since `new DateTimeImmutable('now')` produces a different value at each instantiation (by design).
This change makes sure that `DateTimeImmutable` loses its `@psalm-immutable` class-level marker,
preventing downstream misuse of the constructor inside otherwise referentially transparent code.
Note: only pure methods are stubbed here: all other methods declared by `DateTimeImmutable` (parent interface)
are NOT present here, and are either inferred from runtime reflection, or `CallMap*.php` definitions.
Methods are sorted in the order defined by reflection on PHP 8.1.8, at the time of writing this ( https://3v4l.org/3TGg8 ).
Following simplistic snippet was used to infer the current signature:
```php
<?php
$c = new \ReflectionClass(\DateTimeImmutable::class);
$methods = array_map(function ($m) {
return $m->getName()
. '(' . implode(',', array_map(function ($p) {
return $p->getType()
. ' $' . $p->getName()
. ($p->isOptional() ? ' = ' . var_export($p->getDefaultValue(), true) : '');
}, $m->getParameters())) . ')' . ($m->getReturnType() ? (': ' . $m->getReturnType()) : '');
}, $c->getMethods());
$properties = array_map(function ($m) {
return $m->getName();
}, $c->getProperties());
var_dump($methods, $properties);
```
