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

Use generics #165

Merged
merged 22 commits into from
May 8, 2020
Merged

Use generics #165

merged 22 commits into from
May 8, 2020

Conversation

Jean85
Copy link
Contributor

@Jean85 Jean85 commented Apr 28, 2020

This is a first stab at fixing #121
Fixes #121

It tries to leverage the new generics support from PHPStan to obtain a cleaner way to detect Prophecy's mocks, and allow the ObjectProphecy<OriginalClass> PHPDoc notation.

This is still WIP: I've tried to follow @ondrejmirtes suggestiong from #121 (comment), but I don't know how to proceed to make PHPStan recognize the calls of the original class on the ObjectPropecy.

All green, ready for you to review @localheinz!

extension.neon Outdated Show resolved Hide resolved
src/Prophecy.phpstub Outdated Show resolved Hide resolved
@Jean85
Copy link
Contributor Author

Jean85 commented Apr 28, 2020

@localheinz WDYT? You're welcome to take this and put it forward.

src/Prophecy.phpstub Outdated Show resolved Hide resolved
src/Prophecy.phpstub Outdated Show resolved Hide resolved
src/Prophecy.phpstub Outdated Show resolved Hide resolved
src/Prophecy.phpstub Outdated Show resolved Hide resolved
@localheinz
Copy link
Contributor

@Jean85 @ondrejmirtes

As you have seen in my attempt to make use of genetics in #155, I have no idea how to sort this out.

I’ll happily take a look, though, when you have.

@Jean85
Copy link
Contributor Author

Jean85 commented May 4, 2020

@localheinz I've looked now at yours PR, and we have many parts in common. I didn't split the stub per-class, I'll try that because I have some autoload errors right now, it might help.

@ondrejmirtes I've (partially) followed your suggestions, and I've restored a big part of the previous codebase, except from ObjectProphecy (which is replaced with a generic) and RevealDynamicReturnTypeExtension (since its job of intersecting mocked types is now done upfront by the generic plus the WillExtendOrImplementDynamicReturnTypeExtension).

Almost all seems fixed, the only thing that I need to fix now is the return type inferred on ObjectProphecy calls; it sees the return type of the original method... MethodsClassReflectionExtension doesn't seem to work! Any tips?

@ondrejmirtes
Copy link
Contributor

Dump what's going into the hasMethod and getMethod, I guess that hasMethod returns false.

@Jean85
Copy link
Contributor Author

Jean85 commented May 5, 2020

Thanks @ondrejmirtes, that led me in the right direction!

CI was crapping out on 7.4 + --prefer-lowest. Now it's all green!! 🎉

@Jean85 Jean85 marked this pull request as ready for review May 5, 2020 06:49
@localheinz
Copy link
Contributor

@ondrejmirtes

Can you take a look and let me know if this makes sense, please?

@localheinz localheinz self-requested a review May 8, 2020 14:07
@localheinz localheinz self-assigned this May 8, 2020
@localheinz localheinz merged commit 5f8e53c into Jan0707:master May 8, 2020
@localheinz
Copy link
Contributor

Thank you, @Jean85 and @ondrejmirtes!

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

Successfully merging this pull request may close these issues.

Does not recognize prophecies from wrapper methods
3 participants