🧜🏾 👨🏾‍🏭 🤴🏿 PVS-Studio pour Java sous le capot: développement de diagnostics 🤓 👨🏻‍🎨 🎺

Pour un changement, nous vous parlerons aujourd'hui un peu du processus de développement et de finalisation des règles de diagnostic pour PVS-Studio Java. Voyons pourquoi les anciens déclencheurs de l'analyseur ne flottent pas trop d'une version à l'autre, et les nouveaux ne sont pas trop fous. Et nous gâcherons un peu plus "quels sont les plans des javistes" et montrerons quelques belles (et pas si) erreurs trouvées à l'aide des diagnostics de la prochaine version.

Processus de développement de diagnostics et d'autotesteurs

Naturellement, chaque nouvelle règle de diagnostic commence par une idée. Et comme l'analyseur Java est la direction la plus récente du développement de PVS-Studio, nous volons essentiellement ces idées aux départements C / C ++ et C #. Mais tout n'est pas si mal: nous ajoutons également des règles qui ont été inventées par nous-mêmes (y compris par les utilisateurs - merci!), Pour que plus tard, les mêmes services nous les volent. Le cycle, comme on dit.

L'implémentation même des règles dans le code s'avère dans la plupart des cas être une tâche de pipeline. Vous créez un fichier avec plusieurs exemples synthétiques, marquez avec vos mains où les erreurs devraient être, et avec le débogueur prêt, vous parcourez l'arbre de syntaxe jusqu'à ce que vous vous ennuyiez et que vous couvriez tous les cas inventés. Parfois, les règles se révèlent absurdement simples (par exemple, V6063se compose littéralement de quelques lignes), et parfois vous devez réfléchir assez longtemps à la logique.

Cependant, ce n'est que le début du processus. Comme vous le savez, nous n'aimons pas particulièrement les exemples synthétiques, car ils reflètent très mal le type de déclencheurs d'analyseurs dans des projets réels. À propos, une partie importante de ces exemples dans nos tests unitaires est tirée de projets réels - il est presque impossible d'inventer tous les cas possibles par vous-même. Et les tests unitaires nous permettent également de ne pas perdre de déclencheurs sur des exemples de la documentation. Il y avait des précédents, oui, seulement chut.

Ainsi, les aspects positifs des projets réels doivent d'abord être trouvés d'une manière ou d'une autre. Et vous devez également vérifier que:

La règle ne tombera pas sur la folie open-source, où les solutions «intéressantes» sont courantes;
( - , );
data-flow ( ) - ;
open-source ;
over 9000%;
"" , ;
.

En général, ici, comme un chevalier sur un cheval (un peu boitant, mais on y travaille), SelfTester vient au premier plan. Sa tâche principale et unique est de vérifier automatiquement un tas de projets et de montrer quels déclencheurs ont été ajoutés, disparus ou modifiés par rapport à la «référence» dans le système de contrôle de version. Fournissez des différences pour le rapport de l'analyseur et affichez le code correspondant dans les projets, en bref. Actuellement, SelfTester pour Java teste 62 projets open source de versions barbus, parmi lesquels, par exemple, DBeaver, Hibernate et Spring. Une exécution complète de tous les projets prend 2 à 2,5 heures, ce qui est sans aucun doute douloureux, mais rien ne peut être fait.

Dans la capture d'écran ci-dessus, les projets «verts» sont ceux dans lesquels rien n'a changé. Chaque différence dans les projets «rouges» est examinée manuellement et, si elle est correcte, est confirmée par le même bouton «Approuver». À propos, le kit de distribution de l'analyseur ne sera construit que si SelfTester donne un résultat vert pur. En général, c'est ainsi que nous maintenons la cohérence des résultats entre les différentes versions.

En plus de maintenir la cohérence des résultats, SelfTester nous permet de nous débarrasser d'un grand nombre de faux positifs avant même la publication des diagnostics. Un modèle de développement typique ressemble à ceci:

, . , " double-checked locking" ;
SelfTester-, ;
, -;
SelfTester- , ;
3-4, ;
, , ( , );
, master.

Heureusement, les exécutions complètes de SelfTester sont assez rares et vous n'avez pas à attendre "2 à 2,5 heures" très souvent. De temps en temps, la chance contourne et des déclencheurs apparaissent sur de grands projets comme Sakai et Apache Hive - il est temps de boire du café, de boire du café et de boire du café. Vous pouvez également étudier la documentation, mais ce n'est pas pour tout le monde.

"Pourquoi avons-nous besoin de tests unitaires, puisqu'il existe un outil magique?"

Et puis que les tests sont nettement plus rapides. Quelques minutes - et il y a déjà un résultat. Ils vous permettent également de voir exactement quelle partie de la règle est tombée. De plus, tous les déclenchements autorisés d'une règle ne sont pas toujours capturés dans les projets SelfTester, mais leur opérabilité doit également être vérifiée.

Nouveaux problèmes chez d'anciennes connaissances

Initialement, cette section de l'article commençait par les mots "Les versions des projets dans SelfTester sont assez anciennes, donc la plupart des erreurs présentées ont probablement déjà été corrigées." Cependant, lorsque j'ai décidé de m'en assurer, j'ai eu une surprise. Chaque erreur est restée en place. Tout. Cela signifie deux choses:

Ces erreurs ne sont pas extrêmement critiques pour le fonctionnement de l'application. Beaucoup d'entre eux, d'ailleurs, sont dans le code de test, et des tests incorrects peuvent difficilement être qualifiés de cohérents.
Ces erreurs se trouvent dans des fichiers rarement utilisés de grands projets, auxquels les développeurs se rendent rarement. Pour cette raison, le code incorrect est voué à rester là pendant très longtemps: très probablement, jusqu'à ce qu'un bug critique se produise à cause de cela.

Pour ceux qui souhaitent approfondir, il y aura des liens vers des versions spécifiques que nous vérifions.

PS Ce qui précède ne signifie pas que l'analyse statique ne détecte que des erreurs inoffensives dans le code inutilisé. Nous vérifions les versions de sortie (et presque) des projets - dans lesquelles les développeurs et les testeurs (et parfois, malheureusement, les utilisateurs) ont trouvé à la main les bogues les plus pertinents, ce qui est long, coûteux et douloureux. Vous pouvez en savoir plus à ce sujet dans notre article " Erreurs que l'analyse de code statique ne peut pas trouver, car il n'est pas utilisé ".

Apache Dubbo et menu vierge

Diagnostics GitHub " V6080 Envisagez de vérifier les erreurs d'impression. Il est possible qu'une variable affectée doive être vérifiée dans la condition suivante " a déjà été publié dans la version 7.08, mais n'est pas encore apparu dans nos articles, il est donc temps de le corriger.

Menu.java:40

public class Menu
{
  private Map<String, List<String>> menus = new HashMap<String, List<String>>();

  public void putMenuItem(String menu, String item)
  {
    List<String> items = menus.get(menu);
    if (item == null)                      // <=
    {
      items = new ArrayList<String>();
      menus.put(menu, items);
    }
    items.add(item);
  }
  ....
}

Un exemple classique de dictionnaire "key-collection" et une faute de frappe tout aussi classique. Le développeur voulait créer une collection correspondant à la clé, si elle n'existe pas déjà, mais il a mélangé le nom de la variable et a obtenu non seulement une opération incorrecte de la méthode, mais aussi une NullPointerException dans la dernière ligne. Pour Java 8 et versions ultérieures, pour implémenter de tels dictionnaires, vous devez utiliser la méthode computeIfAbsent :

public class Menu
{
  private Map<String, List<String>> menus = new HashMap<String, List<String>>();

  public void putMenuItem(String menu, String item)
  {
    List<String> items = menus.computeIfAbsent(menu, key -> new ArrayList<>());
    items.add(item);
  }
  ....
}

Glassfish et verrouillage à double contrôle

GitHub

L'un des diagnostics qui sera inclus dans la prochaine version est de vérifier l'implémentation correcte du modèle de "verrouillage double vérifié". Glassfish s'est avéré être le détenteur du record pour les détections des projets SelfTester: au total, PVS-Studio a trouvé 10 zones problématiques dans le projet en utilisant cette règle. J'invite le lecteur à s'amuser et à en chercher deux dans l'extrait de code ci-dessous. Pour obtenir de l'aide, reportez-vous à la documentation: " V6082 Verrouillage non sécurisé contre-vérifié ". Eh bien, ou, si vous ne voulez pas du tout, à la fin de l'article.

EjbComponentAnnotationScanner.java

public class EjbComponentAnnotationScanner
{
  private Set<String> annotations = null;

  public boolean isAnnotation(String value)
  {
    if (annotations == null)
    {
      synchronized (EjbComponentAnnotationScanner.class)
      {
        if (annotations == null)
        {
          init();
        }
      }
    }
    return annotations.contains(value);
  }

  private void init()
  {
    annotations = new HashSet();
    annotations.add("Ljavax/ejb/Stateless;");
    annotations.add("Ljavax/ejb/Stateful;");
    annotations.add("Ljavax/ejb/MessageDriven;");
    annotations.add("Ljavax/ejb/Singleton;");
  }

  ....
}

SonarQube et flux de données

GitHub

L'amélioration des diagnostics ne consiste pas seulement à modifier directement leur code afin d'attraper plus d'endroits suspects ou de supprimer les faux positifs. Le marquage manuel des méthodes pour le flux de données joue également un rôle important dans le développement de l'analyseur - par exemple, vous pouvez écrire que telle ou telle méthode de bibliothèque renvoie toujours non nulle. Lors de l'écriture d'un nouveau diagnostic, nous avons accidentellement découvert que la méthode Map # clear () n'était pas balisée . Hormis le code manifestement stupide que "la collection V6009 est vide. L'appel de la fonction" clear "est insensé " les diagnostics ont commencé à attraper , nous avons pu trouver une grande faute de frappe.

MetricRepositoryRule.java:90

protected void after()
{
  this.metricsById.clear();
  this.metricsById.clear();
}

À première vue, effacer à nouveau le dictionnaire n'est pas une erreur. Et nous penserions même qu'il s'agit d'une ligne dupliquée au hasard, si notre regard n'avait pas baissé un peu plus bas - littéralement vers la méthode suivante.

protected void after()
{
  this.metricsById.clear();
  this.metricsById.clear();
}
public Metric getByKey(String key)
{
  Metric res = metricsByKey.get(key);
  ....
}

Exactement. La classe a deux champs avec des noms similaires metricsById et metricsByKey . Je suis sûr que dans la méthode after , le développeur voulait effacer les deux dictionnaires, mais soit l'auto-complétion a échoué, soit il a, par inertie, entré le même nom. Ainsi, les deux dictionnaires contenant les données associées ne seront pas synchronisés après l'appel à after .

Sakai et collections vides

GitHub

Un autre nouveau diagnostic qui sera inclus dans la prochaine version est " V6084 Retour suspect d'une collection toujours vide ". Il est assez facile d'oublier d'ajouter des éléments à la collection, en particulier lorsque chaque élément doit d'abord être initialisé. D'après l'expérience personnelle, ces erreurs ne conduisent le plus souvent pas à un crash de l'application, mais à un comportement étrange ou à l'absence de toute fonctionnalité.

DateModel.java:361

public List getDaySelectItems()
{
  List selectDays = new ArrayList();
  Integer[] d = this.getDays();
  for (int i = 0; i < d.length; i++)
  {
    SelectItem selectDay = new SelectItem(d[i], d[i].toString());
  }
  return selectDays;
}

À propos, la même classe contient des méthodes très similaires sans la même erreur. Par exemple:

public List getMonthSelectItems()
{
  List selectMonths = new ArrayList();
  Integer[] m = this.getMonths();
  for (int i = 0; i < m.length; i++)
  {
    SelectItem selectMonth = new SelectItem(m[i], m[i].toString());
    selectMonths.add(selectMonth);
  }
  return selectMonths;
}

Projets pour le futur

Outre diverses choses internes peu intéressantes, nous pensons ajouter des diagnostics pour Spring Framework à l'analyseur Java. Ce n'est pas seulement le pain principal des javistes, mais il contient également beaucoup de moments non évidents sur lesquels on peut trébucher. Nous ne sommes pas encore très sûrs sous quelle forme ces diagnostics apparaîtront finalement, quand cela se produira et si cela se produira du tout. Mais nous sommes sûrs que nous avons besoin d'idées pour eux et de projets open-source utilisant Spring for SelfTester. Alors, si vous avez quelque chose en tête, suggérez-le (dans les commentaires ou les messages privés, vous pouvez aussi)! Et plus nous collectons cette bonté, plus la priorité y sera accordée.

Et enfin, il y a des erreurs dans l'implémentation du verrouillage double-vérifié de Glassfish:

Le champ n'est pas déclaré «volatile».
L'objet est d'abord publié puis initialisé.

Pourquoi tout cela est mauvais - encore une fois, vous pouvez le voir dans la documentation .

Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien de traduction: Nikita Lazeba. Sous le capot de PVS-Studio pour Java: comment nous développons des diagnostics .

PVS-Studio pour Java sous le capot: développement de diagnostics