Page d'accueil Mes articles

Qualité du code

La qualité du code est quelque chose d'important surtout lorsqu'on travaille en équipe. L'idée est de vous présenter les outils que j'utilise pour gagner en lisibilité et réduire les incohérences.

Lors que l'on développe, il n'est pas rare de faire des erreurs. Une accolade mal placée qui ne respecte pas PSR-12, un typage incorrect ou encore un appel d'une méthode sur un objet potentiellement nul. Les erreurs possibles ne manquent pas. Heureusement, nous avons des outils pour tester la qualité de notre code avant d'envoyer notre travail sur notre dépôt Git.

php-cs-fixer, pour gagner en lisibilité

PHP CS pour Coding Standards. Lorsque l'on travaille à plusieurs sur un même projet, il est important de se fixer des règles si l'on ne veut pas que le code devienne illisible. En open source, les développeurs s'en sont fixés, les fameuses PHP Standard Recommendation, les PSR. Elles décrivent un ensemble de bonnes pratiques à adopter, un cadre à suivre, permettant au développeur de s'y retrouver plus facilement en passant d'un projet à l'autre. Si vous prenez Symfony par exemple, ils ont implémenté la PSR-6 pour l'usage basique de leur système de cache. Mais ce qui nous intéresse dans les PSRs, ce sont celles liées au style à savoir la PSR-1 et la PSR-12.

La PSR-12 par exemple nous interdit d'écrire ce genre de code car il est considéré pas assez lisible et source d'erreurs.

if ($foo)
    echo 'Hello World !';

Si l'on souhaite respecter la PSR-12, nous devons écrire notre code de cette façon

if ($foo) {
    echo 'Hello World !';
}

Vous imaginez bien que faire toutes ces modifications à la main dans votre code ne serait pas viable. Si vous récupérez le code d'un collègue mal indenté et sans accolades ça risque de vous énerver. C'est pourquoi le projet PHP-CS-Fixer peut vraiment vous intéresser. Il permet de corriger automatiquement votre code en se basant sur le style de codage que vous avez décidé avec votre équipe.

Personnellement lorsque je fais du Symfony j'aime bien suivre le style de code du framework. Il reprend les PSR et y ajoute quelques règles.

Dans un nouveau projet fraichement créé sur la version 5.3 du framework avec PHP 8.0, j'ai créé une entité et le CRUD associé. Étant donné que le CRUD est généré avec le composant maker, il ne devrait pas se passer grand chose avec php-cs-fixer.

Je lance donc la commande suivante en lui précisant le répertoire source src suivi de --dry-run ( pour ne pas modifier mes fichiers ) ainsi que --diff pour voir les corrections à faire.

❯ php-cs-fixer fix src --dry-run --diff
Loaded config default.
Using cache file ".php-cs-fixer.cache".

Checked all files in 0.001 seconds, 12.000 MB memory used

Aucun problème si je ne définis pas de règle particulières. Je vais donc relancer la commande en lui ajoutant la règle Symfony.

❯ php-cs-fixer fix src --dry-run --diff --rules=@Symfony

Et là, le résultat est différent.

-    #[Route('/{id}/edit', name: 'hello_edit', methods: ['GET','POST'])]
+    #[Route('/{id}/edit', name: 'hello_edit', methods: ['GET', 'POST'])]

En effet, dans les coding standards de Symfony, nous devons ajouter un espace après une virgule. Et lorsque mon make:crud m'a créé le controller, il ne l'a pas fait pour les tableaux de méthode.

Si vous souhaitez migrer sur une version plus récente de PHP, PHP-CS-Fixer peut également vous aider. Il est possible d'ajouter d'autres règles comme @PHP81Migration par exemple. Grâce aux règles de migration PHP, cet outil va vous permettre de corriger directement votre code pour répondre aux évolutions du langage. Par exemple, ajoutez la règle @PHP73Migration et l'outil vous proposera automatiquement une modification de code répondant par exemple à la RFC: Flexible Heredoc and Nowdoc Syntaxes.

Vous voulez un typage strict sur votre projet ?

❯ php-cs-fixer fix src --dry-run --diff --rules=@Symfony,@PHP80Migration:risky --allow-risky yes

Vous verrez que cela va vous ajouter le strict_types=1 sur tous vos fichiers. Alors bien sûr, c'est dangereux et c'est pour cela que l'on doit ajouter le --allow-risky à la commande. Mais vu que nous sommes en dev et que nous avons des tests fonctionnels, aucun risques 🤗

phpstan, pour détecter les bugs avant qu'ils ne surviennent

PHPStan est un outil d'analyse statique qui va nous permettre de détecter les incohérences dans notre code pouvant être source de bugs. C'est vraiment un outil que je trouve essentiel pour un bon contrôle qualité. On choisit de l'installer dans notre projet via composer en tant que dépendances de développement.

❯ composer require --dev phpstan/phpstan

Ensuite, on l'exécute via le binaire présent dans les vendors.

❯ vendor/bin/phpstan analyse src
 6/6 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


 [OK] No errors                                                                                                    

💡 Tip of the Day:
PHPStan is performing only the most basic checks.
You can pass a higher rule level through the --level option
(the default and current level is 0) to analyse code more thoroughly.

PHPStan fonctionne avec une notion de niveau. Par défaut c'est le niveau 0 qui est utilisé et qui s'occupe de problèmes basiques : classes inconnues, fonctions inconnues, méthodes inconnues appelées sur $this, etc...

Étant donné que j'ai utilisé uniquement des makers pour générer mon projet Symfony, il ne détecte aucun problèmes. Personnellement, j'aime bien monter jusqu'au niveau maximum lorsque c'est possible.

❯ vendor/bin/phpstan analyse -l 9 src
 6/6 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------------------------------------------------------- 
  Line   Controller/HelloController.php                                                                         
 ------ ------------------------------------------------------------------------------------------------------- 
  76     Parameter #2 $token of method                                                                          
         Symfony\Bundle\FrameworkBundle\Controller\AbstractController::isCsrfTokenValid() expects string|null,  
         bool|float|int|string|null given.                                                                      
 ------ ------------------------------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------- 
  Line   Entity/Hello.php                                               
 ------ --------------------------------------------------------------- 
  20     Property App\Entity\Hello::$id has no type specified.          
  20     Property App\Entity\Hello::$id is never written, only read.    
  25     Property App\Entity\Hello::$name has no type specified.          
 ------ --------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------- 
  Line   Repository/HelloRepository.php                                                                       
 ------ ----------------------------------------------------------------------------------------------------- 
  17     Class App\Repository\HelloRepository extends generic class                                           
         Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository but does not specify its types: T  
         💡 You can turn this off by setting checkGenericClassInNonGenericObjectType: false in your            
         phpstan.neon.                                                                                        
 ------ ----------------------------------------------------------------------------------------------------- 


 [ERROR] Found 6 errors                                                                                            

L'erreur sur le HelloController est assez simple à corriger. isCsrfTokenValid veut du string et seulement du string. De même pour les types non spécifiés sur Hello.php.

--- a/src/Controller/HelloController.php
+++ b/src/Controller/HelloController.php
-        if ($this->isCsrfTokenValid('delete'.$hello->getId(), $request->request->get('_token'))) {
+        if ($this->isCsrfTokenValid('delete'.$hello->getId(), (string)$request->request->get('_token'))) {

--- a/src/Entity/Hello.php
+++ b/src/Entity/Hello.php

-    private $id;
+    private ?int $id;

-    private $name;
+    private ?string $name;

Il ne me reste plus que deux erreurs ! Pour la dernière erreur, ma classe HelloRepository hérite d'une classe générique. J'ai donc deux solutions, soit je précise dans la PHPDoc au dessus de ma classe le type d'héritage, soit je mets checkGenericClassInNonGenericObjectType à false dans le fichier de configuration.

+ * @extends ServiceEntityRepository<HelloRepository>
  */
 class HelloRepository extends ServiceEntityRepository

Lorsque je relance mon analyse, il me dit qu'il attend en paramètre de mon constructeur class-string<App\Repository\HelloRepository>. Ce qui est faux. C'est juste qu'il est un peu perdu avec la réflexion de code 😅. On va l'aider à comprendre doctrine en installant des extensions !

❯ composer require --dev phpstan/extension-installer
❯ composer require --dev phpstan/phpstan-doctrine

Yeah ! Beaucoup mieux, plus qu'une seule erreur qui devrait normalement être gérée par phpstan-doctrine.

❯ vendor/bin/phpstan analyse -l 9 src
 5/5 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------------- 
  Line   Entity/Hello.php                                             
 ------ ------------------------------------------------------------- 
  18     Property App\Entity\Hello::$id is never written, only read.  
 ------ ------------------------------------------------------------- 


 [ERROR] Found 1 error                                                                                             

Après une petite recherche sur github effectivement une issue est ouverte sur le sujet. Temporairement, je vais l'exclure de mon analyse grâce à une expression rationnelle dans un fichier phpstan.neon que je vais créer à la racine de mon projet.

parameters:
        ignoreErrors:
                - '#Property .*::\$id is never written, only read.#'

Et maintenant... Victoire ! 🎉

❯ vendor/bin/phpstan analyse -l 9 src
Note: Using configuration file /Users/yoann/dev/hello_symfony/phpstan.neon.
 5/5 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%



 [OK] No errors                                                                                                    

Pour plus de simplicité et de façon à juste taper la commande vendor/bin/phpstan analyse sans plus de paramètre, je peux définir la source et le niveau d'analyse dans le fichier phpstan.neon.

parameters:
    level: 9
    paths:
        - src
    ignoreErrors:
        - '#Property .*::\$id is never written, only read.#'

Et voilà 😁 En complément vous pouvez bien sûr ajouter d'autres extensions comme phpstan/phpstan-deprecation-rules, phpstan/phpstan-symfony ou encore ekino/phpstan-banned-code qui vous évitera d'envoyer des dumps en production 😅

Un autre outil d'analyse statique en PHP qui monte bien, c'est psalm maintenu par Vimeo. Vous pouvez bien sûr l'utiliser en complément de PHPStan pour être ceinture et bretelles

PHP Insights, pour une vision globale de la qualité du code

Texte alternatif

Je trouve cet outil vraiment intéressant car ça permet d'avoir rapidement une image de la qualité du code. Pour l'installer, rien de plus simple.

❯ composer require nunomaduro/phpinsights --dev

Ensuite, on définit la configuration par défaut en se basant sur les règles de Symfony

❯ cp vendor/nunomaduro/phpinsights/stubs/symfony.php phpinsights.php

Et pour finir, on lance la commande suivante

❯ ./vendor/bin/phpinsights

Si vous êtes sur un projet tout neuf comme moi, vous devriez n'avoir que du vert. Ce qui est intéressant ensuite, c'est que lorsque l'on tape sur la touche Entrée, on voit le détail des modifications à apporter à notre code.

• [Style] Line length:
  src/Kernel.php:19: Line exceeds 80 characters; contains 81 characters
  src/Kernel.php:23: Line exceeds 80 characters; contains 83 characters
  src/Repository/HelloRepository.php:13: Line exceeds 80 characters; contains 99 characters
  +7 issues omitted

Par exemple, je peux considérer qu'une limite de 80 caractères à la ligne, c'est peu. Personnellement, je développe sur un 13 pouces, et 120 caractères je trouve ça pas mal ! Pour faire ça, c'est très simple. Il suffit d'aller sur la documentation et chercher Line length. Je rajoute donc le code suivant à mon fichier phpinsights.php à la racine de mon projet.

    'config' => [
        \PHP_CodeSniffer\Standards\Generic\Sniffs\Files\LineLengthSniff::class => [
            'lineLimit' => 120,
            'absoluteLineLimit' => 140,
            'ignoreComments' => false,
        ]
    ],

Notre pourcentage de réussite sur le style a augmenté ! Autre chose très intéressante sur phpinsights, c'est que vous avez la possibilité de définir des requirements. Autrement dit, si on est en dessous par exemple d'un certain seuil pour le style, et bien cela vous générera une erreur.

[ERROR] The style score is too low

Ça demande un peu de configuration en début de projet, mais je trouve cet outil vraiment sympa à utiliser. Il ne vous dit pas, c'est OK ou c'est KO. L'analyse statique est plus complexe que ça. Vous pouvez très bien avoir une méthode qui dépasse un certain nombre de lignes mais qui coûterait trop chère à refactoriser. Donc on accepte la remarque et on continue notre projet. Ce qui va nous intéresser c'est l'indicateur, le pourcentage de code correct. Si on se retrouve à 60% on peut se dire que le projet prend une mauvaise direction. Bien sûr, l'idée n'est pas de désactiver toutes les règles pour avoir du vert dans chaque colonne, ça c'est tricher 😜

Makefile, pour ne pas taper ces trois commandes à chaque fois

Si on veut prendre l'habitude d'utiliser ces trois outils fréquement, ça doit être simple à utiliser. Pour ça, j'utilise un Makefile à la raçine de mon projet. Pour les utilisateurs de Windows, il est possible d'avoir la commande make dont je vais parler ensuite en l'installant via chocolatey.

Je crée donc un fichier Makefile à la racine de mon projet dans lequel je mets les instructions suivantes

quality:
    php-cs-fixer fix src --rules=@Symfony,@PHP80Migration:risky --allow-risky yes
    ./vendor/bin/phpstan analyse
    ./vendor/bin/phpinsights

Ensuite, exécutez la commande make quality ( ou juste make puisqu'on a qu'une seule instruction dans notre Makefile ). La première instruction va potentiellement modifier le code, je veille donc à toujours faire un premier commit avant de lancer mon make surtout lorsque php-cs-fixer est configuré en risky 😬

PHPStan et Phpinsights, quand à eux, se mettront en échec si le code ne répond pas aux attentes.

On pourrait même configurer un hook sur notre commit pour lancer le make à chaque commit... Mais il y a mieux que ça !

Grumphp, contrôle votre nouveau code à chaque commit

Grumphp

Grumphp est le dernier outil dont je voulais vous parler dans cet article. Il va nous permettre de faire une analyse statique rapide de notre nouveau code avant de le commiter sur git, parfait si on code un peu rapidement et que l'on oublie de lancer notre make.

On peut l'installer au sein de notre projet.

❯ composer require --dev phpro/grumphp

Lorsqu'il vous demandera si vous voulez créer un fichier grumphp.yml, répondez Yes. Ensuite, l'outil va vous demander quelles tâches voulez-vous confier à Grumphp. Saisissez les numéros correspondant à securitychecker_symfony, phpstan et phpcsfixer2.

Cela va vous générer ce fichier grumphp.yml à la racine de votre site.

grumphp:
    tasks: { phpstan: null, securitychecker_symfony: null, phpcsfixer2: null }

La seule chose qu'il nous reste à faire est de définir les paramètres de la commande php-cs-fixer directement dans un fichier de configuration.

A la racine de votre projet, ajouter le fichier .php-cs-fixer.dist.php suivant

<?php

$finder = (new PhpCsFixer\Finder())
    ->in('src')
;

return (new PhpCsFixer\Config())
    ->setRules([
        '@Symfony' => true,
        '@PHP80Migration:risky' => true,
    ])
    ->setRiskyAllowed(true)
    ->setFinder($finder)
;

Modifier ensuite le fichier Makefile pour raccourcir la commande en php-cs-fixer fix seulement.

Et pour terminer, modifions le fichier grumphp.yml

grumphp:
    tasks:
        phpstan: null
        securitychecker_symfony: null
        phpcsfixer2:
            config: '.php-cs-fixer.dist.php'

Et le tour est joué 🤩

Grumphp succes

Et vous ? Qu'utilisez-vous pour améliorer la qualité de votre code ? N'hésitez pas à me le dire dans les commentaires 💬

Cet article vous a été utile ? Faites le connaître sur les réseaux sociaux Twitter twitter LinkedIn linkedin

Un commentaire ?

codeur
Ou connectez-vous avec GitHub