Faisons mumuse avec PHPStan #3

Open
opened 1 year ago by bricebou · 8 comments
bricebou commented 1 year ago
Collaborator

Je me suis essayé à PHPStan et aimerais avoir le retour de personnes plus aguerries, telles @b_b, @Eric, @marcimat... C'est donc sur la branche suivante: https://git.spip.net/spip-contrib-extensions/optimages/src/branch/phpstan
[EDIT] C'est désormais dans le commit a50e5814fe.[/EDIT]

J'ai donc mené les opérations suivantes:

  • on commence par créer un fichier phpstan.neon.dist à la racine de notre plugin et on y insère ceci:
includes:
	- phpstan-baseline.neon

parameters:
    paths:
        - .
    excludePaths:
        analyseAndScan:
            - lang
            - vendor
    level: 0
  • on lance la commande suivante depuis la racine du SPIP
vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist --generate-baseline=plugins/optimages/phpstan-baseline.neon
  • on corrige les éventuelles erreurs qui remontent et on relance alors PHPStan ainsi:
vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist
  • ensuite on peut monter progressivement le niveau d'analyse en ajoutant l'argument --level=[0-8]

  • une fois toutes les erreurs à notre portée corrigées, on peut relancer la génération de la baseline en ayant au préalable modifiée la valeur pour le paramètre level de notre fichier de configuration.

Merci par avance pour vos retours :-)

Je me suis essayé à PHPStan et aimerais avoir le retour de personnes plus aguerries, telles @b_b, @Eric, @marcimat... ~~C'est donc sur la branche suivante: https://git.spip.net/spip-contrib-extensions/optimages/src/branch/phpstan~~ [EDIT] C'est désormais dans le commit https://git.spip.net/spip-contrib-extensions/optimages/commit/a50e5814fe12b1326be7c2b16749389889de9f81.[/EDIT] J'ai donc mené les opérations suivantes: * on commence par créer un fichier `phpstan.neon.dist` à la racine de notre plugin et on y insère ceci: ``` includes: - phpstan-baseline.neon parameters: paths: - . excludePaths: analyseAndScan: - lang - vendor level: 0 ``` * on lance la commande suivante depuis la racine du SPIP ``` vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist --generate-baseline=plugins/optimages/phpstan-baseline.neon ``` * on corrige les éventuelles erreurs qui remontent et on relance alors PHPStan ainsi: ``` vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist ``` * ensuite on peut monter progressivement le niveau d'analyse en ajoutant l'argument --level=[0-8] * une fois toutes les erreurs à notre portée corrigées, on peut relancer la génération de la baseline en ayant au préalable modifiée la valeur pour le paramètre `level` de notre fichier de configuration. Merci par avance pour vos retours :-)
b_b commented 1 year ago
Collaborator

C'est parfait à mon humble niveau de connaissance de l'outil :)

C'est parfait à mon humble niveau de connaissance de l'outil :)
Owner
  • on commence par créer un fichier phpstan.neon.dist à la racine de notre plugin et on y insère ceci:

Ouais, mais alors dans tous les plugins on va avoir le même fichier de configuration. Y a pas moyen de faire autrement, un fichier global un peu comme dans git ?

  • on lance la commande suivante depuis la racine du SPIP
vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist --generate-baseline=plugins/optimages/phpstan-baseline.neon

Alors la je ne pige pas pourquoi on crée déjà une baseline puisque si j'ai bien compris la baseline sert à ne plus détecter des erreurs qu'on considère ne pas pouvoir ou devoir corriger ?

  • ensuite on peut monter progressivement le niveau d'analyse en ajoutant l'argument --level=[0-8]

Pouvons-nous ou avons-nous intérêt à définir un niveau minimum pour SPIP ? Par exemple, le niveau 6. Je ne comprends pas d'ailleurs pourquoi on met 0 dans le fichier de configuration.

Corollaire : ce faisant on va se retrouver avec un dossier vendor/ dans chaque plugin phpstané non ?

Corollaire 2 : dans PHPstorm il est possible de lancer des inspections avec PHPstan. Je l'utilise au niveau 6 mais j'ai quelques soucis pour éviter des erreurs. Je suppose que VSCode peut le faire aussi. Ca serait bien de voir si on peut faire un tuto pour ces éditeurs de façon aussi à inciter à leur utilisation.

> * on commence par créer un fichier `phpstan.neon.dist` à la racine de notre plugin et on y insère ceci: Ouais, mais alors dans tous les plugins on va avoir le même fichier de configuration. Y a pas moyen de faire autrement, un fichier global un peu comme dans git ? > * on lance la commande suivante depuis la racine du SPIP > ``` > vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist --generate-baseline=plugins/optimages/phpstan-baseline.neon > ``` Alors la je ne pige pas pourquoi on crée déjà une baseline puisque si j'ai bien compris la baseline sert à ne plus détecter des erreurs qu'on considère ne pas pouvoir ou devoir corriger ? > * ensuite on peut monter progressivement le niveau d'analyse en ajoutant l'argument --level=[0-8] Pouvons-nous ou avons-nous intérêt à définir un niveau minimum pour SPIP ? Par exemple, le niveau 6. Je ne comprends pas d'ailleurs pourquoi on met 0 dans le fichier de configuration. Corollaire : ce faisant on va se retrouver avec un dossier vendor/ dans chaque plugin phpstané non ? Corollaire 2 : dans PHPstorm il est possible de lancer des inspections avec PHPstan. Je l'utilise au niveau 6 mais j'ai quelques soucis pour éviter des erreurs. Je suppose que VSCode peut le faire aussi. Ca serait bien de voir si on peut faire un tuto pour ces éditeurs de façon aussi à inciter à leur utilisation.
Poster
Collaborator

[snip]

  • on lance la commande suivante depuis la racine du SPIP
vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist --generate-baseline=plugins/optimages/phpstan-baseline.neon

Alors la je ne pige pas pourquoi on crée déjà une baseline puisque si j'ai bien compris la baseline sert à ne plus détecter des erreurs qu'on considère ne pas pouvoir ou devoir corriger ?

  • ensuite on peut monter progressivement le niveau d'analyse en ajoutant l'argument --level=[0-8]

Pouvons-nous ou avons-nous intérêt à définir un niveau minimum pour SPIP ? Par exemple, le niveau 6. Je ne comprends pas d'ailleurs pourquoi on met 0 dans le fichier de configuration.

Alors, je ne sais pas trop, j'ai suivi les quelques ressources trouvées ici ou là dans la galaxie SPIP... J'ai cru comprendre qu'on commençait au niveau 0 pour monter progressivement : le niveau 0 remonte de nombreuses erreurs liées à des fonctions de SPIP que PHPStan ne trouve pas, ce qui est à ce jour normal. Du coup, la baseline permet ensuite de ne pas faire remonter ces erreurs lorsque l'on monte de niveau.

Corollaire : ce faisant on va se retrouver avec un dossier vendor/ dans chaque plugin phpstané non ?

Logiquement non, j'ai fait des erreurs au début de mon utilisation, en faisant à la racine de chaque plugin un composer require... mais en fait, phpcs et phpstan sont disponibles à la racine de SPIP dans vendor... Donc, non, pas nécessairement de vendor/ du moins avec seulement ces outils.

[snip] > > * on lance la commande suivante depuis la racine du SPIP > > ``` > > vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist --generate-baseline=plugins/optimages/phpstan-baseline.neon > > ``` > > Alors la je ne pige pas pourquoi on crée déjà une baseline puisque si j'ai bien compris la baseline sert à ne plus détecter des erreurs qu'on considère ne pas pouvoir ou devoir corriger ? > > > * ensuite on peut monter progressivement le niveau d'analyse en ajoutant l'argument --level=[0-8] > > Pouvons-nous ou avons-nous intérêt à définir un niveau minimum pour SPIP ? Par exemple, le niveau 6. Je ne comprends pas d'ailleurs pourquoi on met 0 dans le fichier de configuration. Alors, je ne sais pas trop, j'ai suivi les quelques ressources trouvées ici ou là dans la galaxie SPIP... J'ai cru comprendre qu'on commençait au niveau 0 pour monter progressivement : le niveau 0 remonte de nombreuses erreurs liées à des fonctions de SPIP que PHPStan ne trouve pas, ce qui est à ce jour normal. Du coup, la baseline permet ensuite de ne pas faire remonter ces erreurs lorsque l'on monte de niveau. > > Corollaire : ce faisant on va se retrouver avec un dossier vendor/ dans chaque plugin phpstané non ? > Logiquement non, j'ai fait des erreurs au début de mon utilisation, en faisant à la racine de chaque plugin un composer require... mais en fait, phpcs et phpstan sont disponibles à la racine de SPIP dans vendor... Donc, non, pas nécessairement de vendor/ du moins avec seulement ces outils.
Collaborator
  • on commence par créer un fichier phpstan.neon.dist à la racine de notre plugin et on y insère ceci:

Ouais, mais alors dans tous les plugins on va avoir le même fichier de configuration. Y a pas moyen de faire autrement, un fichier global un peu comme dans git ?

Non, ça n'est pas le même fichier, exemple dans GIS https://git.spip.net/spip-contrib-extensions/gis/src/branch/master/phpstan.neon.dist

Chaque plugin pourra bien sûr exlcure le répertoire lang, mais ausis certaines classes embarquées ou des libs.

Alors la je ne pige pas pourquoi on crée déjà une baseline puisque si j'ai bien compris la baseline sert à ne plus détecter des erreurs qu'on considère ne pas pouvoir ou devoir corriger ?

C'est ce qu'on a fait avec le core, et j'ai fait pareil dans GIS :)

Le principe est de créer une baseline, puis de corriger tout ce qu'elle remonte et qu'on peut bien corriger pour n'y laisser queles "faux positifs" comme "Function autoriser not found" cf https://git.spip.net/spip-contrib-extensions/gis/src/branch/master/phpstan-baseline.neon#L9

Ainsi, on a une baseline qui ne contient que des alertes sur des problèmes dus "à la manière dont SPIP est codé" pour le dire vite.

Ensuite, on sait qu'on peut relancer l'outil et qu'il ne remontera que les nouvelles erreurs introduites suite à ces premières étapes.

Pouvons-nous ou avons-nous intérêt à définir un niveau minimum pour SPIP ? Par exemple, le niveau 6. Je ne comprends pas d'ailleurs pourquoi on met 0 dans le fichier de configuration.

Ça c'est un débat propre à SPIP, et on en a un peu causé ici https://discuter.spip.net/t/analyse-statique-du-code/155403/10

Corollaire : ce faisant on va se retrouver avec un dossier vendor/ dans chaque plugin phpstané non ?

Pas du tout :)

> > * on commence par créer un fichier `phpstan.neon.dist` à la racine de notre plugin et on y insère ceci: > > Ouais, mais alors dans tous les plugins on va avoir le même fichier de configuration. Y a pas moyen de faire autrement, un fichier global un peu comme dans git ? Non, ça n'est pas le même fichier, exemple dans GIS https://git.spip.net/spip-contrib-extensions/gis/src/branch/master/phpstan.neon.dist Chaque plugin pourra bien sûr exlcure le répertoire lang, mais ausis certaines classes embarquées ou des libs. > > Alors la je ne pige pas pourquoi on crée déjà une baseline puisque si j'ai bien compris la baseline sert à ne plus détecter des erreurs qu'on considère ne pas pouvoir ou devoir corriger ? > C'est ce qu'on a fait avec le core, et j'ai fait pareil dans GIS :) Le principe est de créer une baseline, puis de corriger tout ce qu'elle remonte et qu'on peut bien corriger pour n'y laisser queles "faux positifs" comme "Function autoriser not found" cf https://git.spip.net/spip-contrib-extensions/gis/src/branch/master/phpstan-baseline.neon#L9 Ainsi, on a une baseline qui ne contient que des alertes sur des problèmes dus "à la manière dont SPIP est codé" pour le dire vite. Ensuite, on sait qu'on peut relancer l'outil et qu'il ne remontera que les nouvelles erreurs introduites suite à ces premières étapes. > > Pouvons-nous ou avons-nous intérêt à définir un niveau minimum pour SPIP ? Par exemple, le niveau 6. Je ne comprends pas d'ailleurs pourquoi on met 0 dans le fichier de configuration. > Ça c'est un débat propre à SPIP, et on en a un peu causé ici https://discuter.spip.net/t/analyse-statique-du-code/155403/10 > Corollaire : ce faisant on va se retrouver avec un dossier vendor/ dans chaque plugin phpstané non ? > Pas du tout :)
Owner
  • on lance la commande suivante depuis la racine du SPIP
    vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist --generate-baseline=plugins/optimages/phpstan-baseline.neon

Alors non, on ne peut pas lancer cette commande si la config inclut déjà le fichier baseline car la première fois il ne le trouve pas.

Donc il faut un fichier de config sans includes et apres le rajouter. En tout cas, chez moi c'est comme ça que j'ai réussi à le faire fonctionner.

> * on lance la commande suivante depuis la racine du SPIP > vendor/bin/phpstan --configuration=plugins/optimages/phpstan.neon.dist --generate-baseline=plugins/optimages/phpstan-baseline.neon Alors non, on ne peut pas lancer cette commande si la config inclut déjà le fichier baseline car la première fois il ne le trouve pas. Donc il faut un fichier de config sans includes et apres le rajouter. En tout cas, chez moi c'est comme ça que j'ai réussi à le faire fonctionner.
Collaborator

Alors non, on ne peut pas lancer cette commande si la config inclut déjà le fichier baseline car la première fois il ne le trouve pas.

Et si on peut, je viens de le faire dans gis_geometries avec ce phpstan.neon.dist :

includes:
	- phpstan-baseline.neon

parameters:
    paths:
        - .
    excludePaths:
        analyseAndScan:
            - lang
        analyse:
            - inc/geoPHP_Simplify.php
    level: 0

Et ça passe très bien :

bb@tybook:~/sites/trunk$ vendor/bin/phpstan --configuration=plugins/gis_geometries/phpstan.neon.dist --generate-baseline=plugins/gis_geometries/phpstan-baseline.neon
 4/4 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


                                                                                                                        
 [OK] Baseline generated with 93 errors.                                                                                
> Alors non, on ne peut pas lancer cette commande si la config inclut déjà le fichier baseline car la première fois il ne le trouve pas. Et si on peut, je viens de le faire dans gis_geometries avec ce `phpstan.neon.dist` : ```yaml includes: - phpstan-baseline.neon parameters: paths: - . excludePaths: analyseAndScan: - lang analyse: - inc/geoPHP_Simplify.php level: 0 ``` Et ça passe très bien : ``` bb@tybook:~/sites/trunk$ vendor/bin/phpstan --configuration=plugins/gis_geometries/phpstan.neon.dist --generate-baseline=plugins/gis_geometries/phpstan-baseline.neon 4/4 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100% [OK] Baseline generated with 93 errors. ```
Owner

J'ai l'erreur suivante systématique sur les paramètres de type classe comme ici:

message: "#^Parameter \$boucle of function archobjet_post_boucle\(\) has invalid type Boucle\.$#"

Est-ce que ça peut être corrigé ou pas ?

J'ai l'erreur suivante systématique sur les paramètres de type classe comme ici: > message: "#^Parameter \\$boucle of function archobjet_post_boucle\\(\\) has invalid type Boucle\\.$#" Est-ce que ça peut être corrigé ou pas ?
Collaborator

@Eric je pense que le meilleur endroit pour en causer serait sur discuter.spip :)

@Eric je pense que le meilleur endroit pour en causer serait sur discuter.spip :)
Sign in to join this conversation.
No Label
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.