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 type detection for PDOStatement::fetchAll(PDO::FETCH_CLASS, SomeClass::class) #9974

Closed
thbley opened this issue Jun 28, 2023 · 4 comments
Labels
easy problems Issues that can be fixed without background knowledge of Psalm enhancement good first issue Help wanted type providers

Comments

@thbley
Copy link
Contributor

thbley commented Jun 28, 2023

Example fetchAll: https://psalm.dev/r/a9b0c85b47

Add type detection for PDOStatement::fetchAll(PDO::FETCH_CLASS, SomeClass::class): SomeClass[]|false;

Currently PDOStatement::fetchAll() is currently defined as:

'PDOStatement::fetchAll' => ['array', 'mode='=>'int', '...args='=>'mixed'],

which infers to array<array-key, mixed> instead of array<array-key, SomeClass>

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/a9b0c85b47
<?php

class Task {}

class Database {
    /**
     * @return Task[]
     */
    public function getTasks(): array
    {
        $database = new PDO('');
        return $database
            ->query('SELECT id, title, duedate FROM task')
			->fetchAll(PDO::FETCH_CLASS, Task::class) ?: [];
    }
}
Psalm output (using commit 086b943):

INFO: MixedReturnTypeCoercion - 12:16 - The type 'array<array-key, mixed>' is more general than the declared return type 'array<array-key, Task>' for Database::getTasks

INFO: MixedReturnTypeCoercion - 7:16 - The declared return type 'array<array-key, Task>' for Database::getTasks is more specific than the inferred return type 'array<array-key, mixed>'

@orklah
Copy link
Collaborator

orklah commented Jun 28, 2023

Thanks for the feedback!

Turns out we already do something similar for fetch: https://github.com/vimeo/psalm/blob/master/src/Psalm/Internal/Provider/ReturnTypeProvider/PdoStatementReturnTypeProvider.php#L34

It should be pretty easy to adapt this to work with fetchAll too

@robchett
Copy link
Contributor

Looks like this issue is resolved? There was quite a lot of discussion on the merge request, but it feels beyond the scope of the tags :)

@thbley
Copy link
Contributor Author

thbley commented Sep 17, 2023

Yes, is resolved.

@thbley thbley closed this as completed Sep 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy problems Issues that can be fixed without background knowledge of Psalm enhancement good first issue Help wanted type providers
Projects
None yet
Development

No branches or pull requests

3 participants