Skip to content

Infering the result type of a Query instance #221

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

Closed
arnaud-lb opened this issue Oct 29, 2021 · 9 comments
Closed

Infering the result type of a Query instance #221

arnaud-lb opened this issue Oct 29, 2021 · 9 comments

Comments

@arnaud-lb
Copy link
Contributor

arnaud-lb commented Oct 29, 2021

It may be possible to infer the return type of execute(), getResult(), etc on Query instances when the query string is known statically.

This could be implemented like this:

  • Override the Query and AbstractQuery classes so that they have a type parameter, representing the type of their results (e.g. a Query returning User objects would be a Query<User>)
  • Add a return type extension for EntityManagerInterface::createQuery(), to infer the result type from the query string, and so that we can tel that createQuery() returns a Query<something>

The ResultSetMapper of a Query provides enough info to infer the result type in the most simple cases (e.g. SELECT u FROM User). This would already be super useful. Other cases might require a custom SqlWalker because the ResultSetMapper doesn't provide any information on nullability, for example.

@arnaud-lb
Copy link
Contributor Author

@noemi-salaun
Copy link

I think the type can also be infered with QueryBuilder, like in this case

    /**
     * @return VariableModifier[]
     */
    public function getAll(): array
    {
        return $this->getEntityManager()->getRepository(VariableModifier::class)
            ->createQueryBuilder('modifier')
            ->getQuery()
            ->getResult();
    }

@arnaud-lb
Copy link
Contributor Author

arnaud-lb commented Nov 13, 2021

Here is a more advanced branch that implements type inference for most queries: https://github.com/arnaud-lb/phpstan-doctrine/pull/2/files

It will properly infer the result type of any query, including queries with scalar results, joins or left joins, aggregations, functions, and all kinds of expressions (see the related tests)

There are a few things I left for later: sub queries, identity(), size(). These things will cause some parts of the query result to be mixed, so there will be no false typing.

I may eventually submit this in two PRs:

  • Generic Query stub + query result type inference and the dynamic return type extension for EntityManager::createQuery()
  • Dynamic return type extension for Query::getResult / execute / etc

WDYT ?

@enumag
Copy link
Contributor

enumag commented Nov 16, 2021

@arnaud-lb Sounds great. For now let's focus on the most common cases. Indeed it makes no sense to delay this feature for sub queries and such. Please make a PR where we can discuss the details. You can make it as a draft if unsure about some things.

@arnaud-lb
Copy link
Contributor Author

Thanks @enumag. I opened a PR at #232

@ondrejmirtes
Copy link
Member

#232 merged and released as phpstan-doctrine 1.2.1 :)

@fliespl
Copy link

fliespl commented Feb 10, 2022

@ondrejmirtes It works fine with basic queries, but once: setLockMode is attached to getQuery() it loses context and reports error.

@ondrejmirtes
Copy link
Member

@fliespl Feel free to open a new bug report with more details about it.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants