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

Reflection bugs #119

Closed
enumag opened this issue Jul 31, 2018 · 9 comments
Closed

Reflection bugs #119

enumag opened this issue Jul 31, 2018 · 9 comments

Comments

@enumag
Copy link

enumag commented Jul 31, 2018

Currently reflection on some Ds\* classes doesn't return the expected values which is causing issues with tools such as PHPStan.

Missing Ds\Pair properties

$reflection = new ReflectionClass(\Ds\Pair::class);

echo count($reflection->getProperties());

expected result: 2 (Pair has $key and $value properties)

actual result: 0

These properties are missing in the documentation as well (bug report here).

EDIT: Ds\Pair values are also invisible when debuging with Xdebug. This might be the cause.

Nullability of Map::first()

$reflection = new ReflectionClass(\Ds\Map::class);

echo $reflection->getMethod('first')->getReturnType()->allowsNull() ? 'true' : 'false';

expected result: false (the method throws an exception on empty map and otherwise returns a Pair)

actual result: true

EDIT: Map::last() and Map::skip() both have the same problem.

@enumag
Copy link
Author

enumag commented Aug 1, 2018

Another bug discovered:

Nullability of Map::values()

$reflection = new ReflectionClass(\Ds\Map::class);

echo $reflection->getMethod('values')->getReturnType()->allowsNull() ? 'true' : 'false';

expected result: false (return type is Ds\Sequence, not ?Ds\Sequence - see documentation)

actual result: true

@enumag
Copy link
Author

enumag commented Aug 23, 2018

Looks like this is just the tip of an iceberg, there are a lot more methods in this extension with incorrect reflection types. I have a tool to generate a functionMap for phpstan which I then fixed manually and I could use it to give you a more complete list what's wrong. I won't do it until I see some activity here since it would take me quite a bit of time and it's possible that you don't even need it.

@rtheunissen
Copy link
Member

@enumag are you able to use a master build? This should now be significantly better than 1.6, due to this change here: 21cbf36

@rtheunissen
Copy link
Member

I'll prepare an update regardless.

@enumag
Copy link
Author

enumag commented Sep 4, 2018

I installed the extension using apt-get so not really. Is there some easy way to use the master build?

Anyway aside from the errors I mentioned above there are actually a LOT of other similar cases in Ds reflection. Too many to list actually and I assume most of them will be fixed by the commit you linked anyway.

@rtheunissen
Copy link
Member

Yeah all the nullable return types should be fixed, but I haven't fixed the missing Pair properties yet. There's a quick-hack fix for it but I'd rather do it properly. They are accessed magically but should be defined properties.

@rtheunissen
Copy link
Member

I installed the extension using apt-get

I'm not sure who maintains this.. 🤔

@rtheunissen
Copy link
Member

What version are you using?

@rtheunissen
Copy link
Member

These should now all be fixed as at 1.2.7 🎉

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

No branches or pull requests

2 participants