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

Add ibm_db2 constant stubs #5477

Closed
wants to merge 1 commit into from
Closed

Add ibm_db2 constant stubs #5477

wants to merge 1 commit into from

Conversation

morozov
Copy link
Contributor

@morozov morozov commented Mar 25, 2021

Closes #5466.

@@ -1734,6 +1734,16 @@ public function visitStubFiles(Codebase $codebase, ?Progress $progress = null):
$core_generic_files[] = $stringable_path;
}

if (!\extension_loaded('ibm_db2')) {
Copy link
Contributor Author

@morozov morozov Mar 25, 2021

Choose a reason for hiding this comment

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

I don't really understand why the rest of the conditions look like if (extension_loaded(/* ... */)). As I understand, one of the purposes of the stubs is to enable the static analysis of the code that depends on an extension that is unavailable at runtime.

On one hand, loading the stub only when the extension is loaded doesn't help to solve the issue. On the other hand, the issue is only reproducible when the extension is not loaded.

Copy link
Collaborator

Choose a reason for hiding this comment

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

one of the purposes of the stubs is to enable the static analysis of the code that depends on an extension that is unavailable at runtime.

Oh, in that case you should instead provide your own stubs. If Psalm were to stub all classes it might miss situations where you're missing an extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I don't understand the architecture of stubs in Psalm. There are more or less complete stubs from JetBrains but they are released quite rarely and I want to try to get rid of them given that Psalm and PHPStan provide their own. Having to have 3 sources of stubs, each of which may have its issues is quite a burden.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Psalm needs them to provide more detailed types that are unavailable through reflection. I think you can still force-load them in your psalm.xml, e.g.:

<stubs>
   <file name="vendor/vimeo/psalm/stubs/ext-ibm-db2.phpstub"/>
</stubs>

Copy link
Contributor Author

@morozov morozov Mar 25, 2021

Choose a reason for hiding this comment

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

Psalm needs them to provide more detailed types that are unavailable through reflection.

Right, but Psalm also uses the call maps that provide some basic information on the extension API. As a result, it can analyze most of the code that uses the extension (e.g. calls the functions) but not all of it (the usage of constants). From the end-user standpoint, this doesn't look right.

I think you can still force-load them in your psalm.xml

Thanks, this should work. Currently, Doctrine DBAL loads PhpStorm stubs into Psalm unconditionally w/o any issues.

@@ -1734,6 +1734,16 @@ public function visitStubFiles(Codebase $codebase, ?Progress $progress = null):
$core_generic_files[] = $stringable_path;
}

if (!\extension_loaded('ibm_db2')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!\extension_loaded('ibm_db2')) {
if (\extension_loaded('ibm_db2')) {

@morozov morozov closed this Apr 26, 2021
@morozov morozov deleted the issues/5466 branch April 26, 2021 19:52
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.

IBM_DB2 constants are not stubbed
3 participants