Skip to content

Add class_implements return type extension #1756

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

Closed
wants to merge 1 commit into from

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Sep 25, 2022

Refs: phpstan/phpstan#8059
Closes phpstan/phpstan#4335

See also https://3v4l.org/IufYX

If this is OK, I can do the same for the other related functions either here or in dedicated PRs.

@herndlm herndlm force-pushed the class-implements branch 2 times, most recently from e1d0082 to 2ac50a4 Compare September 25, 2022 20:04
@herndlm herndlm marked this pull request as ready for review September 25, 2022 20:13
@herndlm
Copy link
Contributor Author

herndlm commented Sep 25, 2022

The one nextras failure should be fine. They do a class_exists before which makes the string a class-string and therefore false will never be returned: https://github.com/nextras/orm/blob/v4.0.6/src/Entity/Reflection/MetadataParser.php#L421

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is worth it. You can say these things for sure only for final classes, otherwise the array isn't really complete - a subclass can implement more interfaces.

@herndlm
Copy link
Contributor Author

herndlm commented Sep 26, 2022

I don't think this is worth it. You can say these things for sure only for final classes, otherwise the array isn't really complete - a subclass can implement more interfaces.

It should work fine with subclasses as well, why wouldn't it? Or do you mean something different?

@herndlm
Copy link
Contributor Author

herndlm commented Sep 26, 2022

aaah I think I got you now, makes sense

I'll have to find another fix for the webmozart assert problem then :)

@herndlm herndlm closed this Sep 26, 2022
@herndlm herndlm deleted the class-implements branch September 26, 2022 08:51
@VincentLanglet
Copy link
Contributor

A return type extension is still useful for phpstan/phpstan#4335 @herndlm
Instead of looking for the exact values returned by class_implements just knowing if the return value is class-string[] or class-string[]|false would be great.

Do you want to reopen the PR or should I work on it ? :)

@herndlm
Copy link
Contributor Author

herndlm commented Nov 21, 2022

You can go ahead, I fixed my problem that I was having in a different way :)

@VincentLanglet
Copy link
Contributor

You can go ahead, I fixed my problem that I was having in a different way :)

I started #2023 then if you have time to take a look.
You recently introduce a lot of new method in the Type interface, and I may miss something but didn't find the one I needed and had to use instanceof.

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

Successfully merging this pull request may close these issues.

class_implements can return false only under some circumstances
3 participants