[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

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
Assignees
@kocsismate
Labels
Bug Extension: reflection Status: Verified Stubs

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 •
edited
Loading

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 .

kocsismate added a commit to kocsismate/php-src that referenced this issue Jan 22, 2023
@kocsismate
Fix phpGH-10259 ReflectionClass::getStaticProperties doesn't need nul…
119214f 
…l return type
@kocsismate kocsismate linked a pull request Jan 22, 2023 that will close this issue
Fix GH-10259 ReflectionClass::getStaticProperties doesn't need null return type #10418
Merged
kocsismate added a commit that referenced this issue Jan 24, 2023
@kocsismate
Fix GH-10259 ReflectionClass::getStaticProperties doesn't need null r…
0161608 
…eturn type (#10418)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kocsismate kocsismate

Labels
Bug Extension: reflection Status: Verified Stubs
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix GH-10259 ReflectionClass::getStaticProperties doesn't need null return type kocsismate/php-src
5 participants
@cmb69 @kocsismate @nielsdos @Girgias @othercorey