False positive (?) on non-nullable property

[ThomasLandauer]

With this being my property

/**
 * @ORM\Column(type="string")
 * @Assert\NotBlank()
 */
private ?string $name = null;

... I'm getting this error:

Property Foo::$name type mapping mismatch: property can contain string|null but database expects string.

So what's the correct way to set up a property that cannot be NULL when being saved to the database?
(It has to be nullable in PHP (i.e. ?string) for Symfony forms to work, see EasyCorp/EasyAdminBundle#3889 (comment))

[ondrejmirtes]

Mapping entities directly to forms is an antipattern, that's why you have to typehint it like this and why PHPStan doesn't like it. See: https://blog.martinhujer.cz/symfony-forms-with-request-objects/

[ThomasLandauer]

I disagree.

1. What you're essentially saying is that the entire Symfony documentation in general (and EasyAdmin as concrete implementation in particular) are doing it wrong. Even if this was true in principle (see 2), I wouldn't expect a Symfony-compatible static analyzer to throw an error on me, just for doing it the "Symfony way". In other words: I think PHPStan should concentrate on finding errors, not criticize the high-level architecture of the app.

2. The proposed way with custom data classes (or data transfer objects) may be a good idea in some situations, but certainly not in all. This is also acknowledged by Martin Hujer:

   There may be exceptions. If you have some Excel-like CRUD application, where every field can be edited anytime, then it may be hard to justify using the request objects.

   If I just need a quick form for internal use to edit some entity, it's fine to use EasyAdmin - after all, that's what it was invented for :-)

So I think you should modify this check, or at least make it somehow user configurable.

[ondrejmirtes]

So I think you should

This is open-source, you can send a PR ;)

But PHPStan is just telling you that your code is brittle and easily broken. If you create new Foo() and try to persist it + flush it to the database, you'll get the error PHPStan is telling you about.

[ThomasLandauer]

PR: Sorry, I have way too little insight :-(

new Foo(): What if my ultimate goal is to prevent "zombie" Foo's (i.e. with null as name) in the database? In this case, the non-nullable ORM mapping actually improves the app! Cause if somebody tries to persist an empty Foo(), I'd rather see the app crash than persist the zombie ;-)

Wouldn't you say this is a legitimate use case?

[ondrejmirtes]

Yes, and what PHPStan is telling you is that your app can crash during persist because of this reason.

[ThomasLandauer]

Yeah, sure. So what's your suggested solution? Just add it to ignoreErrors?

[ccKep]

The obvious fix would be to change your typehint from ?string to string and omit the = null part.

At the moment your entity allows states that your database doesn't.

[ondrejmirtes]

I consider this resolved.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.