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 template for QueryBuilder #118

Closed

Conversation

DanielBadura
Copy link

should fix #117

…tyRepository provides the template for QueryBuilder
@andrew-demb
Copy link

See PR that reverted the same changes :)
#114

@DanielBadura
Copy link
Author

Oh yeah, didnt saw it. Maybe we could make select make it mixed? Im not sure, just dropping this here: https://psalm.dev/r/245c5181cb

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/245c5181cb
<?php

/**
 * @template T of mixed
 */
class QueryBuilder
{
    /**
     * @return static<mixed>
     */
    public function select(): self
    {
        return $this;
    }

    /**
     * @return Query<T>
     */
    public function getQuery()
    {
        return new Query();
    }
}

/**
 * @template T of mixed
 */
class Query {}
class Foo {}

/** @var QueryBuilder<Foo> $builder */
$builder = new QueryBuilder();

/** @psalm-trace $query1 */
$query1 = $builder->getQuery();

/** @psalm-trace $query2 */
$query2 = $builder->select()->getQuery();
Psalm output (using commit 10ea05a):

INFO: Trace - 35:1 - $query1: Query<Foo>

INFO: Trace - 38:1 - $query2: Query<mixed>

INFO: UnusedVariable - 35:1 - $query1 is never referenced or the value is not used

INFO: UnusedVariable - 38:1 - $query2 is never referenced or the value is not used

@VincentLanglet
Copy link
Contributor

Oh yeah, didnt saw it. Maybe we could make select make it mixed? Im not sure, just dropping this here: https://psalm.dev/r/245c5181cb

Might be a solution the beginning of a solution, but

  • you should also add support for addSelect.
  • you also need to support
$qb = $this->createQueryBuilder('foo');
$qb->select('bar');
$result = $qb->getQuery()->getResult();

I would recommend to add some test about the issue #114 to prove your solution will not introduce the regression again.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/245c5181cb
<?php

/**
 * @template T of mixed
 */
class QueryBuilder
{
    /**
     * @return static<mixed>
     */
    public function select(): self
    {
        return $this;
    }

    /**
     * @return Query<T>
     */
    public function getQuery()
    {
        return new Query();
    }
}

/**
 * @template T of mixed
 */
class Query {}
class Foo {}

/** @var QueryBuilder<Foo> $builder */
$builder = new QueryBuilder();

/** @psalm-trace $query1 */
$query1 = $builder->getQuery();

/** @psalm-trace $query2 */
$query2 = $builder->select()->getQuery();
Psalm output (using commit 028ac7f):

INFO: Trace - 35:1 - $query1: Query<Foo>

INFO: Trace - 38:1 - $query2: Query<mixed>

INFO: UnusedVariable - 35:1 - $query1 is never referenced or the value is not used

INFO: UnusedVariable - 38:1 - $query2 is never referenced or the value is not used

@DanielBadura DanielBadura deleted the add-templated-query-builder branch March 7, 2023 08:21
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.

Template QueryBuilder
3 participants