PDOStatement::fetchObject(class-string) return type not recognized in 5.0a1

[ghost]

https://psalm.dev/r/d78bf0c48c gives

INFO: [LessSpecificReturnStatement](https://psalm.dev/129) - 10:12 - The type 'false|object' is more general than the declared return type 'Test1|false' for test

INFO: [MoreSpecificReturnType](https://psalm.dev/070) - 5:18 - The declared return type 'Test1|false' for test is more specific than the inferred return type 'false|object'

Previous versions did not give the INFO here.

Psalm 5.0.0-alpha1, PHP 8.0.15.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/d78bf0c48c

<?php

class Test1 {}

function test(): Test1|false {
    $db = new PDO('');
    $result = $db->query('select * from test');
    $test = $result->fetchObject(Test1::class);
    
    return $test;
}

Psalm output (using commit cfce264):

INFO: LessSpecificReturnStatement - 10:12 - The type 'false|object' is more general than the declared return type 'Test1|false' for test

INFO: MoreSpecificReturnType - 5:18 - The declared return type 'Test1|false' for test is more specific than the inferred return type 'false|object'

[orklah]

@phptest2 Can you check if you have this line in your require section of composer.json: "ext-pdo": "*",?

Psalm changed the way it loads definitions for extensions. It now require to be either in your composer.json or in your xml config here: https://psalm.dev/docs/running_psalm/configuration/#enableextensions

[weirdan]

Speaking of which, we should probably enable all extensions on psalm.dev.

[AndrolGenhald]

I definitely think we need to document this somewhere, but I'm not sure where. It would be amazing to give a warning when running Psalm, but that would require detecting when a statement matches something in the extension stub, and that seems far too difficult. @phptest2 Did you check any documentation before opening the issue? Where would you have expected a note about this?

[orklah]

It could be great to display something like
Target PHP version: 8.1 (set by CLI argument) Extensions detected: ...

I don't want to clutter the output, but I think we're going to see a lot of these issues once Psalm 5 is released, we could probably mitigate that by informing users that way

[ghost]

Can you check if you have this line in your require section of composer.json: "ext-pdo": "*",?

the INFO message comes with and without "ext-pdo": "*" in composer.json with psalm 5.0a1.

[AndrolGenhald]

the INFO message comes with and without "ext-pdo": "*" in composer.json with psalm 5.0a1.

Just to clarify, you tried with it in the "require" section, not "require-dev" right? Does it work if you add this to psalm.xml?:

<enableExtensions>
  <extension name="pdo"/>
</enableExtensions>

It shouldn't be necessary, but if that works it at least narrows down what's going wrong.

[ghost]

Yes, adding

<enableExtensions>
  <extension name="pdo"/>

removes the INFO message.

[AndrolGenhald]

Did you have any capitalization in composer.json like "ext-PDO": "*"? I'm fixing that right now as I improve the version message to include a note about extensions.

[ghost]

It's all lowercase, seems resolveFromConfigFile="false" gives the wrong path for composer.json.

[ghost]

Maybe it makes sense to extend psalm.xml with a new attribute composerJson="some-dir/composer.json" similar to autoloader="..." or enable extensions by default if $config_xml->enableExtensions and $config_xml->disableExtensions are not present in the config.

[orklah]

Do you have a custom autoloader? I'd expect to find the composer file if we could find the autoload file...

[AndrolGenhald]

Well it's definitely finding a composer.json that's valid, it should crash if it doesn't. Do you have multiple composer.jsons for some reason? I could maybe see needing to do that if there are separate parts of the same project that have conflicting dependencies, but I can't imagine it's common. Or is it somehow grabbing a composer.json from the vendor directory?

I can't imagine it's common, but if there are multiple composer.jsons somehow it might not be too bad to add a config to specify it explicitly, and I guess it might help in some other weird cases too.

Nevermind, I forgot we support running without composer.json now.

[ghost]

Yes I'm using a custom autoloader to load phpunit.phar into psalm.

My folder structure is:
src/composer.json
src/vendor/...
src/psalm.xml
tests/...

I'm starting psalm from ./ to scan src/ and test/ using resolveFromConfigFile="false".
So Composer::getJsonFilePath($config->base_dir); resolves to Composer::getJsonFilePath(./); and searches "./composer.json" which cannot be found.

[ghost]

Psalm output currently starts with: Target PHP version: 8.0 (inferred from current PHP version)

So here it maybe added:
Target PHP version: 8.0 (inferred from current PHP version)
Cannot find composer.json in "path". Disabling extensions foo, bar, ...
Extensions can be enabled in psalm.xml (see https://psalm.dev/docs/running_psalm/configuration/#enableextensions).

Or in case composer.json can be found:
Target PHP version: 8.0 (inferred from composer.json)
Enabling extensions: foo, bar
Extensions can be customized in psalm.xml (see https://psalm.dev/docs/running_psalm/configuration/#enableextensions).

[ghost]

Target PHP version: 8.0 (inferred from current PHP version)
Enabling extensions: foo, bar (inferred from current PHP cli)

Another option might be to infer loaded extensions from "array_merge(get_loaded_extensions(), get_loaded_extensions(true))" in case composer.json cannot be found.

[AndrolGenhald]

I'd prefer to make sure composer.json is correctly found unless the user is explicitly deciding to run Psalm without a composer.json. In that case, I think using <enableExtensions> is probably the way to go.

I'd rather default to the assumption that no extensions are loaded and make sure users tell Psalm which extensions they need for their project than assume all extensions are enabled and have an error in production that Psalm didn't catch because the extension was enabled locally but not on the production server. I made the same argument here before we merged that change, but I'm not sure @orklah entirely agrees with me. Maybe there's some middle ground that still catches most issues? If a composer.json isn't found I don't personally care as much since it doesn't affect me, I'm just not a fan of assuming extensions that are loaded when Psalm is run will be loaded when the project is run.

I opened #7574 to improve the version message, so that should at least give people a clue if Psalm isn't finding their composer.json.

[orklah]

I made the same argument #7107 (comment) before we merged that change, but I'm not sure @orklah entirely agrees with me

My own experience is kinda different than what you described in your comment. In my case, I installed Psalm on its own composer file on my work computer for analyzing our project. Then we made a docker container for that, still separated from the project (which barely use composer anyway)

I remember I was pretty confused when Psalm wasn't able to find methods for known extensions: #2664 and it wouldn't have helped if Psalm had only looked at the composer file (which was only for Psalm, not for our project).

Now that we have a specific output, this should be more helpful for users, so I'd actually be more fine with dropping reflection for extensions I guess.

[weirdan]

The original issue seems to be solved: https://psalm.dev/r/f0c497ca9c

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/f0c497ca9c

<?php

class Test1 {}

function test(): Test1|false {
    $db = new PDO('');
    $result = $db->query('select * from test');
    $test = $result->fetchObject(Test1::class);
    
    return $test;
}

Psalm output (using commit d3463e3):

No issues!