annotations icon indicating copy to clipboard operation
annotations copied to clipboard

PHP 8 Annotations bug?

Open TimurFlush opened this issue 5 years ago • 4 comments

Hello!

My stack: php: 8.0.0 symfony: 5.2 doctrine/orm: 2.8.1 doctrine/doctrine-bundle: 2.2.2

I have the next entitity: entity

I try to request a user by his login through the repository

return new Response(
    $this->userRepository->findOneBy(['login' => 'someLogin'])->getLogin()
);

and catch the next error: error

This is due to the fact that the Doctrine annotation parser tries to go through all the properties of the annotation @ORM\Id (see the first screenshot). And since they don't exist, the foreach loop passes null and I catch this error.

To confirm my words, I attach screenshots from the xdebug:

Doctrine\Common\Annotations\CachedReader::getPropertyAnnotation() xdeb1

Doctrine\Common\Annotations\CachedReader::getPropertyAnnotations() xdeb2

TimurFlush avatar Dec 09 '20 12:12 TimurFlush

Temporary fix in the Doctrine\Common\Annotations\CachedReader class:

    /**
     * {@inheritDoc}
     */
    public function getPropertyAnnotation(ReflectionProperty $property, $annotationName)
    {
        $propertyAnnotations = $this->getPropertyAnnotations($property) ?? [];

        foreach ($propertyAnnotations as $annot) {
            if ($annot instanceof $annotationName) {
                return $annot;
            }
        }

        return null;
    }

TimurFlush avatar Dec 09 '20 12:12 TimurFlush

The stack to reproduce:

  • PHP: 8.0.0
  • Symfony: 5.2
  • doctrine/orm: 2.8.1
  • doctrine/doctrine-bundle: 2.2.2
  • postgresql: 13

The files to reproduce:

<?php
// File: src/Entity/User.php

namespace App\Entity;

use App\Repository\UserRepository;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Security\Core\User\UserInterface;

/**
 * @ORM\Entity(repositoryClass=UserRepository::class)
 * @ORM\Table(name="`users`")
 */
class User implements UserInterface
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="bigint")
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=180, unique=true, nullable=false)
     */
    private $login;

    /**
     * Returns the user identifier.
     *
     * @return int|null
     */
    public function getId(): ?int
    {
        return $this->id;
    }

    /**
     * Returns a user login.
     *
     * @return string|null
     */
    public function getLogin(): ?string
    {
        return $this->login;
    }

    /**
     * Sets a user login.
     *
     * @param string $login
     * @return $this
     */
    public function setLogin(string $login): self
    {
        $this->login = $login;
        return $this;
    }

    public function setRoles(array $roles): self
    {
        $this->roles = $roles;

        return $this;
    }

    public function setPassword(string $password): self
    {
        $this->password = $password;

        return $this;
    }

}
<?php
// File: src/Repository/UserRepository.php

namespace App\Repository;

use App\Entity\User;
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use Doctrine\Persistence\ManagerRegistry;
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
 * @method User|null find($id, $lockMode = null, $lockVersion = null)
 * @method User|null findOneBy(array $criteria, array $orderBy = null)
 * @method User[]    findAll()
 * @method User[]    findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null)
 */
class UserRepository extends ServiceEntityRepository implements PasswordUpgraderInterface
{
    public function __construct(ManagerRegistry $registry)
    {
        parent::__construct($registry, User::class);
    }

    /**
     * Used to upgrade (rehash) the user's password automatically over time.
     */
    public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
    {
        if (!$user instanceof User) {
            throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
        }

        $user->setPassword($newEncodedPassword);
        $this->_em->persist($user);
        $this->_em->flush();
    }
}

<?php
// File: /path/to/some/controller

namespace App\Controller;

class SomeController extends AbstractController
{
    public function __construct(
        private \App\Repository\UserRepository $userRepository
    ) {
         // nop
    }

    public function reproduce()
    {
        $this->userRepository->find(1);
    }
}

The steps to reproduce:

  1. Create the postgresql users table
  2. Create a user
  3. Perform the SomeController::reproduce() action

TimurFlush avatar Dec 09 '20 12:12 TimurFlush

This is due to the fact that the Doctrine annotation parser tries to go through all the properties of the annotation @Orm\Id (see the first screenshot).

this assertion looks wrong to me. getPropertyAnnotations return an array of all annotations on that property. The loop is over that array, not over properties of the annotation.

stof avatar Dec 09 '20 13:12 stof

The Xdebug output even shows that it returns an array, so its really confusing that you get this foreach. Maybe its caused by another method than the one you have been showing?

beberlei avatar Dec 29 '20 15:12 beberlei