Top 10 des bugs trouvés dans les projets C # en 2019

Salut à tous les fans de bugs! La nouvelle année arrive bientôt, il est donc temps de faire le bilan de l'année sortante. Par tradition, nous sommes heureux de présenter la première liste des erreurs trouvées par l'équipe PVS-Studio dans les projets C # ouverts en 2019. Prêt? Alors allons-y.

Dixième place "Tromper tout le monde"

V3066 Ordre incorrect possible des arguments passés à la méthode 'AdjustCellBorderStyle': 'isFirstDisplayedRow' et 'isFirstDisplayedColumn'. DataGridViewComboBoxCell.cs 1934

protected override void OnMouseMove(DataGridViewCellMouseEventArgs e) { .... dgvabsEffective = AdjustCellBorderStyle( DataGridView.AdvancedCellBorderStyle, dgvabsPlaceholder, singleVerticalBorderAdded, singleHorizontalBorderAdded, isFirstDisplayedRow, // <= isFirstDisplayedColumn); // <= .... } 

L'erreur de l'article " WinForms: Erreurs, Holmes ". L'analyseur souligne que lorsque deux derniers arguments de la méthode sont mélangés. Regardons la déclaration AdjustCellBorderStyle :

 public virtual DataGridViewAdvancedBorderStyle AdjustCellBorderStyle( DataGridViewAdvancedBorderStyledataGridViewAdvancedBorderStyleInput, DataGridViewAdvancedBorderStyle dataGridViewAdvancedBorderStylePlaceholder, bool singleVerticalBorderAdded, bool singleHorizontalBorderAdded, bool isFirstDisplayedColumn, bool isFirstDisplayedRow) { .... } 

On dirait que l'analyseur a raison. Souvent, les développeurs transmettent intentionnellement certains arguments dans l'ordre inverse, par exemple, pour permuter les variables. Mais il ne semble pas que ce soit le cas. Tout d'abord, les variables de type booléen sont mélangées. Deuxièmement, il n'y a pas de noms de méthodes inhabituelles: pas de "Swap" ou "Reverse". De plus, ce n'est pas si difficile de faire une erreur comme celle-ci: les gens perçoivent différemment l'ordre de classement "ligne / colonne".

Neuvième place "Si proche de l'éternité"

V3110 Récursion infinie possible dans la méthode 'TryValidateModel'. PrefixedModuleUpdater.cs 48

 public bool TryValidateModel(object model, string prefix) { return TryValidateModel(model, Prefix(prefix)); } 

L'erreur de l'article " Analyse du code d'Orchard CMS pour les bogues ". Il y a eu une erreur qui a conduit à une récursion infinie. Pour comprendre la façon dont l'erreur a été commise, il faut considérer la surcharge de la méthode TryValidateModel :

 public bool TryValidateModel(object model) { return _updateModel.TryValidateModel(model); } 

Il est probable que le premier cas devrait également utiliser un tel appel:

 public bool TryValidateModel(object model, string prefix) { return _updateModel.TryValidateModel(model, Prefix(prefix)); } 

Le code a été compilé avec succès, car _updateModel est du type IUpdateModel et la classe actuelle implémente également l'interface IUpdateModel .

Huitième place "Trouvez-moi si vous le pouvez"

V3091 Analyse empirique. Il est possible qu'une faute de frappe soit présente à l'intérieur du littéral de chaîne: "ID du groupe de gestion". Le mot «Id» est suspect. Constants.cs 36

 public class HelpMessages { public const string SubscriptionId = "Subscription Id of the subscription associated with the management"; public const string GroupId = "Management Group Id"; // <= public const string Recurse = "Recursively list the children of the management group"; public const string ParentId = "Parent Id of the management group"; public const string GroupName = "Management Group Id"; // <= public const string DisplayName = "Display Name of the management group"; public const string Expand = "Expand the output to list the children of the management group"; public const string Force = "Force the action and skip confirmations"; public const string InputObject = "Input Object from the Get call"; public const string ParentObject = "Parent Object"; } 

L'erreur de l'article " Azure PowerShell: principalement inoffensif ". L'analyseur a suspecté la constante GroupName d'être initialisée par une chaîne incorrecte. Il devrait probablement y avoir quelque chose comme "Nom du groupe de gestion" . La criticité de cette erreur est encore discutable, mais l'erreur est certainement rare et difficile à détecter.

Septième place "Juste sous-estimée"

V3078 L'ordre de tri d'origine sera perdu après un appel répétitif à la méthode 'OrderBy'. Utilisez la méthode 'ThenBy' pour conserver le tri d'origine. GridModel.Selection.cs 107

 internal partial class GridModel { private void BuildCellSelectionRegions(....) { .... this.MergeCellSelectionRegions(selectedItemsInView .OrderBy(c => c.Column.ItemInfo.LayoutInfo.Line) .OrderBy(c => c.RowItemInfo.LayoutInfo.Line)); } } 

L'erreur de l'article " Vérification de l'interface utilisateur de Telerik pour UWP comme moyen de démarrer avec PVS-Studio ." Le résultat du tri précédent sera perdu en raison d'un appel répété à OrderBy sur la collection déjà triée. Il faut utiliser ThenBy dans ce cas:

 this.MergeCellSelectionRegions(selectedItemsInView .OrderBy(c => c.Column.ItemInfo.LayoutInfo.Line) .ThenBy(c => c.RowItemInfo.LayoutInfo.Line)); 

De telles erreurs sont faites par inattention ou par ignorance. Je pense que le copier-coller est à blâmer ici.

Sixième place "Le code est documenté", ont-ils déclaré

V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. MaskedTextProvider.cs 1529

 public bool Remove(out int testPosition, out MaskedTextResultHint resultHint) { .... if (lastAssignedPos == INVALID_INDEX) { .... return true; // nothing to remove. } .... return true; } 

L'erreur de l'article " Vérification du code source des bibliothèques .NET Core par l'analyseur statique PVS-Studio ". La méthode retournera toujours true . Oui, c'est une erreur, mais il y a une autre chose qui est vraiment curieuse. La méthode est suivie du commentaire détaillé:

Supprime le dernier caractère de la chaîne formatée. (Supprimer le dernier caractère de la chaîne virtuelle). A la sortie, le paramètre out contient la position où l'opération a été réellement effectuée. Cette position est relative à la chaîne de test. Le paramètre MaskedTextResultHint out donne plus d'informations sur le résultat de l'opération. Renvoie vrai en cas de succès, faux sinon.

Faites attention à la dernière phrase. Qui lit même ces commentaires? Néanmoins, si nous prenons cela au sérieux, une telle erreur est facilement insinuée, par exemple, lors de la refactorisation ou du débogage. Eh bien, les auteurs ont voulu vérifier la variante lorsque le résultat de la méthode est toujours vrai, mais ont oublié de tout remettre en l'état.

Cinquième place "Indexez-moi maintenant!"

V3102 Accès suspect à l'élément de l'objet 'seq' par un index constant à l'intérieur d'une boucle. XmlQueryRuntime.cs 738

 public bool MatchesXmlType(IList<XPathItem> seq, ....) { .... for (int i = 0; i < seq.Count; i++) { if (!CreateXmlType(seq[0]).IsSubtypeOf(....)) return false; } return true; } 

L'erreur de l'article " Vérification du code source des bibliothèques .NET Core par l'analyseur statique PVS-Studio ". Lors de la traversée de la collection seq dans la boucle for , le développeur utilise par erreur l'accès uniquement à son premier élément sur toutes les itérations (index 0 au lieu de i ).

Quatrième place "Juste un dollar à court"

V3138 Le littéral de chaîne contient une expression interpolée potentielle. Envisagez d'inspecter: e. SSPIHandleCache.cs 42

 internal static void CacheCredential(SafeFreeCredentials newHandle) { try { .... } catch (Exception e) { if (!ExceptionCheck.IsFatal(e)) { NetEventSource.Fail(null, "Attempted to throw: {e}"); } } } 

L'erreur de l'article " Vérification du code source des bibliothèques .NET Core par l'analyseur statique PVS-Studio ". Apparemment, la chaîne "Tentative to throw: {e}" doit être interpolée. En raison du caractère $ manquant, la représentation en ligne de l'exception e ne sera pas mise dans la chaîne. Par conséquent, la ligne sera utilisée "telle quelle".

Troisième place "Il n'y a pas d'issue"

V3008 [CWE-563] La variable 'this.linker.s3.region' reçoit des valeurs successives deux fois. C'est peut-être une erreur. Vérifiez les lignes: 116, 114. AWSSDK.DynamoDBv2.Net45 S3Link.cs 116

 public string Region { get { .... } set { if (String.IsNullOrEmpty(value)) { this.linker.s3.region = "us-east-1"; } this.linker.s3.region = value; } } 

L'erreur de l'article « Recherche d'erreurs dans le code source Amazon Web Services SDK pour .NET ». Le retour a été manqué dans le corps du bloc if . Par conséquent, la variable this.linker.s3.region obtiendra toujours une valeur, y compris une ligne vide et null.

Deuxième place "Robe droite!"

V3070 La variable non initialisée 'LANG_USER_DEFAULT' est utilisée lors de l'initialisation de la variable 'LOCALE_USER_DEFAULT'. NativeMethods.cs 890

 internal static class NativeMethods { .... public static readonly int LOCALE_USER_DEFAULT = MAKELCID(LANG_USER_DEFAULT); public static readonly int LANG_USER_DEFAULT = MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT); .... } 

L'erreur de l'article " WinForms: Erreurs, Holmes ". L'ordre d'initialisation des champs de classe est confus. Pour calculer la valeur du champ LOCALE_USER_DEFAULT , le champ LANG_USER_DEFAULT est utilisé, qui n'est pas encore initialisé pour le moment et vaut 0. La variable n'est utilisée nulle part plus loin dans le code. Pour savoir à quoi cette erreur conduit, un programme de test complet contenant des méthodes du code WinForms a été écrit. Au lieu de certaines constantes utilisées, leurs valeurs réelles ont été substituées à la simplicité:

 internal static class NativeMethods { public static readonly int LOCALE_USER_DEFAULT = MAKELCID(LANG_USER_DEFAULT); public static readonly int LANG_USER_DEFAULT = MAKELANGID(0x00, 0x01); public static int MAKELANGID(int primary, int sub) { return ((((ushort)(sub)) << 10) | (ushort)(primary)); } public static int MAKELCID(int lgid) { return MAKELCID(lgid, 0x0); } public static int MAKELCID(int lgid, int sort) { return ((0xFFFF & lgid) | (((0x000f) & sort) << 16)); } } class Program { static void Main() { System.Console.WriteLine(NativeMethods.LOCALE_USER_DEFAULT); } } 

À la suite de l'exécution, nous aurons les éléments suivants: 0. Corrigeons maintenant l'erreur en échangeant la déclaration des champs LOCALE_USER_DEFAULT et LANG_USER_DEFAULT . Résultat de l'exécution du programme: 1024.

Première place "Essayez d'abord, puis faites confiance"

Ce n'est jamais facile avec la première place. Il doit y avoir quelque chose d'extraordinaire et de captivant ici. Au départ, pour cet article, j'ai sélectionné plus de vingt erreurs intéressantes, mais il n'y avait rien de digne de la première place parmi elles. C'est à ce moment que j'ai rappelé l'article de mon collègue Sergey Vasiliev. L'article n'a développé qu'une seule erreur. La beauté de cette erreur est qu'elle a directement influencé le travail de notre analyseur. Comment? Vous pouvez déjà l'obtenir à partir du titre de l'article: " L'histoire de la façon dont PVS-Studio a trouvé une erreur dans la bibliothèque utilisée dans ... PVS-Studio ". Voici où je me suis senti trop paresseux pour donner la description de l'erreur et je vous suggère donc de suivre le lien et de découvrir les détails. :) Je vous garantis que ça vaut le coup. De plus, l'article est court.

Conclusion

J'espère que les erreurs étaient en suspens pour vous et que l'article n'était pas fatigant. Juste pour mémoire, vous pouvez toujours télécharger l'analyseur PVS-Studio pour trouver des bogues dans vos projets et ceux de tiers pour vous faire plaisir, vos collègues et tout Tom, Dick ou Harry. Que les erreurs soient moindres et le temps de s'améliorer - plus! :)

Avez-vous lu jusqu'à la fin? Mes félicitations pour avoir atteint le nouveau niveau! Ne manquez pas nos articles upcomimg dans notre blog - meilleurs bugs dans les projets Java et C ++ trouvés en 2019.