Enfin, une année 2020 aussi difficile touche à sa fin, ce qui signifie qu'il est temps de faire le point! Au cours de cette année, l'équipe PVS-Studio a rédigé de nombreux articles traitant de diverses erreurs trouvées par l'analyseur dans des projets open-source. Vous pouvez voir les plus intéressants d'entre eux ici, dans les TOP erreurs trouvées dans les projets C # en 2020. Bonne visite!
Comment le sommet a été formé
Cette liste contient les déclencheurs les plus intéressants, à mon avis, sur lesquels mes collègues et moi avons écrit dans des articles pour 2020. Un critère de sélection important était le degré de confiance qu'une erreur avait bien été commise dans le fragment de code correspondant. Et, bien sûr, lors de la sélection, ainsi que du placement des lieux, `` l'intérêt '' du déclenchement a été pris en compte, mais c'est mon opinion subjective - vous pouvez toujours le contester dans les commentaires.
J'ai essayé de rendre le top aussi diversifié que possible: à la fois en termes de messages PVS-Studio, et en termes de projets pour lesquels des avertissements de code ont été émis. La liste comprend des détections sur les sources de 8 projets vérifiés. Dans le même temps, les règles de diagnostic ne sont presque jamais répétées - vous ne pouvez rencontrer ici que V3022 et V3106 (et non, ils n'ont pas été fabriqués par moi, mais apparemment ce sont mes préférés). Ainsi, la variété est garantie ici :).
Eh bien, commençons! Top 10!
10e place - Nouvelle ancienne licence
Notre principale réponse s'ouvre à partir d'un article d' une très bonne personne sur la vérification des projets C # pour Linux et macOS, où le projet RavenDB a été utilisé comme exemple:
private static void UpdateEnvironmentVariableLicenseString(....)
{
....
if (ValidateLicense(newLicense, rsaParameters, oldLicense) == false)
return;
....
}
Avertissement de l'analyseur : V3066 Ordre incorrect possible des arguments passés à la méthode 'ValidateLicense': 'newLicense' et 'oldLicense'. LicenseHelper.cs (177) Raven.Server
Il semblerait, qu'est-ce qui ne va pas ici? Le code se compile assez bien. Pourquoi l'analyseur a-t-il décidé qu'il était nécessaire de transférer d'abord l' ancienne licence , puis la nouvelle licence ? Vous l'avez deviné, non? Jetons un coup d'œil à la déclaration ValidateLicense :
private static bool ValidateLicense(License oldLicense,
RSAParameters rsaParameters,
License newLicense)
Wow, c’est vrai - l’ancien passe d’abord dans les paramètres, puis seulement la nouvelle licence. Maintenant, dites-moi, votre analyse dynamique peut-elle comprendre cela? :)
En tout cas, le déclenchement est intéressant. Peut-être que l'ordre n'est pas vraiment important là-bas, mais il vaut mieux revérifier ces fragments, d'accord?
9e place - 'FirstOrDefault' et 'null' inattendu
A la 9ème place, la réponse de l'article " Jouez dans" osu! ", N'oubliez pas les erreurs ", écrit au début de l'année en cours:
public ScoreInfo CreateScoreInfo(RulesetStore rulesets)
{
var ruleset = rulesets.GetRuleset(OnlineRulesetID);
var mods = Mods != null ? ruleset.CreateInstance()
.GetAllMods().Where(....)
.ToArray() : Array.Empty<Mod>();
....
}
Vous voyez l'erreur? Et elle est! Que dit l'analyseur?
Avertissement de l'analyseur : V3146 [CWE-476] Déréférence nulle possible de 'ensemble de règles'. Le 'FirstOrDefault' peut renvoyer une valeur nulle par défaut. APILegacyScoreInfo.cs 24
Oui, oui, encore une fois, je n'ai pas donné toutes les informations nécessaires à la fois. En fait, vous ne voyez vraiment rien de suspect dans ce code. Après tout, FirstOrDefault , dont l'analyseur nous parle, est dans la définition de la méthode GetRuleset :
public RulesetInfo GetRuleset(int id) =>
AvailableRulesets.FirstOrDefault(....);
Terrible affaire! La méthode retournera RulesetInfo si elle en trouve un approprié. Et sinon? Retourne calmement null . Et il tirera déjà à l'endroit où le résultat de l'appel sera utilisé. Dans ce cas, lors de l'appel de ruleset.CreateInstance () .
La question peut se poser: et s'il n'y a jamais nul ? Que faire si la collection contient toujours l'élément requis? Eh bien, si le développeur en est sûr, pourquoi ne pas utiliser First au lieu de FirstOrDefault ?
8e place - Bonjour de Python
Le dernier déclencheur des trois premiers a été émis pour le code de projet RunUO. Un article sur la façon de le vérifier a été rédigé en février et est disponible ici .
Le fragment trouvé est extrêmement suspect, même s'il est difficile de dire avec certitude s'il s'agit d'une erreur:
public override void OnCast()
{
if ( Core.AOS )
{
damage = m.Hits / 2;
if ( !m.Player )
damage = Math.Max( Math.Min( damage, 100 ), 15 );
damage += Utility.RandomMinMax( 0, 15 );
}
else { .... }
}
Avertissement analyseur : V3043 La logique de fonctionnement du code ne correspond pas à son formatage. L'instruction est en retrait vers la droite, mais elle est toujours exécutée. Il est possible que des accolades manquent. Earthquake.cs 57
C'est vrai - c'est une question d'indentation! Il semble que la ligne damage + = Utility.RandomMinMax (0, 15) n'aurait dû être exécutée que si m.Player est faux... Le code Python fonctionnerait de la même manière, où l'indentation est écrite non seulement pour la beauté, mais aussi pour définir la logique de l'application. Mais le compilateur C # a une opinion différente à ce sujet! Je me demande quelle était l'opinion du développeur?
En général, dans cette situation, il existe 2 options. Soit les accolades manquent vraiment ici, et tout ne fonctionne pas correctement, soit tout fonctionne correctement, mais avec le temps, il y aura certainement une personne qui considérera cela comme une erreur et la "réparera".
Peut-être que je me trompe, et il y a vraiment des moments où il est acceptable d'écrire quelque chose comme ça. Si vous êtes au courant de tels cas, veuillez écrire un commentaire - je serais vraiment intéressé de connaître ces cas.
7e place - Parfait ou Parfait - telle est la question!
Il devient de plus en plus difficile de diffuser des avertissements par endroits, et nous passons à la deuxième alarme de cette liste de l' article sur osu! ...
Combien de temps vous faudra-t-il pour voir l'erreur?
protected override void CheckForResult(....)
{
....
ApplyResult(r =>
{
if ( holdNote.hasBroken
&& (result == HitResult.Perfect || result == HitResult.Perfect))
result = HitResult.Good;
....
});
}
Avertissement de l'analyseur : V3001 Il existe des sous-expressions identiques 'result == HitResult.Perfect' à gauche et à droite de '||' opérateur. DrawableHoldNote.cs 266 Pas
grand-chose, je pense, il vous suffit de lire l'avertissement PVS-Studio. Les développeurs utilisant l'analyse statique le font généralement :). On pourrait discuter des places précédentes dans le top, mais ici l'erreur est évidente. Il est difficile de dire quel élément particulier de l'énumération HitResult aurait dû être utilisé ici au lieu du deuxième Perfect (ou au lieu du premier), mais à la fin, quelque chose était clairement mal écrit. Eh bien, ça va - l'erreur a été trouvée, ce qui signifie qu'elle peut être facilement corrigée.
6ème place - nul (pas) passe!
La 6ème place était une réponse très cool au code du projet Open XML SDK. Vous pouvez lire l'article sur la vérification ici .
Le développeur voulait implémenter une propriété qui ne retournerait pas null , même si elle était affectée directement. Et c'est vraiment génial quand on peut être sûr que null ne sera écrit en aucun cas. C'est dommage que ce ne soit pas du tout la situation:
internal string RawOuterXml
{
get => _rawOuterXml;
set
{
if (string.IsNullOrEmpty(value))
{
_rawOuterXml = string.Empty;
}
_rawOuterXml = value;
}
}
Avertissement de l'analyseur : V3008 La variable '_rawOuterXml' se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 164, 161. OpenXmlElement.cs 164
Il s'avère que dans _rawOuterXml sera valeur enregistrée indépendamment du fait qu'elle soit nulle ou non. Si vous regardez ce code sans attention, vous pourriez penser que null ne sera jamais écrit dans cette propriété - après tout, il y a une vérification! Eh bien, si tel est le cas, sous l'arbre, vous ne pouvez pas trouver de cadeaux, mais une exception NullReferenceException inattendue :(
5e place - Tiré à partir d'un tableau avec un tableau à l'intérieur
Le top 5 des déclenchements de 2020 a été émis par l'analyseur du projet TensorFlow.NET que j'ai personnellement testé. En cliquant sur le lien , vous pouvez lire l'article sur la vérification de ce projet (oh, et j'y ai beaucoup vu tout le monde).
Au fait, si vous aimez regarder des exemples d'erreurs intéressantes de vrais projets C #, alors je vous suggère de vous abonner à mon twitter . Là, je prévois de publier des déclencheurs curieux et juste des fragments de code intéressants, dont beaucoup, hélas, ne seront pas inclus dans les articles. Je serai ravi de vous voir! :)
Eh bien, passons maintenant au déclenchement:
public TensorShape(int[][] dims)
{
if(dims.Length == 1)
{
switch (dims[0].Length)
{
case 0: shape = new Shape(new int[0]); break;
case 1: shape = Shape.Vector((int)dims[0][0]); break;
case 2: shape = Shape.Matrix(dims[0][0], dims[1][2]); break; // <=
default: shape = new Shape(dims[0]); break;
}
}
else
{
throw new NotImplementedException("TensorShape int[][] dims");
}
}
Avertissement de l'analyseur : V3106 Peut-être que l'index est hors limite. L'index «1» pointe au-delà de la limite «dims». TensorShape.cs 107
En fait, il était très difficile de choisir où placer ce déclencheur, car il est vraiment intéressant, mais d'autres ne sont pas loin derrière à cet égard. Alors, voyons ce qui se passe ici.
Si le nombre de tableaux dans dims n'est pas égal à 1, une exception du type NotImplementedException est levée . Et que se passera-t-il si dans l' obscuritéexactement un tableau? Le nombre d'éléments de ce «tableau dans un tableau» est vérifié. Notez ce qui se passe quand il est 2. Comme l'un des arguments du constructeur Shape.Matrix , de manière inattendue, dims [1] [2] est passé . Maintenant, rappelons-nous - combien d'éléments y avait-il dans le tableau dims ?
C'est vrai, exactement un - nous venons de le vérifier! Une tentative d'obtenir le deuxième élément d'un tableau, qui n'a qu'un seul élément, lèvera une exception IndexOutOfRangeException . De toute évidence une erreur. Mais y a-t-il un moyen évident de résoudre ce problème?
La première chose qui me vient à l'esprit est le changement dims [1] [2] à dims [0] [2] . L'erreur disparaîtra-t-elle? Peu importe comment c'est! Il y aura à nouveau la même exception. Mais cette fois, il sera lié au fait que dans ce cas-branche le nombre d'éléments dans dims [0] est égal à 2. Le développeur a-t-il commis deux erreurs dans l'index d'affilée? Ou devrait-on utiliser une autre variable ici? Qui sait ... Le travail de l'analyseur est de trouver une erreur, et la personne qui l'a faite ou ses collègues devront la corriger.
4ème place - Propriété d'un objet qui n'existe pas
Un autre déclencheur a atteint ce sommet de l' article sur la vérification OpenRA . Peut-être qu'il mérite plus, mais par la volonté du destin, ce déclencheur était à la 4ème place. Eh bien, c'est plutôt bien aussi! Voyons quelle erreur PVS-Studio a pu détecter cette fois:
public ConnectionSwitchModLogic(....)
{
....
var logo = panel.GetOrNull<RGBASpriteWidget>("MOD_ICON");
if (logo != null)
{
logo.GetSprite = () =>
{
....
};
}
if (logo != null && mod.Icon == null) // <=
{
// Hide the logo and center just the text
if (title != null)
title.Bounds.X = logo.Bounds.Left;
if (version != null)
version.Bounds.X = logo.Bounds.X;
width -= logo.Bounds.Width;
}
else
{
// Add an equal logo margin on the right of the text
width += logo.Bounds.Width; // <=
}
....
}
Avertissement de l'analyseur : V3125 L'objet 'logo' a été utilisé après avoir été vérifié par rapport à null. Vérifiez les lignes: 236, 222. ConnectionLogic.cs 236
À quoi devez-vous faire attention ici? Eh bien, pour commencer, le logo peut probablement contenir null . Cela est indiqué à la fois par les vérifications des constantes et par le nom de la méthode GetOrNull , qui renvoie la valeur écrite dans le logo . Si tel est le cas, réfléchissons à ce qui se passe si GetOrNull renvoie vraiment null . Au début, tout est en ordre, mais ensuite la condition est vérifiée logo! = null && mod.Icon == null . Comme vous pouvez l'imaginer, le résultat sera une transition vers le else -branch et ... Une tentative d'accès à la propriété Bounds de la variable dans laquelle null est écrit , puis - bdysh! NullReferenceException frappe à la porte.
3e place - élément de Schrödinger
Enfin, nous arrivons aux trois premiers finalistes. Les 3 principaux bogues pour 2020 ont été trouvés dans le projet Nethermind, à propos de la vérification dont un article a été écrit avec le titre intrigant " Code en une ligne ou vérification de Nethermind en utilisant PVS-Studio C # pour Linux ". L'erreur est incroyablement simple, mais invisible à l'œil humain, surtout si l'on considère la taille du projet. Pensez-vous que ce déclencheur mérite sa place?
public ReceiptsMessage Deserialize(byte[] bytes)
{
if (bytes.Length == 0 && bytes[0] == Rlp.OfEmptySequence[0])
return new ReceiptsMessage(null);
....
}
Avertissement de l'analyseur : V3106 Peut-être que l'index est hors limite. L'index «0» pointe au-delà de la limite «octets». Nethermind.Network ReceiptsMessageSerializer.cs 50
Probablement pouvoir prendre la première chose dans une boîte vide serait cool, mais ici un tel désir ne mènera qu'à la levée d' une IndexOutOfRangeException . Juste une bagatelle - une petite erreur dans l'opérateur, et l'application ne fonctionne déjà pas correctement, ou peut-être même se bloque.
Évidemment, vous devriez utiliser «||» au lieu de «&&». De telles erreurs logiques ne sont pas rares, en particulier dans les conceptions complexes. Par conséquent, il est assez pratique de vérifier ces moments en mode automatique.
2e place - Moins de 2, mais plus de 3
En second lieu, j'ai mis un autre déclencheur sur le code du projet RavenDB. Permettez-moi de vous rappeler que vous pouvez lire les résultats de sa vérification (et pas seulement) dans l' article correspondant .
Eh bien, maintenant bienvenue - la principale erreur 2 de 2020:
private OrderByField ExtractOrderByFromMethod(....)
{
....
if (me.Arguments.Count < 2 && me.Arguments.Count > 3)
throw new InvalidQueryException(....);
....
}
Avertissement de l'analyseur : L' expression V3022 «me.Arguments.Count <2 && me.Arguments.Count> 3» est toujours fausse. Probablement le "||" l'opérateur doit être utilisé ici. QueryMetadata.cs (861) Raven.Server
Précédemment, nous avons examiné les moments où une exception inattendue a été levée, mais maintenant, au contraire, l'exception attendue ne sera jamais levée. Eh bien, ou il sera toujours rejeté si quelqu'un trouve un nombre inférieur à 2, mais supérieur à 3.
Je ne serais pas surpris si vous n'êtes pas d'accord, mais j'aime vraiment ce déclencheur plus que tous les précédents. Oui, l'erreur est incroyablement simple, et pour la corriger, il vous suffit de changer d'opérateur. En passant, cela est également indiqué par le message passé au constructeur InvalidQueryException : " Appel ORDER BY 'spatial.distance (from, to, roundFactor)' non valide, 2-3 arguments attendus, obtenu" + me.Arguments.Count .
Je suis d'accord, c'est un oubli élémentaire, mais personne ne l'a remarqué ou corrigé, du moins jusqu'à ce qu'il soit trouvé à l'aide de PVS-Studio. Cela me rappelle que les programmeurs, peu importe leur expérience, sont toujours (malheureusement?) Humains. Et les gens, quelles que soient leurs qualifications, peuvent rater même de telles erreurs stupides pour diverses raisons. Parfois, l'erreur se déclenche immédiatement et parfois vous pouvez deviner longtemps pourquoi l'utilisateur ne voit pas le message concernant l'appel ORDER BY incorrect.
1ère place - Citations pour + 100% de sécurité du code
Hourra, hourra, hourra! Nous sommes finalement arrivés à la gâchette, que j'ai trouvée la plus intéressante, drôle, cool et ainsi de suite. Il a été publié pour le code du projet ONLYOFFICE, dont l'analyse est associée à l'un des articles les plus récents de cette année - " ONLYOFFICE Community Server: Comment les bogues contribuent aux problèmes de sécurité ".
Eh bien, je vous présente l'histoire la plus triste d' ArgumentException , qui ne sera jamais lancée:
public void SetCredentials(string userName, string password, string domain)
{
if (string.IsNullOrEmpty(userName))
{
throw new ArgumentException("Empty user name.", "userName");
}
if (string.IsNullOrEmpty("password"))
{
throw new ArgumentException("Empty password.", "password");
}
CredentialsUserName = userName;
CredentialsUserPassword = password;
CredentialsDomain = domain;
}
Avertissement de l'analyseur : L' expression V3022 'string.IsNullOrEmpty ("password")' est toujours fausse. SmtpSettings.cs 104
Il était très difficile de choisir quelle erreur mettre à quel endroit, mais ce déclencheur pour moi depuis le début a été le leader parmi tous. La faute de frappe mineure la plus simple - et le code fonctionne déjà incorrectement. Ni la mise en évidence dans l'IDE, ni la revue, ni le bon vieux bon sens n'ont aidé. C'est une petite fonction simple et magnifiquement écrite. Et même ici, PVS-Studio a pu trouver ce qui manquait aux professionnels.
Comme d'habitude, le diable est dans les détails. Ne serait-il pas formidable que tous ces détails soient recherchés automatiquement? Bien sûr que ce serait le cas! Et laissez le développeur faire ce qu'un analyseur statique ne peut pas faire - il crée de nouvelles applications belles et sûres. Crée sans se demander s'il a mis des guillemets supplémentaires lors de la vérification d'une variable ou non.
Conclusion
Trouver 10 erreurs intéressantes dans les articles de 2020 était facile. Mais les distribuer par endroits s'est avéré être une autre tâche. D'une part, certains déclencheurs reflètent mieux le travail des mécanismes d'analyseurs avancés. D'un autre côté, certaines des erreurs semblent simplement amusantes dans une certaine mesure. Bon nombre des positions présentées pourraient être inversées - par exemple, les 2 premiers et les 3 premiers.
Ou peut-être pensez-vous qu'il devrait y avoir d'autres points positifs sur cette liste? En fait, vous pouvez même créer votre propre top en suivant le lienà la liste des articles et trouvez les déclencheurs les plus délicieux à votre avis. Dans ce cas, n'oubliez pas de jeter vos tops 2020 dans les commentaires, je vais le lire avec grand plaisir. Pouvez-vous faire une liste plus intéressante que la mienne?
Bien entendu, la question de «l'intérêt» des avertissements est de toute façon subjective. À mon avis, le critère principal pour évaluer la réponse est de savoir si un programmeur qui voit un avertissement de PVS-Studio changera quelque chose dans le code correspondant? Cette liste a simplement été compilée à partir de déclencheurs pour les fragments, ce qui, à mon avis, serait meilleur si les développeurs utilisaient une analyse statique. De plus, il n'y a aucun problème à essayer PVS-Studio en vérifiant vos propres projets ou d'autres. Il vous suffit de suivre le lien, téléchargez-y la version requise de l'analyseur et demandez une clé d'essai en remplissant un petit formulaire.
Eh bien, c'est tout pour moi, merci beaucoup pour votre attention et à bientôt!
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien de traduction: Nikita Lipilin. Top-10 des bogues trouvés dans les projets C # en 2020 .