New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Declaring more precise types and purity boundaries on ext-reflection
symbols in .phpstub
files
#8722
Declaring more precise types and purity boundaries on ext-reflection
symbols in .phpstub
files
#8722
Commits on Dec 6, 2022
-
Declaring more precise types and purity boundaries on `ext-reflection…
…` symbols in `.phpstub` files 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.
-
Marking
ReflectionProperty#$name
asstring
rather than `non-empty……-string` Because @weirdan is a party pooper (they poop at the parties) Ref: https://www.youtube.com/watch?v=gjwofYhUJEM Ref: vimeo#8722 (comment)
-
Prevent usage of callable objects in
ReflectionFunction::__construct()
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: vimeo#8722 (comment)
-
Removed templated parameters from
ReflectionClass#isInstance()
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: vimeo#8722 (comment)
-
Refine
ReflectionUnionType
andReflectionIntersectionType
for PHP…… 8.1 and PHP 8.2 * 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/vimeo#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) ```
Commits on Dec 7, 2022
-
Mark
Reflection(Method|Property)#setAccessible()
as pure starting f……rom PHP 8.1 onwards This will highlight unused code. Ref: php/php-src#5412 Ref: https://wiki.php.net/rfc/make-reflection-setaccessible-no-op Ref: php/php-src#5411 Example https://3v4l.org/PNeeZ ```php <?php class Foo { private $bar = 'baz'; private function taz() { return 'waz'; } } //var_dump((new ReflectionProperty(Foo::class, 'bar'))->getValue(new Foo)); //var_dump((new ReflectionMethod(Foo::class, 'taz'))->invoke(new Foo)); ``` Produces (starting from PHP 8.1): ``` string(3) "baz" string(3) "waz" ```
-
Corrected
AttributeTest
expectation:ReflectionAttribute
s always ……come in a `list`
-
Always load preloaded stub files when the analysis version is compatible
Before this change, preloaded stubs would only be loaded when running on a PHP version lower than the one that is in the stubs. With this change, the analysis PHP version (defined via config, input parameter, or inferred from the runtime) becomes authoritative. Since the PHP-version-specific stubs are not just polyfills, but actually type refinements on top of the PHP core symbols at hand, this change always loads them, so that it is possible to get more precise type inference downstream. This will likely lead to downstream breakages, because the stubs do indeed provide better type resolution, but indeed formalizes the idea that these stubs will provide better value for finding problems in analyzed code.
Commits on Dec 8, 2022
-
Load PHP-version-specific stubs based on analysis PHP version, and on…
…ly when visiting stub files While `visitPreloadedStubFiles` seemed to work at first, it led to overriding symbols declared by PHP itself too eagerly. By only loading PHP-version-specific stubs in `visitStubFiles`, we ensure that the reflection-declared symbols take priority, and that our stubs overlay on top of that, without actually replacing the symbol entirely, but rather merging with its definition. This fixes current test failures too, and works with the code examples from vimeo#8722 (comment)