-
Notifications
You must be signed in to change notification settings - Fork 88
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
Analyzing services with 'get' methods breaks following files #96
Comments
So I figured out what's the root problem and it has nothing to do with phpstan-symfony.
|
Both |
You are right, it does not seems to be related to the 😞 Altering architecture to accommodate tooling is not a very satisfying solution. I've found related issues, seems like it clash with infection too:
If there's no practical solution to the problem, maybe you should do what the creator of infection suggested:
|
Maybe you could do like phpunit and provide a hook. This way we could re-bind bypass final after each stream wrapper restoration. That's a bit of plumbing, but the other solutions:
... are not acceptable. PHPStan is a quality tool, if I have to write worse code to be able to use it, it defeat the purpose. |
Seems like the only thing you need to register stream wrappers is their FQCN, so you could just use a config parameter for that: parameters:
streamWrappers:
# FQCN of the stream wrappers you wan't the analyzer to use
# see https://www.php.net/manual/en/class.streamwrapper.php
file: 'DG\BypassFinals'
# phar: 'streamWrapper\FQCN' At bootstrap you register them: foreach ($parameters->streamWrappers as $protocol => $streamWrapperFQCN) {
spl_autoload_call($streamWrapperFQCN);
stream_wrapper_unregister($protocol);
stream_wrapper_register($protocol, $streamWrapperFQCN);
} In try {
// ...
} finally {
foreach ($streamWrapperProtocols as $protocol) {
if (array_key_exists($protocol, $parameters->streamWrappers)) {
stream_wrapper_unregister($protocol);
stream_wrapper_register($protocol, $parameters->streamWrappers[$protocol]);
} else {
stream_wrapper_restore($protocol);
}
}
} |
Bug report
Analyzing nested services which have
get
methods produces weird side effect on other files.It seems to break something container/autoloader related.
It then causes classes to not be loaded properly and not be processed by dg/bypass-finals.
And breaks (in this example) intersection types.
Having one service with a
get
method doesn't seems to be enough, I can only reproduce it with two.This is really hard to pinpoint because it is order dependent, so it may work on some machines because the analysis order is not deterministic. Also parallel processing makes that even more flaky.
The bug may seems to be really obscure edge case, but it's really not that hard to stumble on it: if you have a service/repository architecture (which symfony project usually have) you really likely to have nested
get
calls.Minimal reproducible setup
https://github.com/yannickroger/phpstan-troubleshoot
In the neon file if you swap the order of the two analyzed files you can trigger the error on and off.
Actual result
Depending on the analysis order you may have this error:
Expected result
Type should be resolved and the analysis shouldn't yield any error, whatever the analysis order is.
Related to
phpstan/phpstan#3179
Likely culprit
Considering it's really dependant on the
get
method name (renaming methods fix the bug) it is very likely related to this part of the code:https://github.com/phpstan/phpstan-symfony/blob/master/src/Type/Symfony/ServiceDynamicReturnTypeExtension.php#L41
The text was updated successfully, but these errors were encountered: