Analyse statique IntelliJ IDEA vs esprit humain

Il n'y a pas si longtemps, j'ai étudié la sortie de l'analyseur statique IntelliJ IDEA pour le code Java et suis tombé sur un cas intéressant. Étant donné que le fragment de code correspondant n'est pas open source, je l'ai anonymisé et détaché des dépendances externes. Nous supposons qu'il ressemblait à ceci:


private static List<Integer> process(Map<String, Integer> options, List<String> inputs) { List<Integer> res = new ArrayList<>(); int cur = -1; for (String str : inputs) { if (str.startsWith("-")) if (options.containsKey(str)) { if (cur == -1) cur = options.get(str); } else if (options.containsKey("+" + str)) { if (cur == -1) cur = res.isEmpty() ? -1 : res.remove(res.size() - 1); if (cur != -1) res.add(cur + str.length()); } } return res; } 

Le code est comme du code, quelque chose se transforme, quelque chose se fait, mais l'analyseur statique ne l'aime pas. Ici, nous voyons deux avertissements:


Capture d'écran du code


Sur l'expression res.isEmpty() EDI dit que la condition est toujours vraie, et sur cur que l'affectation n'a pas de sens, puisque la même valeur se trouve déjà dans cette variable. Il est facile de voir que le problème d'affectation est une conséquence directe du premier problème. Si res.isEmpty() vraiment toujours vrai, alors la chaîne est réduite à


 if (cur == -1) cur = -1; 

C'est en effet superflu. Mais pourquoi l'expression est-elle toujours vraie? Après tout, res est une liste, elle est remplie dans le même cycle. Le nombre d'itérations de la boucle et les branches dans lesquelles nous allons dépend des paramètres d'entrée que l'IDE ne peut pas connaître. Nous pourrions ajouter un élément à res à l'itération précédente, puis la liste ne sera pas vide.


J'ai vu ce code pour la première fois et j'ai passé beaucoup de temps à traiter ce cas. Au début, j'étais presque convaincu que je suis tombé sur un bug dans l'analyseur, et je devrais le corriger. Voyons voir si c'est le cas.


Tout d'abord, nous marquerons toutes les lignes où l'état de la méthode change. Il s'agit d'une modification de la variable cur ou d'une modification de la liste res :


 private static List<Integer> process(Map<String, Integer> options, List<String> inputs) { List<Integer> res = new ArrayList<>(); int cur = -1; for (String str : inputs) { if (str.startsWith("-")) if (options.containsKey(str)) { if (cur == -1) cur = options.get(str); // A } else if (options.containsKey("+" + str)) { if (cur == -1) cur = res.isEmpty() ? -1 : // B res.remove(res.size() - 1); // C if (cur != -1) res.add(cur + str.length()); // D } } return res; } 

Les lignes 'A' et 'B' ( 'B' est la première branche de l'instruction conditionnelle) modifient la variable cur , 'D' modifient la liste et 'C' (la deuxième branche de l'instruction conditionnelle) modifie à la fois la liste et la variable cur . Il nous importe de savoir si cur -1 se trouve et si la liste est vide. Autrement dit, vous devez surveiller quatre états:



La chaîne 'A' change cur s'il y avait -1 avant. Et nous ne savons pas si le résultat sera -1 ou non. Par conséquent, deux options sont possibles:



La chaîne 'B' ne fonctionne également que si cur vaut -1 . En même temps, comme nous l'avons déjà remarqué, en principe, elle ne fait rien. Mais on note quand même cette côte pour compléter le tableau:



La chaîne 'C' , comme les précédentes, fonctionne avec cur == -1 et la change arbitrairement (comme 'A' ). Mais en même temps, il peut toujours transformer la liste non vide en vide ou la laisser non vide s'il y avait plus d'un élément.



Enfin, la chaîne 'D' augmente la taille de la liste: elle peut devenir vide à non vide, ou augmenter non vide. Il ne peut pas transformer le non vide en vide:



Qu'est-ce que cela nous donne? Rien du tout. Il est totalement incompréhensible que la condition res.isEmpty() toujours vraie.


En fait, nous avons mal commencé. Dans ce cas, il ne suffit pas de surveiller séparément l'état de chaque variable. Ici, les états corrélés jouent un rôle important. Heureusement, du fait que 2+2 = 2*2 , nous n'en avons également que quatre:



Avec une double bordure, j'ai marqué l'état initial que nous avons lors de la saisie de la méthode. Eh bien, essayez encore. 'A' change ou enregistre le cur pour toute res , res ne change pas:



'B' ne fonctionne qu'avec cur == -1 && res.isEmpty() et ne fait rien. Ajouter:



'C' ne fonctionne qu'avec cur == -1 && !res.isEmpty() . En même temps, cur et res changent arbitrairement: après 'C' nous nous retrouvons dans n'importe quel état:



Enfin, 'D' peut commencer dans cur != -1 && res.isEmpty() et rendre la liste non vide, ou commencer dans cur != -1 && !res.isEmpty() et y rester:



À première vue, il semble que cela n'a fait qu'empirer: le graphique est devenu plus compliqué et on ne sait pas comment l'utiliser. Mais en fait, nous sommes proches d'une solution. Les flèches montrent maintenant tout le flux possible d'exécution de notre méthode. Puisque nous savons de quel état nous sommes partis, faisons un tour le long des flèches:



Et ici, une chose très intéressante se révèle. Nous ne pouvons pas accéder au coin inférieur gauche. Et puisque nous ne pouvons pas y entrer, cela signifie que nous ne pouvons pas marcher le long des flèches 'C' . Autrement dit, la ligne 'C' vraiment inaccessible et le 'B' peut être exécuté. Ceci n'est possible que si la condition res.isEmpty() effet toujours vraie! L'IntelliJ IDEA est tout à fait correct. Désolé, analyseur, en vain, je pensais que vous étiez buggy. Vous êtes tellement intelligent que c'est difficile pour moi, une personne ordinaire, de vous rattraper.


Notre analyseur ne dispose d'aucune technologie «hype» d'intelligence artificielle, mais utilise les approches de l'analyse des flux de contrôle et de l'analyse des flux de données, qui ne sont pas moins d'un demi-siècle. Néanmoins, il tire parfois des conclusions très non triviales. Cependant, cela est compréhensible: pendant longtemps, il vaut mieux construire des graphiques et marcher dessus avec des machines qu'avec des personnes. Il y a un problème important non résolu: il ne suffit pas de simplement dire à une personne qu'elle a une erreur de programme. Le cerveau de silicium doit expliquer au biologique pourquoi il en a décidé ainsi, et pour que le cerveau biologique le comprenne. Si quelqu'un a des idées brillantes sur la façon de procéder, je serai heureux de vous entendre. Si vous êtes prêt à réaliser vos idées vous-même, notre équipe ne refusera pas de coopérer avec vous!


L'un des tests d'acceptation est devant vous: pour cet exemple, une explication doit être générée automatiquement. Cela peut être un texte, un graphique, un arbre, une image avec des sceaux - n'importe quoi, si seulement une personne pouvait comprendre.


La question reste ouverte, que voulait dire l'auteur de la méthode et à quoi devrait ressembler le code. Les responsables du sous-système m'ont informé que cette partie est quelque peu abandonnée, et eux-mêmes ne savent pas comment la réparer ou il vaut mieux la supprimer complètement.

Source: https://habr.com/ru/post/fr472932/


All Articles