Skip to content

fix(deprecations): PHP 8.2 ${var} & 8.4 implicit-nullable params (#648)#649

Open
hschoenenberger wants to merge 1 commit into
mainfrom
fix/gh-648-php-deprecations
Open

fix(deprecations): PHP 8.2 ${var} & 8.4 implicit-nullable params (#648)#649
hschoenenberger wants to merge 1 commit into
mainfrom
fix/gh-648-php-deprecations

Conversation

@hschoenenberger

Copy link
Copy Markdown
Contributor

Closes #648

Contexte

L'issue #648 remonte deux familles de deprecations PHP sur PS 9.1.4 / PHP 8.3+ :

  1. ${var} dans les chaînes — deprecated PHP 8.2, FATAL en PHP 9.0 — dans segmentio/analytics-php.
  2. Paramètres implicitement nullables (Type $x = null) — deprecated PHP 8.4 — dans notre code src/.

Contrainte structurante : le module ships un seul codebase de PHP 5.6 à 8.3. Le fix « officiel » proposé dans l'issue (?Type $x = null) est inapplicable : la syntaxe ?Type est PHP 7.1+ et casse le plancher 5.6 vérifié en CI. Les deps sont aussi épinglées vieilles volontairement (majeures suivantes ≥ PHP 7.2/7.4), donc pas de montée de version (vérifié : aucune 1.x de segmentio ne corrige ${var}).

Modifications

1. ${var}{$var} (segmentio) — Makefile

segmentio/analytics-php est une lib non scopée (psr0) embarquée telle quelle et régénérée par composer → patch au build : nouvelle cible vendor-patch-segmentio qui réécrit ${var} en {$var}, branchée dans la chaîne php-scoper.

⚠️ Le patch communautaire de l'issue propose {$$type} pour Segment.php:112 — c'est un bug (variable-variable). La regex ici produit le correct {$type}.

2. Paramètres nullables PHP 8.4 dans src/

Règle : jamais ?Type (5.6). On retire le type-hint natif et on conserve le @param Type|null en docblock (DI vérifiée explicite, aucune résolution par réflexion) :

  • ConfigurationRepository::__construct, Adapter\Link::__construct, StatusManager::cacheInvalidated/cacheExpired — constructeurs/méthodes sans parent → détypage sûr.
  • ShopSession::getValidToken ⚠️ (zone restreinte auth/session) : c'est un override. Détyper créerait un warning de variance en PHP < 7.2. On aligne donc la signature sur le parent (array $scope = [], array $audience = []) + sentinelle empty(). Signature désormais identique au parent/interface → aucune variance sur aucune version PHP. Vérifié : aucun appelant ne passe de scope/audience explicite → comportement inchangé.

Hors périmètre

Patches nullables 8.4 sur lcobucci/jwt et ramsey/uuid — au-delà du support officiel (8.3) et nécessiteraient un mécanisme de patch vendor (revue §10).

Vérifications

  • Lint des fichiers modifiés + segmentio patché : ✅ ; aucune syntaxe PHP 7+ introduite
  • sed segmentio testé sur copies (${type}{$type}) : ✅ + lint clean
  • Tests GetValidToken (OAuth2 + Firebase) : ✅ 14/14, 32 assertions
  • Tests StatusManager / ConfigurationRepository / Link : ✅ aucun échec
  • PHPStan : ✅ 0 nouvelle erreur (l'unique erreur restante est pré-existante, fichier non touché)

⚠️ La modif de ShopSession touche une zone restreinte (auth/session, §5) → revue obligatoire.

🤖 Generated with Claude Code

)

Two families of PHP deprecations reported on PS 9.1.4 / PHP 8.3+:

1. "${var}" string interpolation in segmentio/analytics-php (deprecated
   PHP 8.2, FATAL in PHP 9.0). The lib is a non-scoped psr0 dependency
   shipped as-is and regenerated by composer, so it is patched at build
   time: new Makefile target vendor-patch-segmentio rewrites ${var} into
   {$var}, wired into the php-scoper chain.

2. Implicitly-nullable parameters (Type $x = null, deprecated PHP 8.4) in
   our own src/. The issue's suggested ?Type fix is rejected: it breaks the
   PHP 5.6 floor enforced in CI. Instead we drop the native type hint and
   keep the @param Type|null docblock (ConfigurationRepository, Adapter\Link,
   StatusManager). ShopSession::getValidToken is an override, so rather than
   widening the parameter type (which warns pre-7.2) its signature is aligned
   with the parent (array $scope = []) using an empty() sentinel; verified no
   caller passes an explicit scope/audience, so behavior is unchanged.

Out of scope: 8.4 implicit-nullable patches for lcobucci/jwt and ramsey/uuid
(beyond the 8.3 support boundary; would need a vendor patch mechanism).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@hschoenenberger
hschoenenberger requested a review from a team as a code owner July 17, 2026 13:33
@sonarqubecloud

Copy link
Copy Markdown

@hschoenenberger hschoenenberger added ready to review need-claude-review Trigger Claude automated PR review labels Jul 17, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Claude

Verdict : ⚠️ A corriger

Scope : Corrections de dépréciations PHP 8.2/8.4 — ${var} interpolation (segmentio vendor, Makefile), implicit-nullable type hints (ShopSession, StatusManager, Link, ConfigurationRepository)


[MAJEUR] Zone restreinte modifiée — review humaine obligatoire

Fichier : src/Account/Session/ShopSession.php:86

src/Account/Session/ fait partie des zones restreintes du projet (CLAUDE.md §5 et §10) : "auth/sessions : any modification requires mandatory review". Toute modification de cette zone doit être validée explicitement par un membre humain de l'équipe, indépendamment de la justification technique.

Le changement lui-même est correct et justifié :

  • Il aligne la signature de ShopSession::getValidToken() sur celle déjà en place dans SessionInterface et Session (parent), qui utilisent déjà array $scope = [], array $audience = [].
  • Tous les appelants connus (StatusManager, FirebaseSession, PsAccountsService, handlers CQRS) n'utilisent que les valeurs par défaut — la substitution empty($scope) à $scope === null est donc sans impact comportemental pour ces callers.

Ce point n'est pas un blocage technique mais la procédure de zone restreinte doit être respectée.


[MINEUR] Aucun test unitaire pour ShopSession

Fichier : tests/src/Unit/Account/Session/ (absent)

ShopSessionTest.php n'existe pas. La méthode getValidToken() a été modifiée dans une zone restreinte (comportement de la logique de scope par défaut). Un test couvrant au moins les cas scope vide → defaults appliqués et scope non vide → passé tel quel renforcerait la non-régression.


Resume

Niveau Nombre
CRITIQUE 0
MAJEUR 1
MINEUR 1
SUGGESTION 0

Pour demander une re-review après corrections, re-ajoutez le label need-claude-review.

@github-actions github-actions Bot removed the need-claude-review Trigger Claude automated PR review label Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PS 9.1.4, PHP 8.3+list of deprecated scripts/ functions

1 participant