Manage innerJoin not nullable associations

[soullivaneuh]

With this PHP code:

$qb = $this->invoiceRepository->createQueryBuilder('i')
    ->select('i, c, ba')
    ->innerJoin('i.customer', 'c', Expr\Join::WITH, 'c.automaticPaymentMethod = :automatic_payment_method')
    ->innerJoin('c.bankAccount', 'ba', Expr\Join::WITH, 'ba.activated = :activated AND ba.mandateSigned = :mandateSigned AND ba.mandateSignedDate IS NOT NULL')

foreach ($invoices as $invoice) {
    $customer = $invoice->getCustomer();
    $bankAccount = $customer->getBankAccount();

    $bankAccount->setFirstDebit(true);
    $this->persist($bankAccount);
}
;

I'll have this error:

Parameter #1 $object of method Doctrine\Common\Persistence\ObjectManagerDecorator::persist() expects object, AppBundle\Entity\BankAccount|null given.

But this is not true as innerJoin will drop customers without a bank account. Am I right?

If I'm right, checking if $bankAccount in this case is overkill and will never match.

[ondrejmirtes]

This would be really hard to track in a codebase. Also: you can fetch customer with a bank account and later set it to null, confusing the analyser. I recommend you to add a method to your entity that you only call when you’re sure there’s an account. I have a convention of find (nullable return type) vs. get (non-nullable).

On Wed, 24 Jan 2018 at 13:09, Sullivan SENECHAL ***@***.***> wrote: With this PHP code: $qb = $this->invoiceRepository->createQueryBuilder('i') ->select('i, c, ba') ->innerJoin('i.customer', 'c', Expr\Join::WITH, 'c.automaticPaymentMethod = :automatic_payment_method') ->innerJoin('c.bankAccount', 'ba', Expr\Join::WITH, 'ba.activated = :activated AND ba.mandateSigned = :mandateSigned AND ba.mandateSignedDate IS NOT NULL')foreach ($invoices as $invoice) { $customer = $invoice->getCustomer(); $bankAccount = $customer->getBankAccount(); $bankAccount->setFirstDebit(true); $this->persist($bankAccount);};I'll have this error: Parameter #1 <#1> $object of method Doctrine\Common\Persistence\ObjectManagerDecorator::persist() expects object, AppBundle\Entity\BankAccount|null given. But this is not `true` as `innerJoin` will drop customers without a bank account. Am I right? If I'm right, checking if `$bankAccount` in this case is overkill and will never match. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#22>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGZuKYL278D2auFOhWglu3HZy26j-aRks5tNx1hgaJpZM4RrKDP> .

-- Ondřej Mirtes

[soullivaneuh]

This would be really hard to track in a codebase.

I don't know, but it's still a false positive AFAIK. IMHO, the issue should be still open. Maybe somebody will find a solution for that. 👍

you can fetch
customer with a bank account and later set it to null, confusing the
analyser.

I'm not sure to follow you here. What do you mean by set it to null? On the codebase? In this case, it can be done for anything. 🤔

Or are you referencing the fact that the return value of getBankAccount can be null? You are maybe right indeed, but what should we do in this case? A special method getExistingBankAccount without possible null return? Something else? 🤔

[ondrejmirtes]

It's simple - getBankAccount() can always return null because you cannot be sure about the inner state of the object.

Yep, your suggestion is correct - you should use a non-nullable getter in cases you're sure there is definitely the bank account set.

[soullivaneuh]

Ok I see, thanks for your time @ondrejmirtes! 👍

[github-actions]

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.