phpstan-symfony icon indicating copy to clipboard operation
phpstan-symfony copied to clipboard

False positive when an entity's property gets changed by a form inside an if

Open ThomasLandauer opened this issue 4 years ago • 2 comments

I guess the title doesn't really say it all ;-)

So let me show you what I have:

foreach($users as $user)
{
    // If the user is not OK yet, generate the form to edit the user:
    if (false === $user->getOk()) {
        $form[$user->getId()] = $this->get('form.factory')->createNamed('form' . $user->getId(), UserType::class, $user);
        $form[$user->getId()]->handleRequest($request);
        if ($form[$user->getId()]->isSubmitted() and $form[$user->getId()]->isValid())
        {
            $entityManager->persist($user);
            $entityManager->flush();
            // Here phpstan reports "If condition is always false.", cause it obviously remembers the `if` from above
            // and doesn't see that `$user->ok` could have been changed by the form meanwhile
            if ($user->getOk()) {
                $this->addFlash('success', 'User is now OK');
            }
        }
    }
}

ThomasLandauer avatar Jan 07 '21 23:01 ThomasLandauer

I'd recommend you to rewrite your code like this:

foreach($users as $user)
{
    // If the user is not OK yet, generate the form to edit the user:
    $ok = $user->getOk();
    if (!$ok) {
        $form[$user->getId()] = $this->get('form.factory')->createNamed('form' . $user->getId(), UserType::class, $user);
        $form[$user->getId()]->handleRequest($request);
        if ($form[$user->getId()]->isSubmitted() and $form[$user->getId()]->isValid())
        {
            $entityManager->persist($user);
            $entityManager->flush();
            // Here phpstan reports "If condition is always false.", cause it obviously remembers the `if` from above
            // and doesn't see that `$user->ok` could have been changed by the form meanwhile
            if ($user->getOk()) {
                $this->addFlash('success', 'User is now OK');
            }
        }
    }
}

ondrejmirtes avatar Jan 08 '21 09:01 ondrejmirtes

Cool, thanks, that indeed fixes it :-) So if you can't think of a general solution for this, you can close the issue.

ThomasLandauer avatar Jan 09 '21 23:01 ThomasLandauer