Skip to content
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

ReflectionClass::getStaticProperties doesn't need null return type #10259

Closed
othercorey opened this issue Jan 8, 2023 · 4 comments · Fixed by #10418
Closed

ReflectionClass::getStaticProperties doesn't need null return type #10259

othercorey opened this issue Jan 8, 2023 · 4 comments · Fixed by #10418

Comments

@othercorey
Copy link
Contributor

Description

The following code:

<?php
$r = new ReflectionMethod('ReflectionClass::getStaticProperties');
echo (string)$r->getTentativeReturnType();

Resulted in this output:

?array

But I expected this output instead:

array

Looks like the return paths were changed to RETURN_THROWS() in 8664ff7 and 8176059 but the stub wasn't updated.

Usually, after these cleanups, the stub is correct but the docs are outdated, however this seems to be the wrong stub.

I'm not sure how to update the stub and generated code from it.

PHP Version

8.1.14

Operating System

No response

@nielsdos
Copy link
Member

nielsdos commented Jan 8, 2023

I'm not sure how to update the stub and generated code from it.

You can edit the stub file ext/reflection/php_reflection.stub.php. After you edited it you should run make (*) and it will automatically update the generated files for you. Upon creating a PR include both the generated files and the stub file.

(*) Assuming you already ran ./buildconf and ./configure.

@Girgias
Copy link
Member

Girgias commented Jan 8, 2023

I'm not sure how to update the stub and generated code from it.

You can edit the stub file ext/reflection/php_reflection.stub.php. After you edited it you should run make (*) and it will automatically update the generated files for you. Upon creating a PR include both the generated files and the stub file.

(*) Assuming you already ran ./buildconf and ./configure.

No need to run make, just runing ./build/gen_stubs.php should do the trick.

@Girgias Girgias added the Stubs label Jan 8, 2023
@kocsismate kocsismate self-assigned this Jan 11, 2023
@othercorey
Copy link
Contributor Author

othercorey commented Jan 15, 2023

I could be wrong, but looks like ReflectionFunctionAbstract::getClosureScopeClass() and getClosureThis() no longer return null also.

This issue was assigned to someone, so I'm not working on a PR.

@kocsismate
Copy link
Member

Thanks for the report! Your initial claim is true, ReflectionClass:getStaticProperties indeed cannot return true, so I'll fix the return type in master.

However, ReflectionFunctionAbstract::getClosureScopeClass() can return null, there is even a test for this in php-src. A short excerpt: https://3v4l.org/hV7MZ#v8.2.1 The same is true for getClosureThis .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants