Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove question mark when the property is not nullable on getter #1074

Closed
seb-jean opened this issue Feb 18, 2022 · 3 comments
Closed

Remove question mark when the property is not nullable on getter #1074

seb-jean opened this issue Feb 18, 2022 · 3 comments

Comments

@seb-jean
Copy link
Contributor

Hi,

I'm wondering whether to prefix the type name with a question mark when the property is not nullable on getter.

Here is an example of the code of the entity generated by the maker-bundle:

<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
use Symfony\Component\Security\Core\User\UserInterface;

class User implements UserInterface, PasswordAuthenticatedUserInterface
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column(type: 'integer')]
    private $id;
    
    #[ORM\Column(type: 'string', length: 255)]
    private $lastName;
    
    public function getId(): ?int
    {
        return $this->id;
    }

    public function getLastName(): ?string
    {
        return $this->lastName;
    }

    public function setLastName(string $lastName): self
    {
        $this->lastName = $lastName;

        return $this;
    }
}

Shouldn't we generate the code below instead:

public function getLastName(): string
{
    return $this->lastName;
}

Thanks 😄

@jrushlow
Copy link
Collaborator

There were "bugs" in ORM && Symfony related to typed properties in PHP 7.4 that required the properties to be nullable for doctrine.

What I do not know, is if we remove the ? from properties that are not nullable, will this cause any conflicts with earlier supported symfony / doctrine versions.

From a different issue (copy and pasted)
Doctrine & Symfony have fixed the bugs which did not allow to proxy a doctrine entity with uninitialized properties. See doctrine/orm#8030 & symfony/symfony#36332

@thsmrtone1
Copy link

@seb-jean In your example, the getter should be nullable. See this case:

$user = new User();
dump($user->getLastName()); // null

If getLastName() is called before setLastName() is called, and/or before the entity is persisted to the database, then the value will be null regardless what the column definition says.

@weaverryan
Copy link
Member

@seb-jean In your example, the getter should be nullable. See this case:

Yup, exactly right :).

One thing we DO need to do still is start adding property types to generated code. In this case, the property would be generated as private ?string $lastName;.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants