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

psalm doesn't recognize some grpc classes & constants #6796

Closed
nocive opened this issue Nov 2, 2021 · 8 comments
Closed

psalm doesn't recognize some grpc classes & constants #6796

nocive opened this issue Nov 2, 2021 · 8 comments
Labels

Comments

@nocive
Copy link

nocive commented Nov 2, 2021

psalm seems to have some trouble recognizing some grpc classes and constants.

$ php --version
PHP 8.0.12 (cli) (built: Oct 21 2021 16:45:17) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.12, Copyright (c) Zend Technologies
    with Zend OPcache v8.0.12, Copyright (c), by Zend Technologies

$ php -i | grep grpc
/usr/local/etc/php/conf.d/docker-php-ext-grpc.ini,
grpc
grpc support => enabled
grpc module version => 1.41.0
grpc.enable_fork_support => 0 => 0
grpc.grpc_trace => no value => no value
grpc.grpc_verbosity => no value => no value
grpc.log_filename => no value => no value
grpc.poll_strategy => no value => no value

$ psalm --version
Psalm 4.x-dev@

Despite having the extension enabled, the following code fails https://psalm.dev/r/60285c8421.
Additionally, all the constants from the Grpc namespace are also not recognized, eg: https://psalm.dev/r/1a904c1046.

I noticed that there are some stubs defined in the dictionaries/CallMap.php and also that some internal psalm code disables the grpc extension under some circumstances, so I'm wondering if that could have anything to do with it.

Any help or hints would be appreciated. Thanks!

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/60285c8421
<?php

use Grpc\ChannelCredentials;

$c = ChannelCredentials::createInsecure();
Psalm output (using commit 96ae8e7):

ERROR: UndefinedClass - 5:6 - Class, interface or enum named Grpc\ChannelCredentials does not exist
https://psalm.dev/r/608770e40d
<?php

use Grpc\STATUS_OK as GRPC_STATUS_OK;

echo GRPC_STATUS_OK;
Psalm output (using commit 96ae8e7):

ERROR: UndefinedConstant - 5:6 - Const GRPC_STATUS_OK is not defined

INFO: MixedArgument - 5:6 - Argument 1 of echo cannot be mixed, expecting string

@weirdan
Copy link
Collaborator

weirdan commented Nov 2, 2021

Does it work with --threads=1?

@nocive
Copy link
Author

nocive commented Nov 2, 2021

Does it work with --threads=1?

@weirdan yes!

I guess it's indeed because of https://github.com/vimeo/psalm/blob/master/src/Psalm/Internal/Cli/Psalm.php#L895?
I can easily force --threads=1 for my runs, to work around the issue for now, thanks!

Out of curiosity, what's the background here for having to disable the extension if threads > 1?

@weirdan
Copy link
Collaborator

weirdan commented Nov 2, 2021

I'm not sure. The commit is here: 73aa0df, but it doesn't reference any issue and does not provide any explanation. Not sure if @muglug remembers that either.

@orklah
Copy link
Collaborator

orklah commented Nov 2, 2021

I'm starting to wonder if we should really require having enabled extensions in order to use the signatures in callmaps. There's a few other issues that are related where people are suprised the signature they documented are not used because they lack the extension. Here it's even worse because we disable the extension ourselves

@muglug
Copy link
Collaborator

muglug commented Nov 2, 2021

Sorry for no documentation — I added this code because of this bug (the same was true I think of the APC extension). Since that issue has been fixed for a few years, and users can selectively disable extensions anyway, it should be good to remove this exception.

@nocive
Copy link
Author

nocive commented Nov 2, 2021

Happy to provide a PR if necessary 🖖

@weirdan
Copy link
Collaborator

weirdan commented Nov 2, 2021

From the discussion on the linked issue:

From the command line, you have to set two environment variables to enable fork support, like this:

$ GRPC_ENABLE_FORK_SUPPORT=1 GRPC_POLL_STRATEGY=epoll1 php -d extension=grpc.so test.php

So I suspect we would still have to have some special treatment for grpc ext, e.g. check if the environment var is set and warn + downgrade to single thread otherwise.

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

No branches or pull requests

4 participants