Incorrect behavior of TypedPropertyRector: Generates non-nullable properties and initializes them with null

[YetiCGN]

Bug Report

Subject	Details
Rector version	last dev-master
Installed as	composer dependency

Minimal PHP Code Causing Issue

See https://getrector.org/demo/1ec4d5b4-8951-6fd0-ae3f-410987af81e1

<?php

namespace AppBundle\Entity;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Validator\Constraints as Assert;
use Vich\UploaderBundle\Mapping\Annotation as Vich;

/**
 * @ORM\Table(name="board", indexes={@ORM\Index(columns={"name"}, flags={"fulltext"})})
 * @ORM\Entity(repositoryClass="AppBundle\Repository\BoardRepository")
 * @Vich\Uploadable
 */
class Board
{
    /**
     * @var int
     *
     * @ORM\Column(name="id", type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    private $id;

    /**
     * @var string
     *
     * @Assert\Length(max="128", min="4")
     * @Assert\NotBlank()
     * @ORM\Column(name="name", type="string")
     */
    private $name;

    /**
     * @ORM\OneToMany(targetEntity="UserBoardAccess", mappedBy="board", cascade={"persist", "remove"}, orphanRemoval=true)
     */
    private $users;

    /**
     * @ORM\OneToMany(targetEntity="AppBundle\Entity\BoardMoodImage", mappedBy="board", cascade={"persist", "remove"}, orphanRemoval=true)
     * @ORM\OrderBy({"index" = "ASC"})
     * @Assert\Valid()
     */
    private $moodImages;

    public function __construct()
    {
        $this->moodImages = new ArrayCollection();
    }

    public function __clone()
    {
        if ($this->id) {
            $this->id = null;
        }
    }

    /**
     * Get id
     *
     * @return int
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * @param int $id
     */
    public function setId(int $id)
    {
        $this->id = $id;
    }

    /**
     * @return string
     */
    public function getName()
    {
        return $this->name;
    }

    /**
     * @param string $name
     */
    public function setName($name)
    {
        $this->name = $name;
    }

    /**
     * Get users
     *
     * @return mixed
     */
    public function getUsers()
    {
        return $this->users;
    }

    /**
     * Add user
     *
     * @param \AppBundle\Entity\UserBoardAccess $user
     *
     * @return Board
     */
    public function addUser(\AppBundle\Entity\UserBoardAccess $user)
    {
        $this->users[] = $user;

        return $this;
    }

    /**
     * Remove user
     *
     * @param \AppBundle\Entity\UserBoardAccess $user
     */
    public function removeUser(\AppBundle\Entity\UserBoardAccess $user)
    {
        $this->users->removeElement($user);
    }

    /**
     * @return mixed
     */
    public function getMoodImages()
    {
        return $this->moodImages;
    }

    /**
     * @param mixed $moodImages
     */
    public function setMoodImages($moodImages)
    {
        $this->moodImages = $moodImages;
    }

    /**
     * Add BoardMoodImage
     *
     * @param \AppBundle\Entity\BoardMoodImage $moodImage
     *
     * @return Board
     */
    public function addMoodImage(BoardMoodImage $moodImage)
    {
        $this->moodImages[] = $moodImage;

        return $this;
    }

    /**
     * Remove BoardMoodImage
     *
     * @param \AppBundle\Entity\BoardMoodImage $moodImage
     */
    public function removeMoodImage(BoardMoodImage $moodImage)
    {
        $this->moodImages->removeElement($moodImage);
    }
}

Responsible rules

• TypedPropertyRector

Expected Behavior

The moodImage property is initialized with null but not declared as nullable, whereas the users property works just fine.

[TomasVotruba]

Thanks for sharing 👍
The demo link code is quite huge and hard to read. Could you narrow it to the single property you talk about so it's easier to spot?

[YetiCGN]

I already narrowed it down to the most important properties, I thought. Especially the difference between the two Collection propreties. But here's a real minimal example:

https://getrector.org/demo/1ec4dc79-065c-630c-bdc6-375aa537fbf0

Interestingly, if I remove the add and remove functions, it correctly makes the type nullable: https://getrector.org/demo/1ec4dc78-700e-6ea4-a7a6-2dceb40ff9dd

[TomasVotruba]

Thank you 👍 it looks more clear now.

Could you add this to test fixtures of responsible rule?

[YetiCGN]

Like this? 🙂 rectorphp/rector-src#1303

[TomasVotruba]

That's perfect 🙂 , thank you 👍

[samsonasik]

Already covered in latest main as I commented at rectorphp/rector-src#1303 (comment)