Qualité du code et analyse statique : Error Prone, SpotBugs, PMD et SonarQube

Les tests vous disent si le code a fait ce qu'il fallait sur les entrées que vous avez choisies. L'analyse statique lit le code sans l'exécuter et raisonne sur tous les chemins, y compris ceux qu'aucun test n'a jamais empruntés. Cette différence résume à elle seule l'intérêt d'utiliser les deux : une suite de tests avec une excellente couverture peut très bien laisser passer un flux jamais fermé sur le chemin d'exception, un equals appelé sur deux tableaux, une chaîne de format avec le mauvais nombre de marqueurs, ou une comparaison d'entiers encapsulés avec == qui fonctionne jusqu'à 127 et échoue à 128.

L'autre raison de lancer une analyse est économique. Une anomalie remontée par un outil ne coûte rien à détecter, arrive quelques secondes après l'écriture du code, et n'exige de personne qu'il juge utile d'en faire un commentaire de revue. Le temps de revue humaine est rare ; il doit servir à vérifier que la conception est la bonne, pas qu'une ressource ne fuit pas. C'est exactement l'argument que développe le guide des conventions de code pour automatiser la mise en forme.

Ce que l'analyse voit, et ce qu'elle ne peut pas voir

Les analyseurs travaillent sur l'arbre syntaxique abstrait, le graphe de flot de contrôle et, pour les plus puissants, une approximation du flot de données. Cela leur donne des capacités qu'aucun test ne possède : ils peuvent examiner une branche qui ne s'est jamais exécutée, prouver qu'un null peut atteindre un déréférencement quelle que soit l'entrée, ou suivre des données utilisateur non fiables depuis un paramètre de requête jusqu'à une chaîne SQL.

Cela leur impose aussi des limites définitives. Ils ignorent tout de l'intention : aucun analyseur ne vous dira jamais qu'un taux de TVA est faux. Ils doivent procéder par approximation, car les questions sous-jacentes sont indécidables, et qui dit approximation dit faux positifs (signaler quelque chose de sûr) ou faux négatifs (manquer un vrai problème). Chaque outil choisit un point sur ce spectre, et savoir où il se situe vous indique comment traiter ses résultats : Error Prone est réglé pour des faux positifs quasi nuls et peut donc faire échouer un build, alors qu'un scanner de sécurité penche délibérément vers le signalement et exige un tri.

Les familles d'outils

Tout ranger sous l'étiquette « analyse statique » pousse les équipes à adopter des outils qui se recouvrent, puis à s'y noyer. Il y a cinq rôles distincts :

Adoptez un outil par ligne, pas trois sur la même. Deux vérificateurs de style aux opinions différentes produisent des résultats contradictoires, et une dispute.

Commencer par le compilateur

Avant tout outil, le JDK signale déjà beaucoup de choses que la plupart des projets ignorent. Activer les avertissements et les traiter comme des erreurs est l'amélioration de qualité la moins chère qui soit :

<plugin>
  <groupId>org.apache.maven.plugins</groupId>
  <artifactId>maven-compiler-plugin</artifactId>
  <configuration>
    <release>21</release>
    <compilerArgs>
      <arg>-Xlint:all,-serial,-processing</arg>
      <arg>-Werror</arg>
    </compilerArgs>
  </configuration>
</plugin>

-Xlint:all couvre les opérations génériques non vérifiées, les dépréciations, le fall-through dans les switch, les @Override manquants, et d'autres encore. -Werror les rend bloquants pour le build, et c'est le seul réglage qui fonctionne : un build qui affiche quatre cents avertissements est un build où personne ne lit les avertissements. Sur un projet existant, activez les catégories une par une.

Les détecteurs de bugs qui valent la peine

Error Prone se branche sur javac et analyse le code pendant la compilation, avec toute l'information de typage. Ses règles sont sélectionnées pour leur fiabilité, ce qui explique que beaucoup soient des erreurs par défaut : appeler Object.equals sur des types incompatibles, un argument incohérent avec une @FormatMethod, une valeur de retour ignorée alors qu'elle ne doit pas l'être, une Duration comparée avec ==. Comme il tourne dans le compilateur, il est rapide et peut proposer des corrections automatiques.

<compilerArgs>
  <arg>-XDcompilePolicy=simple</arg>
  <arg>--should-stop=ifError=FLOW</arg>
  <arg>-Xplugin:ErrorProne -Xep:MissingOverride:ERROR -Xep:UnusedVariable:WARN</arg>
</compilerArgs>

SpotBugs analyse le bytecode compilé et trouve d'autres choses : déréférencement de null sur un chemin, fuites de ressources, état interne mutable exposé via un getter, synchronisation incohérente, classes Serializable dotées de champs non sérialisables. Le plugin find-sec-bugs ajoute des motifs d'injection et de cryptographie. L'analyse de bytecode voit ce que le compilateur a produit plutôt que ce que vous avez écrit, ce qui rend parfois certaines anomalies déroutantes jusqu'à ce qu'on regarde le code généré.

PMD travaille sur le code source et se spécialise dans les règles de conception et de complexité (méthodes trop longues, trop de paramètres, conditions profondément imbriquées, blocs catch vides), sans oublier son détecteur de copier-coller CPD, qui repère entre fichiers des blocs dupliqués qu'aucun autre outil de cette liste ne remarque.

Checkstyle fait respecter des conventions plutôt que la correction du code : nommage, ordre des imports, Javadoc sur l'API publique, structure des fichiers. C'est celui qui recoupe le moins les autres, et celui dont le jeu de règles doit le plus être élagué pour convenir à l'équipe, car une configuration par défaut se plaindra de quantité de choses dont personne n'a décidé de se soucier.

Deux règles de configuration s'appliquent à tous. Faites échouer le build sur les règles dont vous avez décidé qu'elles comptent, parce qu'un rapport que personne n'ouvre est un rapport que personne ne lit. Et retirez de la configuration partagée les règles avec lesquelles vous n'êtes pas d'accord, plutôt que de les supprimer une à une dans le code : un commentaire de suppression est une décision locale à un fichier qui, sans bruit, devient une politique.

SonarQube et la gate sur le nouveau code

SonarQube agrège ses propres analyseurs et les rapports importés des outils ci-dessus, conserve l'historique et applique une quality gate : un ensemble de conditions qu'un build satisfait ou non.

La décision de configuration la plus importante, de loin, est d'appliquer la gate au nouveau code (les lignes ajoutées ou modifiées depuis une référence choisie) plutôt qu'à l'ensemble du projet. C'est ce qui rend l'analyse adoptable sur une vraie base de code. Une gate exigeant 80 % de couverture sur un système vieux de quinze ans est inatteignable et sera désactivée dans le mois ; une gate exigeant 80 % de couverture sur les lignes touchées cette semaine est une demande raisonnable, et la base de code s'améliore partout où le travail avance.

Une gate réaliste sur le nouveau code :

• Couverture des nouvelles lignes supérieure à 80 %
• Lignes dupliquées dans le nouveau code inférieures à 3 %
• Aucune nouvelle anomalie classée blocker ou critical
• Aucun nouveau security hotspot laissé sans revue
• Note globale du nouveau code à A

Le modèle plus récent de SonarQube classe les anomalies selon qu'elles menacent le caractère consistent, intentional, adaptable ou responsible du code, et les rattache à des qualités logicielles (sécurité, fiabilité, maintenabilité) au lieu de l'ancien triptyque bug / vulnérabilité / code smell. Cette correspondance compte moins que la discipline consistant à trier chaque nouveau blocker avant le merge.

Branchez-la dans le pipeline pour que la gate soit une condition de merge, pas un tableau de bord que quelqu'un consulte de temps en temps :

mvn -B verify sonar:sonar \
    -Dsonar.projectKey=shop-service \
    -Dsonar.qualitygate.wait=true

sonar.qualitygate.wait=true bloque la commande jusqu'à ce que le résultat de la gate soit connu et la fait sortir avec un code non nul en cas d'échec : c'est ce qui en fait une vraie gate. Le guide de l'intégration continue explique où placer cette étape dans un pipeline par rapport aux étapes de test.

Les métriques, évaluées honnêtement

L'analyse statique produit des chiffres, et les chiffres finissent sur des tableaux de bord, qu'ils veuillent dire quelque chose ou non.

La complexité cyclomatique compte les chemins indépendants à travers une méthode. C'est un outil de tri correct (triez les méthodes selon cette valeur et les pires spécimens de la base de code sont généralement en haut) et une mauvaise cible. Un switch plat sur vingt valeurs d'enum obtient un score élevé et se lit parfaitement ; une méthode avec trois boucles imbriquées et un continue obtient un score plus bas et est bien plus difficile à suivre. La complexité cognitive a été conçue précisément pour corriger cela, en pénalisant l'imbrication et en ignorant les structures abrégées, et elle suit nettement mieux la difficulté ressentie par un humain.

Le pourcentage de duplication est utile lorsqu'il désigne de la logique métier copiée-collée, là où un bug finit corrigé à deux endroits sur trois. Il n'est que du bruit lorsqu'il signale du code généré, des tests volontairement explicites ou du boilerplate de DTO.

La dette technique exprimée en temps de remédiation (« 4 jours 2 heures ») est une somme grossière d'estimations par règle. Traitez-la comme un moyen de prioriser, pas comme une estimation : personne n'a jamais assaini une base de code dans le temps prédit par l'outil. Sa tendance est plus parlante que sa valeur, et sa valeur par module plus parlante que le total.

La couverture est importée plutôt que calculée par l'analyseur, et s'accompagne de la mise en garde du guide de stratégie d'automatisation des tests : elle mesure l'exécution, pas les assertions, et se comporte mal dès qu'on en fait un objectif.

Les notes de maintenabilité, de fiabilité et de sécurité condensent des nombres d'anomalies en une lettre. Elles communiquent bien auprès des personnes extérieures à l'équipe et masquent tout ce dont un spécialiste a besoin : servez-vous-en pour le reporting, et de la liste des anomalies pour le travail.

L'analyse orientée sécurité

Deux analyses distinctes ont leur place dans tout pipeline.

Le SAST cherche des motifs exploitables dans votre propre code : du SQL construit par concaténation de chaînes, une commande exécutée à partir d'une entrée utilisateur, du path traversal, un hachage faible, des identifiants codés en dur, une vérification de certificat désactivée, un parsing XML avec les entités externes activées. Les moteurs les plus solides font du taint tracking, c'est-à-dire qu'ils suivent les données d'une source non fiable jusqu'à un point d'utilisation dangereux ; c'est ce qui leur permet de distinguer un SQL concaténé à partir d'une constante d'un SQL concaténé à partir d'un paramètre de requête.

Le SCA (analyse des dépendances) confronte vos bibliothèques aux bases de vulnérabilités. En pratique, il trouve plus de risques réels que le SAST, parce que la plupart des applications embarquent bien plus de code tiers que de code maison. Les règles opérationnelles qui le rendent supportable : bloquer sur les nouvelles anomalies de sévérité élevée pour que la file d'attente ne bloque pas chaque build, suivre les dépendances transitives autant que les directes, et consigner les risques acceptés avec une date d'expiration plutôt qu'une suppression définitive.

La détection de secrets est une troisième analyse, peu coûteuse et qui vaut la peine : un scan du diff à la recherche de clés privées, de tokens et de chaînes de connexion. Elle doit tourner dans un hook pre-receive ou au tout début du pipeline, car une fois qu'un secret a atteint l'historique partagé, le seul remède est la rotation de l'identifiant.

Aucune de ces analyses ne remplace une revue de sécurité ni un test d'intrusion, et un scan propre est un plancher, pas une garantie. Ce qu'elles font de manière fiable, c'est empêcher les motifs bien connus d'atteindre main.

Adopter l'analyse sur une base de code legacy

La première analyse complète d'une base de code établie remonte des milliers d'anomalies. C'est ce chiffre qui fait échouer la plupart des tentatives d'adoption : un backlog de cette taille n'est pas exploitable, donc il est ignoré, et une fois ignoré, l'outil n'est plus que du bruit.

Une séquence qui fonctionne :

1. Figer la référence

   Lancez l'analyse, enregistrez l'état actuel et configurez la gate pour qu'elle ne s'applique qu'au nouveau code. L'objectif immédiat est que le chiffre cesse d'augmenter, pas qu'il diminue.

2. Activer partout une poignée de règles sûres

   Choisissez dix à quinze règles très fiables portant sur la correction du code (fuites de ressources, valeurs de retour ignorées, erreurs de comparaison, blocs catch vides) et rendez-les bloquantes pour le build sur toute la base de code. Corrigez les violations existantes de ces règles-là seulement. C'est en général un ou deux jours de travail, et cela élimine de vrais défauts.

3. Automatiser la mise en forme en un seul commit

   Appliquez le formateur à toute la base de code dans un unique commit consacré à la mise en forme, ajoutez son hash au fichier ignore-revs pour que l'historique reste lisible, puis imposez-le. Cela élimine définitivement toute une catégorie d'anomalies.

4. Corriger au fil de l'eau

   Quand quelqu'un modifie un fichier, il corrige les anomalies dans la partie qu'il a touchée. En un an, les fichiers qui comptent, ceux qui évoluent activement, deviennent propres, et les fichiers dormants restent en l'état, ce qui est la bonne répartition de l'effort.

5. Ajouter des règles progressivement

   Une nouvelle catégorie de règles par itération, discutée à l'avance et acceptée par l'équipe. Une règle imposée sans discussion finit supprimée plutôt que respectée.

6. Revoir les suppressions

   Une fois par trimestre, listez chaque suppression et chaque exclusion. Certaines traduisent une décision qui mérite d'être conservée ; d'autres contournaient un faux positif propre à une version, corrigé depuis.

Le principe sous-jacent est que l'analyse doit faire partie de la boucle dans laquelle le développeur se trouve déjà. Une anomalie qui apparaît dans l'IDE pendant la frappe coûte quelques secondes ; la même dans un rapport nocturne coûte un changement de contexte, et la même dans un audit trimestriel coûte une réunion. Configurez les plugins de l'IDE à partir des mêmes fichiers de configuration partagés que le build, pour que les résultats locaux et ceux du pipeline concordent : un désaccord entre les deux est le moyen le plus rapide de faire perdre confiance dans l'ensemble.

Points clés à retenir

• L'analyse statique raisonne sur des chemins qu'aucun test n'exerce, et les tests vérifient une intention qu'aucun analyseur ne peut connaître ; les deux couvrent des classes de défauts différentes et aucun ne remplace l'autre.
• Choisissez un outil par famille (formateur, vérificateur de style, détecteur de bugs, SAST, scanner de dépendances) et configurez délibérément le jeu de règles partagé au lieu d'accepter les valeurs par défaut.
• Commencez par le compilateur : -Xlint:all -Werror ne coûte rien et détecte une quantité surprenante de problèmes.
• Appliquez la gate au nouveau code, pas à toute la base de code ; ce seul choix rend l'analyse adoptable sur un système legacy.
• Traitez les chiffres de complexité, de duplication et de dette technique comme des outils de tri et des tendances, jamais comme des objectifs, et préférez la complexité cognitive pour les questions de lisibilité.
• L'analyse des dépendances trouve généralement plus de risques réels que l'analyse de votre propre code : bloquez sur les nouvelles anomalies de sévérité élevée et donnez une date d'expiration aux risques acceptés.