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

PHPStan unresolvable type for intersection of final and MockObject #8901

Closed
zeleznypa opened this issue Feb 16, 2023 · 19 comments
Closed

PHPStan unresolvable type for intersection of final and MockObject #8901

zeleznypa opened this issue Feb 16, 2023 · 19 comments

Comments

@zeleznypa
Copy link

Bug report

Because somebody use final in the third-party code that I need to use in my own and need to test integration with it,
I have to use the dg/bypass-finals which allows me to use the PhpUnit.

Unfortunately the PHPStan detect the problem of using „not so final class“ together with MockObject and returns the following report

Method Test::createFinalClassMock() has unresolvable native return type.                         
PHPDoc tag @return contains unresolvable type.                                                                              
Return type of call to method PHPUnit\Framework\TestCase::createPartialMock() contains unresolvable type.                   
Method Test:: createFinalClassMock() with return type void returns *NEVER* but should not return  anything. 

that is not true, because It works.

Code snippet that reproduces the problem

    /**
     * Link mock factory
     *
     * @param string[] $methods [OPTIONAL] List of mocked method names
     * @return FinalClass&MockObject
     */
    protected function createFinalClassMock(array $methods = []): FinalClass&MockObject
    {
        $mock = $this->createPartialMock(FinalClass::class, $methods);
        return $mock;
    }

Expected output

The issue was reported incorrectly

Did PHPStan help you today? Did it make you happy in any way?

Many times the PHPStan helps me. Thanks much.

@mergeable
Copy link

mergeable bot commented Feb 16, 2023

This bug report is missing a link to reproduction at phpstan.org/try.

It will most likely be closed after manual review.

@zeleznypa
Copy link
Author

I have found many similar looking tickets, where was written:

use bootstrap or autoload where I can run the DG\BypassFinals::enable();

but it not works.

@herndlm
Copy link
Contributor

herndlm commented Feb 16, 2023

The bypass finals lib removes final when the autoloader loads the class AFAIK. Which is almost borderline magic/crazy IMO :) phpstan can't know that when it uses reflection.

Is there no way to use a real instance of the final class I suppose? Or does it implement an interface you could use and mock instead?

@zeleznypa
Copy link
Author

zeleznypa commented Feb 16, 2023

I have investigated, that issue is there, when I tried to access class that not exist. For example when I add use Foo in the class definition.

After that, the PhpStan can't identify, that class Foo not exists:

https://phpstan.org/blog/solving-phpstan-error-unable-to-resolve-template-type

but using DG\BypassFinals::enable(); in bootstrap stop work.

@zeleznypa
Copy link
Author

@herndlm Until now, I have found many ugly way, how to workaround existence of final that sometimes lead to sacrifice the quality of code, but I still had hope, that when people using final there also use PhpStan and have some way, how to test the code properly (Mock -> UnitTests)...

@zeleznypa
Copy link
Author

I have also investigated, that this issue happen, when I have the folder (that contains the final class) in the excludePaths

like:

    excludePaths:
        - */.git/*
        - */log/*
        - */node_modules/*
        - */temp/*
        - */vendor/*

But when i put the folder in the analyse subsection, the error disapeer (when there are no other reason)

    excludePaths:
        analyseAndScan:
            - */.git/*
            - */log/*
            - */node_modules/*
            - */temp/*
        analyse:
            - */vendor/*

@herndlm
Copy link
Contributor

herndlm commented Feb 17, 2023

I'm not sure any more if I fully understood the issue tbh, but I would have also suggested to try out
DG\BypassFinals::enable();
in phpstan's bootstrap file next if you can't get rid of the final mock.

I also found #3179 now, which you found too I guess. So it sounds as the problem could be an autoloader race condition. E.g. something else might still be loading the final class earlier before bypass-finals is registered via phpstan (bootstrap or config). It might be hard to do, but a minimal reproducer would be really useful I guess.

@zeleznypa
Copy link
Author

zeleznypa commented Feb 17, 2023

@herndlm the issue is based on DG\BypassFinals::enable() not work, when there are some not so relevant cases :(

I really like to use it...

Yes, the issue is based on the topic, that class was loaded before the DG\BypassFinals::enable(); was called. But the question is by what?

I have found at least 2 strange cases:

Now I'm facing another strange issue, but I can't reproduce it in another repository to make it public.

When I add not used phpunit dataset trait it works well, but:

How can number of array items have impact to loading te autoloader???

@ondrejmirtes
Copy link
Member

I have also investigated, that this issue happen, when I have the folder (that contains the final class) in the excludePaths

This doesn't make sense. You put things into excludePaths when you don't want PHPStan to see the things in these paths at all. So the expected behaviour would be something like "Class FinalClass not found."

The root issue of the behaviour you're seeing is because both https://github.com/dg/bypass-finals and PHPStan's https://github.com/phpstan/phpstan-src/blob/1.10.x/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php use the same hacky technique of overriding default file:// stream wrapper to achieve their own goals. And both stream wrappers overrides cannot be active at the same time. It's basically this #3854.

This code https://github.com/phpstan/phpstan-src/blob/1.10.x/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php and this documentation page https://phpstan.org/user-guide/discovering-symbols are closely related.

Once searching for a symbol reaches AutoloadSourceLocator here (https://github.com/phpstan/phpstan-src/blob/28565ffe784f4bb244f0aa63ada2ecd03624df62/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php#L132), the game is over for bypass-finals.

This explains all the behaviour you're seeing.

@zeleznypa
Copy link
Author

zeleznypa commented Feb 17, 2023

@ondrejmirtes And will it be possible to have such a feature in PHPStan like "Ignore finals"?

Or if I understand the core of the problem well, when the bypass-finals is registered and I force him to autoload the class before the AutoloadSourceLocator, it will works?

@ondrejmirtes
Copy link
Member

I think you're much better served by refactoring your code in the way I described here: https://twitter.com/OndrejMirtes/status/1622311556531867651

You can also check other replies in the thread: https://twitter.com/matthieunapoli/status/1622258374837420032

Or if I understand the core of the problem well, when the bypass-finals is registered and I force him to autoload the class before the AutoloadSourceLocator, it will works?

Yeah, probably :)

@zeleznypa
Copy link
Author

@ondrejmirtes Many thanks four your patience with this topic

@zeleznypa
Copy link
Author

@ondrejmirtes Unfortunately it does not work. The builtin PHP reflection was fixed and return that the class is not final, but PHPStan still thinks that it is final :(

zeleznypa/phpstan-final-intersection@b676642

@dg
Copy link
Contributor

dg commented Feb 18, 2023

The root issue of the behaviour you're seeing is because both https://github.com/dg/bypass-finals and PHPStan's https://github.com/phpstan/phpstan-src/blob/1.10.x/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php use the same hacky technique of overriding default file:// stream wrapper to achieve their own goals. And both stream wrappers overrides cannot be active at the same time. It's basically this #3854.

Well, actually, they might, I solved it somehow because of the collaboration with Infection dg/bypass-finals@8151a24

@ondrejmirtes
Copy link
Member

But what if mine gets called first? If I implement the same fix then we end in infinite recursion.

@dg
Copy link
Contributor

dg commented Feb 22, 2023

It shouldn't infinite loop, should it? Simply each handler, when initialized, remembers the previous one and calls it.

@ondrejmirtes
Copy link
Member

I'm not really sure 🤷‍♂️

@zeleznypa
Copy link
Author

@ondrejmirtes, @dg to prevent the possible issue with infinite loop can help maybe this: https://gitlab.com/interitty/infinite-loop-prevention

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants