From eea5266adfac326278f0c23295ec5061ea34a7f2 Mon Sep 17 00:00:00 2001 From: Nathan Boiron Date: Thu, 2 Jul 2026 23:57:06 +0200 Subject: [PATCH] =?UTF-8?q?Id=20non-nullable=20des=20entit=C3=A9s=20Doctri?= =?UTF-8?q?ne?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/config/reference.php | 2 +- .../ADR-003-doctrine-id-non-nullable.md | 123 ++++++++++++++++++ phpstan-baseline.php | 12 -- .../AppBundle/Accounting/Entity/Account.php | 2 +- .../AppBundle/Accounting/Entity/Category.php | 2 +- sources/AppBundle/Accounting/Entity/Event.php | 2 +- .../AppBundle/Accounting/Entity/Operation.php | 2 +- .../AppBundle/Accounting/Entity/Payment.php | 2 +- .../AppBundle/Accounting/Entity/Produit.php | 2 +- sources/AppBundle/Accounting/Entity/Rule.php | 2 +- .../AssembleeGenerale/Entity/Presence.php | 2 +- .../AssembleeGenerale/Entity/Question.php | 2 +- .../Association/Entity/Utilisateur.php | 2 +- sources/AppBundle/Site/Entity/Article.php | 2 +- sources/AppBundle/Site/Entity/Feuille.php | 2 +- sources/AppBundle/Site/Entity/Rubrique.php | 2 +- .../SuperApero/Entity/SuperApero.php | 2 +- .../SuperApero/Entity/SuperAperoMeetup.php | 2 +- .../SuperApero/Form/SuperAperoType.php | 32 +++-- sources/AppBundle/Veille/Entity/Envoi.php | 2 +- .../Entity/NewsletterDesinscription.php | 2 +- .../Veille/Entity/NewsletterInscription.php | 2 +- templates/admin/site/article_form.html.twig | 2 +- 23 files changed, 164 insertions(+), 43 deletions(-) create mode 100644 doc/decisions/ADR-003-doctrine-id-non-nullable.md diff --git a/app/config/reference.php b/app/config/reference.php index 1ddd0e95b..d06f58d5e 100644 --- a/app/config/reference.php +++ b/app/config/reference.php @@ -1045,7 +1045,7 @@ * enabled?: bool|Param, // Default: false * }, * markdown?: bool|array{ - * enabled?: bool|Param, // Default: false + * enabled?: bool|Param, // Default: true * }, * intl?: bool|array{ * enabled?: bool|Param, // Default: true diff --git a/doc/decisions/ADR-003-doctrine-id-non-nullable.md b/doc/decisions/ADR-003-doctrine-id-non-nullable.md new file mode 100644 index 000000000..458922cd0 --- /dev/null +++ b/doc/decisions/ADR-003-doctrine-id-non-nullable.md @@ -0,0 +1,123 @@ +--- +Id: ADR-003 +Date: 2026-06-29 +Statut: Proposé +--- + +# Les entités Doctrine ont un id non-nullable + +## Contexte + +Avec l'ajout de la baseline PHPStan et le passage au niveau 10, il y a pas mal d'endroits dans le code où l'id nullable +des entités pose problème. + +Par exemple, quand on récupère une liste d'entités depuis un repository, on sait que l'id est présent, mais pas PHPStan +car la propriété reste nullable dans la classe de l'entité. + +Cela force des vérifications qui n'apportent pas grand chose et rendent le code plus difficile à lire et naviguer. + +Par exemple : + +```php +#[ORM\Entity] +#[ORM\Table(name: 'exemple')] +class Entity +{ + #[ORM\Id] + #[ORM\GeneratedValue] + #[ORM\Column] + public ?int $id = null; + + #[ORM\Column(length: 255, nullable: true)] + public string $foo = null; +} + +class ExampleRepository +{ + /** + * @return array + */ + public function all(): array { /* return ... */ } +} + +$entities = $exempleRepository->all(); + +$map = []; +foreach ($entities as $entity) { + // Cette ligne va déclencher une erreur PHPStan car l'id pourrait être nullable, + // alors qu'on sait ici que ce n'est pas le cas. + $map[$entity->id] = $entity->foo; + + // Il faudrait faire ça à chaque fois : + if (!isset($entity->id)) { + continue; + } + + $map[$entity->id] = $entity->foo; +} +``` + +## Décision + +Dans une entité Doctrine, la propriété `id` est déclarée non-nullable. + +Cela permet à PHPStan de mieux analyser le code, tout en conservant une certaine sécurité. Si une entité n'est pas +persistée et qu'on tente de lire son id, cela déclenche une erreur. + +### Détails d'implémentation + +```php +use AppBundle\Doctrine\Entity; +use Doctrine\ORM\Mapping as ORM; + +#[ORM\Entity] +#[ORM\Table(name: 'exemple')] +class Exemple +{ + #[ORM\Id] + #[ORM\GeneratedValue] + #[ORM\Column] + public int $id; + + #[ORM\Column(length: 255, nullable: false)] + public string $nonNullbale; + + #[ORM\Column(length: 255, nullable: true)] + public ?string $nullable = null; +} +``` + +Et à l'utilisation : + +```php +if (isset($exemple->id)) { + // $exemple->id est initialisé et non-null +} +``` + +## Alternatives considérées + +1. **Une classe parente** : Cela force toutes les entités à hériter d'une classe, et, limite aux tables qui ont un id auto-incrément. +2. **Un trait** : C'est plus difficile et lent à analyser pour PHPStan qu'une classe parente. +3. **Vérifier l'id à chaque fois** : Le code devient moins lisible pour peu d'intérêt. + +## Conséquences + +### Positives + +Quand on récupère une ou plusieurs entités depuis la base de données, plus besoin de vérifier la présence de l'id dans +l'instance. + +Si on tente d'accéder à l'id d'une entité à un endroit non vérifié, une erreur survient fort et au bon endroit (au lieu +de trimballer un `null` plus loin dans le code). + +### Négatives + +Il faut faire un peu plus attention en manipulant des entités, car tenter de lire un id qui ne serait pas présent +déclenche une erreur. + +## Références + +Analyse des traits par PHPStan : https://phpstan.org/blog/how-phpstan-analyses-traits + +Exemple PHPStan : https://phpstan.org/r/3f3354d7-d6f5-4493-bfbd-3f7a37cf8d32 diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 30c4b3e7c..b5ed59d7a 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -115,18 +115,6 @@ 'count' => 1, 'path' => __DIR__ . '/sources/Afup/Corporate/Page.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Parameter \\#1 \\$parentId of method AppBundle\\\\Site\\\\Entity\\\\Repository\\\\FeuilleRepository\\:\\:getFeuillesEnfant\\(\\) expects int, int\\|null given\\.$#', - 'identifier' => 'argument.type', - 'count' => 3, - 'path' => __DIR__ . '/sources/Afup/Corporate/Page.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Possibly invalid array key type int\\|null\\.$#', - 'identifier' => 'offsetAccess.invalidOffset', - 'count' => 1, - 'path' => __DIR__ . '/sources/Afup/Corporate/Page.php', -]; $ignoreErrors[] = [ 'message' => '#^Cannot access offset \'elements\' on mixed\\.$#', 'identifier' => 'offsetAccess.nonOffsetAccessible', diff --git a/sources/AppBundle/Accounting/Entity/Account.php b/sources/AppBundle/Accounting/Entity/Account.php index 38d87ba42..d6125cebd 100644 --- a/sources/AppBundle/Accounting/Entity/Account.php +++ b/sources/AppBundle/Accounting/Entity/Account.php @@ -13,7 +13,7 @@ class Account #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(name: 'nom_compte', length: 45, nullable: false)] public string $name; diff --git a/sources/AppBundle/Accounting/Entity/Category.php b/sources/AppBundle/Accounting/Entity/Category.php index 98072f573..1980a3e3b 100644 --- a/sources/AppBundle/Accounting/Entity/Category.php +++ b/sources/AppBundle/Accounting/Entity/Category.php @@ -13,7 +13,7 @@ class Category #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(name: 'categorie', length: 255, nullable: false)] public string $name; diff --git a/sources/AppBundle/Accounting/Entity/Event.php b/sources/AppBundle/Accounting/Entity/Event.php index d6110ac23..aaee9f909 100644 --- a/sources/AppBundle/Accounting/Entity/Event.php +++ b/sources/AppBundle/Accounting/Entity/Event.php @@ -13,7 +13,7 @@ class Event #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(name: 'evenement', length: 50, nullable: false)] public string $name; diff --git a/sources/AppBundle/Accounting/Entity/Operation.php b/sources/AppBundle/Accounting/Entity/Operation.php index cf3fe01ce..44fbdc479 100644 --- a/sources/AppBundle/Accounting/Entity/Operation.php +++ b/sources/AppBundle/Accounting/Entity/Operation.php @@ -13,7 +13,7 @@ class Operation #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(name: 'operation', length: 255, nullable: false)] public string $name; diff --git a/sources/AppBundle/Accounting/Entity/Payment.php b/sources/AppBundle/Accounting/Entity/Payment.php index 2cc9a5899..c25e9ba59 100644 --- a/sources/AppBundle/Accounting/Entity/Payment.php +++ b/sources/AppBundle/Accounting/Entity/Payment.php @@ -13,7 +13,7 @@ class Payment #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(name: 'reglement', length: 50, nullable: false)] public string $name; diff --git a/sources/AppBundle/Accounting/Entity/Produit.php b/sources/AppBundle/Accounting/Entity/Produit.php index 6748582e4..4443924a3 100644 --- a/sources/AppBundle/Accounting/Entity/Produit.php +++ b/sources/AppBundle/Accounting/Entity/Produit.php @@ -14,7 +14,7 @@ class Produit #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(length: 255, nullable: false)] public string $reference; diff --git a/sources/AppBundle/Accounting/Entity/Rule.php b/sources/AppBundle/Accounting/Entity/Rule.php index df6377f27..40888116b 100644 --- a/sources/AppBundle/Accounting/Entity/Rule.php +++ b/sources/AppBundle/Accounting/Entity/Rule.php @@ -13,7 +13,7 @@ class Rule #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(length: 255, nullable: false)] public string $label; diff --git a/sources/AppBundle/AssembleeGenerale/Entity/Presence.php b/sources/AppBundle/AssembleeGenerale/Entity/Presence.php index 475a00387..dc141b1f4 100644 --- a/sources/AppBundle/AssembleeGenerale/Entity/Presence.php +++ b/sources/AppBundle/AssembleeGenerale/Entity/Presence.php @@ -16,7 +16,7 @@ class Presence #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(type: UnixTimestampType::NAME, nullable: false)] public \DateTime $date; diff --git a/sources/AppBundle/AssembleeGenerale/Entity/Question.php b/sources/AppBundle/AssembleeGenerale/Entity/Question.php index abaf63289..96c61106e 100644 --- a/sources/AppBundle/AssembleeGenerale/Entity/Question.php +++ b/sources/AppBundle/AssembleeGenerale/Entity/Question.php @@ -16,7 +16,7 @@ class Question #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(type: UnixTimestampType::NAME, nullable: true)] public ?\DateTime $date = null; diff --git a/sources/AppBundle/Association/Entity/Utilisateur.php b/sources/AppBundle/Association/Entity/Utilisateur.php index ab4cf3e9e..a791a1495 100644 --- a/sources/AppBundle/Association/Entity/Utilisateur.php +++ b/sources/AppBundle/Association/Entity/Utilisateur.php @@ -13,7 +13,7 @@ class Utilisateur #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(length: 255, nullable: true)] public ?string $email = null; diff --git a/sources/AppBundle/Site/Entity/Article.php b/sources/AppBundle/Site/Entity/Article.php index b5001e3d5..3e255cb89 100644 --- a/sources/AppBundle/Site/Entity/Article.php +++ b/sources/AppBundle/Site/Entity/Article.php @@ -16,7 +16,7 @@ class Article #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\ManyToOne(targetEntity: Rubrique::class)] #[ORM\JoinColumn(name: 'id_site_rubrique', referencedColumnName: 'id', nullable: true)] diff --git a/sources/AppBundle/Site/Entity/Feuille.php b/sources/AppBundle/Site/Entity/Feuille.php index a825bbd88..9e634f2d2 100644 --- a/sources/AppBundle/Site/Entity/Feuille.php +++ b/sources/AppBundle/Site/Entity/Feuille.php @@ -14,7 +14,7 @@ class Feuille #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(nullable: true)] public ?int $idParent = null; diff --git a/sources/AppBundle/Site/Entity/Rubrique.php b/sources/AppBundle/Site/Entity/Rubrique.php index ec8248041..7596d0671 100644 --- a/sources/AppBundle/Site/Entity/Rubrique.php +++ b/sources/AppBundle/Site/Entity/Rubrique.php @@ -21,7 +21,7 @@ class Rubrique #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(nullable: true)] public ?int $idParent = null; diff --git a/sources/AppBundle/SuperApero/Entity/SuperApero.php b/sources/AppBundle/SuperApero/Entity/SuperApero.php index 590c59719..cbe78ac5b 100644 --- a/sources/AppBundle/SuperApero/Entity/SuperApero.php +++ b/sources/AppBundle/SuperApero/Entity/SuperApero.php @@ -17,7 +17,7 @@ class SuperApero #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(type: 'date_immutable', nullable: false)] public DateTimeImmutable $date; diff --git a/sources/AppBundle/SuperApero/Entity/SuperAperoMeetup.php b/sources/AppBundle/SuperApero/Entity/SuperAperoMeetup.php index 074dac3f3..60ed8886d 100644 --- a/sources/AppBundle/SuperApero/Entity/SuperAperoMeetup.php +++ b/sources/AppBundle/SuperApero/Entity/SuperAperoMeetup.php @@ -13,7 +13,7 @@ class SuperAperoMeetup #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\ManyToOne(targetEntity: SuperApero::class, inversedBy: 'meetups')] #[ORM\JoinColumn(nullable: false)] diff --git a/sources/AppBundle/SuperApero/Form/SuperAperoType.php b/sources/AppBundle/SuperApero/Form/SuperAperoType.php index 63b5eaaa4..1cfc35d85 100644 --- a/sources/AppBundle/SuperApero/Form/SuperAperoType.php +++ b/sources/AppBundle/SuperApero/Form/SuperAperoType.php @@ -57,23 +57,33 @@ public function buildForm(FormBuilderInterface $builder, array $options): void } }); - $builder->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event) use ($antennes): void { + $builder->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event): void { /** @var SuperApero $superApero */ $superApero = $event->getData(); - $form = $event->getForm(); - if (isset($superApero->date)) { - $year = $superApero->annee(); - $existing = $this->superAperoRepository->findOneByYear($year); + if (!isset($superApero->date)) { + return; + } - if ($existing !== null && $existing->id !== $superApero->id) { - $form->get('date')->addError( - new FormError("Un Super Apéro existe déjà pour l'année {$year}."), - ); - } + $year = $superApero->annee(); + $existing = $this->superAperoRepository->findOneByYear($year); + + if ($existing === null) { + return; + } + + if (!isset($superApero->id) || $existing->id !== $superApero->id) { + $event->getForm()->get('date')->addError( + new FormError("Un Super Apéro existe déjà pour l'année {$year}."), + ); } + }); + + $builder->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event) use ($antennes): void { + /** @var SuperApero $superApero */ + $superApero = $event->getData(); - $meetupsForm = $form->get('meetups'); + $meetupsForm = $event->getForm()->get('meetups'); $submittedAntennes = []; foreach ($antennes as $antenne) { diff --git a/sources/AppBundle/Veille/Entity/Envoi.php b/sources/AppBundle/Veille/Entity/Envoi.php index 2f2a9a1c9..39ca1fdc2 100644 --- a/sources/AppBundle/Veille/Entity/Envoi.php +++ b/sources/AppBundle/Veille/Entity/Envoi.php @@ -14,7 +14,7 @@ class Envoi #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(name: 'sending_date', type: 'datetime')] public \DateTimeInterface $dateEnvoi; diff --git a/sources/AppBundle/Veille/Entity/NewsletterDesinscription.php b/sources/AppBundle/Veille/Entity/NewsletterDesinscription.php index 9b2f39ba9..5e50c4094 100644 --- a/sources/AppBundle/Veille/Entity/NewsletterDesinscription.php +++ b/sources/AppBundle/Veille/Entity/NewsletterDesinscription.php @@ -13,7 +13,7 @@ class NewsletterDesinscription #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\Column(length: 255, nullable: true)] public ?string $email = null; diff --git a/sources/AppBundle/Veille/Entity/NewsletterInscription.php b/sources/AppBundle/Veille/Entity/NewsletterInscription.php index b9cd0da6b..1ab6cf95c 100644 --- a/sources/AppBundle/Veille/Entity/NewsletterInscription.php +++ b/sources/AppBundle/Veille/Entity/NewsletterInscription.php @@ -14,7 +14,7 @@ class NewsletterInscription #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - public ?int $id = null; + public int $id; #[ORM\ManyToOne(targetEntity: Utilisateur::class)] #[ORM\JoinColumn(name: 'user_id', referencedColumnName: 'id', nullable: true)] diff --git a/templates/admin/site/article_form.html.twig b/templates/admin/site/article_form.html.twig index d35de261f..e07eae19c 100644 --- a/templates/admin/site/article_form.html.twig +++ b/templates/admin/site/article_form.html.twig @@ -5,7 +5,7 @@ {% block content %}

{{ formTitle }}

- {% if article.slug != '-' %} + {% if article.id is defined %}