JMRI: Analyse statique avec SpotBugs

SpotBugs est un outil qui peut examiner le code pour trouver d'éventuels problèmes. Il fait un "analyse statique", en regardant à travers le code pour trouver certaines "mauvaises idées connues": Les choses qui sont susceptibles de causer des problèmes occasionnels/intermittents, mauvaise performance, etc Parce que ces genre de problèmes sont difficile à trouver avec les tests, les trouver par l'inspection est souvent la seule approche réaliste. Disposer d'un outil qui peut faire les inspections automatiquement, afin qu'elles puissent se faire à chaque fois que quelque chose est changé, protège le code d'une dégradation lente sans que quelqu'un le remarque jusqu'à ce qu'il soit trop tard.

Pour plus d'informations sur SpotBugs, voir la page d'accueil SpotBugs .

Nous exécutons sous forme d'une routine SpotBugs comme une partie de notre processus continue d'intégration. Vous pouvez voir les résultats des plus récentes constructions en ligne ici.

Si SpotBugs trouve une erreur positive, un bug qui n'est pas réellemnt un bug, vous pouvez l'éteindre avec une annotation tels que:


@edu.umd.cs.findbugs.annotations.SuppressFBWarnings("FE_FLOATING_POINT_EQUALITY","OK to compare floats, as even tiny differences should
trigger update")

Bien que Java lui-même estime qu'il est facultatif, nous avons besoin du deuxième argument "justification". Expliquer pourquoi vous avez ajouté cette annotation pour supprimer un message aidera celui qui vient après vous et tente de comprendre le code. Il aidera également à vous assurer que vous comprenez bien la cause du rapport de bogue sous-jacent: Parfois, ce qui semble un faux positif ne l'est vraiment pas. Les Annotations sans une clause de justification seront périodiquement éliminées.

Si vous avez besoin de mettre plus d'un type de message dans une annotation, utilisez la syntaxe de tableau:


@edu.umd.cs.findbugs.annotations.SuppressFBWarnings("{type1},{type2}","why both are needed")

Il y a ausi des annotations Java et FingBugs qui peuvent vous aider à mieux comprendre votre code. Parfois, ils vont lui donner suffisamment de compréhension, par exemple: quand une variable peut être nulle, que ça ne va plus faire des erreurs de faux positifs. Pour en savoir plus, voir les Annotations Java et les pages d'annotations SpotBugs .

Les bases d'annotations sont couverts dans un Tutoriel annotation Java.

Il peut être utile de marquer le code avec une des annotaions suivantes de telle sorte que SpotBugs fasse un bon travail de raisonnement à ce sujet:

javax.annotation.Nonnull - L'element annoté ne doit pas être nul. Les champs annotés ne doivent pas être nuls après la construction terminée. Les méthodes annotées doivent avoir des valeurs non-nulles retournées Utiliser javax.annotation.ParametersAreNonnullByDefault pour définir pour toute une classe. Préférer l'usage de CheckForNull.
javax.annotation.CheckForNull - La variable, paramètre ou valeur annotée peut avoir une valeur nulle, de sorte que tous les utilisateurs devraient les traiter convenablement. S'il vous plaît mettre cela dans des définitions de méthode pour signaler que la valeur de retournée doit être vérifié pour être nulle
edu.umd.cs.findbugs.annotations.OverrideMustInvoke Utilisé pour annoter une méthode qui, si surchargée, doit (ou devrait) être super invoquée dans le méthode redéfinie. Des exemples de tels procédés comprennent la finalisation () et clone (). L'argument de la méthode indique quand la super invocation doit se produire: à tout moment ( ANYTIME ), au début de la méthode dérogatoire ( FIRST ), ou à la fin de la méthode dérogatoire ( LAST ), par défaut à tout moment, par exemple @OverrideMustInvoke ( valeur = ANYTIME )
javax.annotation.CheckReturnValue - annoter une méthode pour dire QUE la méthode n'a pas d'effets secondaires, donc il n'y a aucun point en appelant sans vérifier sa valeur de retour
net.jcip.annotations.Immutable - Un objet de cettte classe ne peut pas être changé après avoir été créé. Ceci permet à la fois une meilleure vérification de la logique et aussi simplifie l'usage pour vos collègues, aussi il est bon de créer des classes qui ont cette propriété et de les annoter.
net.jcip.annotations.ThreadSafe - classes qui doivent être utilisables pour plusieurs threads.Fidbugs l'assume généralement, mais il es bon de de le mettre dans une classe qui est prévue pour être thread-safe comme un rappel pour de futures développeurs.

Pour avoir plus d'information à propos de ces annotations, SVP voir la Page dans l'API Concurrency.

Nous n'utilisons pas ces annotations:

javax.annotation.Nullable - cela ne signifie pas vraiment ce que les gens pensent qu'il fait, comme SpotBugs ne vérifie pas vraiment pas quelque chose quand il est utilisé. De la documentation: SpotBugs traitera les articles annotés comme s'ils n'avaient aucune annotation. Dans la pratique, cette annotation est utile que pour des raisons impérieuses d'une annotation globale non nulle. Utilisez javax.annotation.ParametersAreNullableByDefault pour le définir comme une classe entière. Preférez l'usage de CheckForNull.

Suppression Avertissements

Nous avons désactivé les SpotBugs de routine de contrôle de certains types de conditions:

RI_REDUNDANT_INTERFACES: Ce drapeaux au cas où une classe implémente une interface, et aussi hérite d'une superclasse qui implémente déjà cette interface. Ceci est redondant et inutile, mais il ne peut pas provoquer de dysfonctionnement du code. Nous avons assez d'entre eux que nousavons éteint l'avertissement, et nous y reviendrons plus tard.
SIC_INNER_SHOULD_BE_STATIC_ANON ,
SIC_INNER_SHOULD_BE_STATIC_NEEDS_THIS: Static opposé à non-static, les classes internes ( classes définies au cœur d'autres classes ) prennent moins de place parce qu'ils ne maintiennent pas de références à l'objet contenant. Cet avertissement suggère de déplacer une classe anonyme interne (définie dans la ligne du code) à une classe régulière (non définie en ligne) ainsi elles peuvent être statiques. Bien que ce soit probablement une petite amélioration, c'est un peu de travail pour une petite amélioration. Nous avons assez de ceux où nous avons coupé les avertissements, et et nous y reviendront plus tard.
SIC_INNER_SHOULD_BE_STATIC_ANON, SIC_INNER_SHOULD_BE_STATIC_NEEDS_THIS: Statique, comme opposé à non-statique, Les inner classes ( classes définies au sein d'autres classes ) prennent peu de place parce qu'lles ne permettent pas de maintenir les références à l'objet contenant. Cet avertissement suggère de déplacer une classe interne anonyme ( définie dans la ligne du code ) vers une classe régulière ( non défini pas en ligne ) ainsi elle peut être rendue statique.Bien que ce soit probablement une petite amélioration, c'est un peu de travail pour une petite amélioration. Nous en avons assez aussi nous avons eteint l'avertissement, et il reviendra un peu plus tard.
PZLA_PREFER_ZERO_LENGTH_ARRAYS: Que doit faire une méthode qui retourne un tableau de valeurs quand il n'y en a pas? Retourner "Nulle", opposé à un tableau vides, exige tout le code d'appel pour traiter un cas particulier. Dans beaucoup de cas, retourner un tableau vide rend le code plus simple. Mais pour du code existant, la simplification est marginale. Comme cela est un problème de conception, mais il surgit souvent nous avons supprimé ceci pour nous focaliser maintenant sur des problèmes plus presssants
Code Malveillant: C'est une classe d'avertissement centrée autour d'une idée que les données et les méthodes de codes ne doivent pas être trop visibles spécialement en statique. Ceci est vrai, mais JMRI n'est pas une bibliothèque trempée qui est publiée dans un monde de gens qui essaient de le briser, aussi ces changements ne sont pas une grande priorité.
Nm: La convention est que les noms de cette clase démarre avec une lettre capitale et les noms de procédé ( donné et code ) avec une lettre minuscule.
Se,SvVI: Cette une grande classe d'averissementss associées avec la sérialisation Java. JMRI n'utilise pas la sérialisation et il est peu probable qu'il le fasse à l'avenir, de sorte que nous supprimons ces derniers pour élever la qualité moyenne des avertissements émis.

Arrière Plan

Simon White a ajouté le support FindBugs à notre Chaine de construction basée sur Ant pendant le développement du JMRI 2.5.5. Sa note sur ceci suit...

Comme demande de fonctionnalité par 1716873, j'ai ajouté une tâche Ant pour exécuter SpotBugs. Ceci va produire un rapport en HTML dans le méme emplacement que le JMRI jar ( c'est à dire le plus haut niveau du répertoire projet ). Notez la nouvelle tâche exécute en premier ant dist parce SpotBugs examine le dossier jmri.jar.

Pour exécuter la tâche:

Installer SpotBugs ( pour moi j'ai mis cela dans C:/SpotBugs-3.1.1 )
soit exécuter ant-Dspotbugs.home=C:/SpotBugs-4.2.3 spotbugs ( remplaéant le paramètre de votre emplacement d'installation ) ou modifier le build.xml et modifier le paramètre commenté de spotbugs.home à votre emplacement d'installation, puis exécutez ant spotbugs

Sur mon ancienne machine il faut environ 20 minutes, mais votre durée peut varier.

Notez que dans le build.xml j'ai mis le réglage maximal de mémoire-Xmx pour le tâche SpotBugs pour 1024m. Si votre système a plus de mémoire, la stimuler peut accélérer les choses.

L'exécution de ceci sur JMRI 2.5.4 donné les résultats suivants:

Total	1322
Avertissements Mauvaise pratique	164
Avertissements Correction	77
Avertissements expérimentaux	7
Avertissements vulnérabilité code malveillant	221
Avertissements multithread exactitude	90
Avertissements Performance	459
Avertissements Louche	304

Beaucoup de travail a été fait dans JMRI pendant le cycle de libération 2.12 pour amener le compteur de bug à zéro. Le serveur d'intégration continue a fait fonctionner SpotBugs deux fois par jour, ce qui permet aux développeurs de voir les résultats de leur codage sans avoir à exécuter spotbugs eux-mémes.

Si vous regardez le code dans le dépôt Subversion JMRI, s'il vous plaît vérifier le serveur CI et assurez-vous que vos modifications ne vont pas introduire de nouvelles erreurs.