Simplifications de code pour php >= 7.4 #4990

Closed
opened 5 days ago by marcimat · 10 comments
marcimat commented 5 days ago
Owner

SPIP 4.1 nécessite php 7.4 minimum. À ce titre, on peut utiliser certains sucres syntaxiques de PHP, notamment ??= et ?? ou encore ?: à différents endroits du code.

L’outil Rector https://github.com/rectorphp/rector permet de faire cela (et d’autres choses) presque automatiquement : une relecture est nécessaire cependant, car

  • dès fois il peut créer des expressions peu pertinentes.
  • dès fois il comprend les types des variables lorsqu’elles sont initialisées (dans un if par exemple) et va les caster en array ou string si besoin si elles sont redéclarées en dehors de la première initialisation : mais dans notre code on réécrit parfois la variable en passant de array à string ou inversement, donc il faut surveiller.
  • il ajoute en tête de fonction toute variable utilisée qui a pu parfois ne pas été initialisée (en la déclarant null) : donc ne pas s’inquiéter de quelques ajouts de code

Enfin il remplace certains éléments, notamment dans les count($x) s’il ne connait pas le type de $x, il ajoutera systématiquement un (is_countable($x) ? count($x) : 0) et d’autres choses comme ça.

SPIP 4.1 nécessite php 7.4 minimum. À ce titre, on peut utiliser certains sucres syntaxiques de PHP, notamment `??=` et `??` ou encore `?:` à différents endroits du code. L’outil Rector https://github.com/rectorphp/rector permet de faire cela (et d’autres choses) presque automatiquement : une relecture est nécessaire cependant, car - dès fois il peut créer des expressions peu pertinentes. - dès fois il comprend les types des variables lorsqu’elles sont initialisées (dans un if par exemple) et va les caster en `array` ou `string` si besoin si elles sont redéclarées en dehors de la première initialisation : mais dans notre code on réécrit parfois la variable en passant de array à string ou inversement, donc il faut surveiller. - il ajoute en tête de fonction toute variable utilisée qui a pu parfois ne pas été initialisée (en la déclarant null) : donc ne pas s’inquiéter de quelques ajouts de code Enfin il remplace certains éléments, notamment dans les `count($x)` s’il ne connait pas le type de `$x`, il ajoutera systématiquement un `(is_countable($x) ? count($x) : 0)` et d’autres choses comme ça.
Poster
Owner

Il faudra surveiller les json_encode et json_decode car Rector ajoute systématiquement dessus JSON_THROW_ON_ERROR pour ne pas cacher les erreurs, mais nous n’avons pas de try / catch il me semble.

À surveiller surtout sur le json_decode() des sources externes (iterateur inc_json_to_array_dist)

Il faudra surveiller les `json_encode` et `json_decode` car Rector ajoute systématiquement dessus `JSON_THROW_ON_ERROR` pour ne pas cacher les erreurs, mais nous n’avons pas de try / catch il me semble. À surveiller surtout sur le json_decode() des sources externes (iterateur inc_json_to_array_dist)
marcimat added this to the 4.1 milestone 5 days ago
Poster
Owner
  • Pour le json_decode, j’ai ajouté un try/catch.
  • Il avait créé des erreurs aussi en tentant de simplifier des reset($array); $key = key($array) par $key = array_key_first($array); mais parfois on veut effectivement que ça remonte le pointeur de tableau aussi… J’ai corrigé après coup.
- Pour le json_decode, j’ai ajouté un try/catch. - Il avait créé des erreurs aussi en tentant de simplifier des `reset($array); $key = key($array)` par `$key = array_key_first($array);` mais parfois on veut effectivement que ça remonte le pointeur de tableau aussi… J’ai corrigé après coup.
b_b commented 4 days ago
Owner

À surveiller surtout sur le json_decode() des sources externes (iterateur inc_json_to_array_dist)

Il faudra donc tester intensivement les itérateurs JSON sur cette branche, c'est bien ça ?

> À surveiller surtout sur le json_decode() des sources externes (iterateur inc_json_to_array_dist) Il faudra donc tester intensivement les itérateurs JSON sur cette branche, c'est bien ça ?
Poster
Owner

Ah mais je l’ai ajouté le try / catch sur l’itérateur du coup.

D’ailleurs (rien à voir là mais j’y pense) il faudra faire un ticket ou une correction mais j’ai vu plein de spip_log mal formées (ça m’étonne pas vu la syntaxe) avec des

  • spip_log('texte', 'module' . _LOG_XX) au lieu de :
  • spip_log('texte', 'module.' . _LOG_XX)
Ah mais je l’ai ajouté le try / catch sur l’itérateur du coup. D’ailleurs (rien à voir là mais j’y pense) il faudra faire un ticket ou une correction mais j’ai vu **plein** de spip_log mal formées (ça m’étonne pas vu la syntaxe) avec des - `spip_log('texte', 'module' . _LOG_XX)` au lieu de : - `spip_log('texte', 'module.' . _LOG_XX)`
Poster
Owner

Appliqué pour le core par #4991
En cours sur les plugins-dist.

Appliqué pour le core par #4991 En cours sur les plugins-dist.
Poster
Owner

Ah mince, les constantes sont sensibles à la casse, et Rector en a changé 2 il me semble, qui avait une partie de leur casse en minuscule, et les a passé en majuscule… faut que je les retrouve, pour remettre comme c’était (mélange majuscule, minuscule hum…) si on veut pas casser l’existant d’avant…

Sait on jamais…

Ah mince, les constantes sont sensibles à la casse, et Rector en a changé 2 il me semble, qui avait une partie de leur casse en minuscule, et les a passé en majuscule… faut que je les retrouve, pour remettre comme c’était (mélange majuscule, minuscule hum…) si on veut pas casser l’existant d’avant… Sait on jamais…
Poster
Owner

Voilà, j’ai remis avec 1b45a203 des constantes modifiées.

Je laisse SQL_ABSTRACT_VERSION par contre qui est interne.

Voilà, j’ai remis avec 1b45a203 des constantes modifiées. Je laisse `SQL_ABSTRACT_VERSION` par contre qui est interne.
b_b commented 4 days ago
Owner

Merci pour le fix sur les constantes, ça serait peut-être un truc à corriger dans le futur car dans nos règles SCS1 on indique bien qu'elles doivent être en majuscules cf https://www.spip.net/fr_article6677.html#Constantes-Variables-et-proprietes-de-classe

Merci pour le fix sur les constantes, ça serait peut-être un truc à corriger dans le futur car dans nos règles SCS1 on indique bien qu'elles doivent être en majuscules cf https://www.spip.net/fr_article6677.html#Constantes-Variables-et-proprietes-de-classe
b_b added the
amélioration
label 4 days ago
Poster
Owner

J’ai fait les plugins core maintenus aussi.
Je pense qu’on peut fermer le ticket du coup.

J’ai fait les plugins core maintenus aussi. Je pense qu’on peut fermer le ticket du coup.
b_b commented 4 days ago
Owner

\o/

\o/
b_b closed this issue 4 days ago
Sign in to join this conversation.
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.