Looks like your PHP doc is false (it should be TravelDestinationType|null), why duplicating informations anyway ?

Looks like your PHP doc is false (it should be TravelDestinationType|null)

I think the point is that PHPDoc might be incomplete. If we can merge information from property + methods that'd go a long way. Better still if we can merge information from different extractors...

why duplicating informations anyway ?

What do you mean? Let's not get into a CS discussion here... 😄

But I think the point of this issue is whether this kind of behavioural change should be considered a BC break.

I think the point is that PHPDoc might be incomplete.

Imo the point of having PHPDocs is to complete missing informations of php type hints.

But I think the point of this issue is whether this kind of behavioural change should be considered a BC break.

For me it's not worth a bc layer, php docs should always be more complete than php type hints so there's only a bc break if your type hints contradict each other.

Yes, PHPDoc is more expressive in most cases, but I do not see PHPDoc as the single source of truth, or the authoritative source of type information. Because it's documentation and it needs to balance readability with accuracy.

The way I think it should work is for PHPDoc to complement type information we retrieve from reflection (and other sources). Basically we should not expect a single source to hold all the information.

The way I think it should work is for PHPDoc to complement type information we retrieve from reflection

In my opinion, your example shows an error in your code. Your PHPDoc block is not complementing the type information about the $type parameter, as it does not specify the parameter to be nullable.

Well, yeah, I've since bitten the bullet and decided to make sure my PHPDoc is "complete". I guess it's for the better, even if it's only to make a certain static analyzer happy.

I dont think the current behavior is good. This nullable case is a trap for beginners, since its not entirely clear what can be done. For example the following works, despite the setter being called:

class TravelDestination
{
    /**
     * @var TravelDestinationType|null Type of the travel destination.
     *
     * @ORM\Column(type="travel_destination_type_enum", nullable=true)
     */
    protected $type;

    /**
     * Sets type.
     *
     * @param TravelDestinationType $type
     *
     * @return $this
     */
    public function setType(?TravelDestinationType $type)
    {
        $this->type = $type;

        return $this;
    }

    /**
     * Gets type.
     *
     * @return TravelDestinationType|null
     */
    public function getType(): ?TravelDestinationType
    {
        return $this->type;
    }
}

and i guess the following will work too:

class TravelDestination
{
    /**
     * @var TravelDestinationType Type of the travel destination.
     *
     * @ORM\Column(type="travel_destination_type_enum", nullable=true)
     */
    protected $type;

    /**
     * Sets type.
     *
     * @param TravelDestinationType|null $type
     *
     * @return $this
     */
    public function setType(?TravelDestinationType $type)
    {
        $this->type = $type;

        return $this;
    }

    /**
     * Gets type.
     *
     * @return TravelDestinationType|null
     */
    public function getType(): ?TravelDestinationType
    {
        return $this->type;
    }
}

This gets especially annoying, when you work with the serializer, because you will only notice on runtime that the OP code example doesnt work and not on configuration time. If the different sources of type info arent in sync, the serializer should be graceful and widen the target type as much as possible by merging the type information available from different sources with a certain strategy and throw an error if that merging isnt possible. Checking if the type information in the source is well done is the job of static analysis tools and not the serializer.

what's the status of this ?

I don't think it was a BC break. 😆