Nous aimons rechercher les erreurs dans les projets Microsoft. Pourquoi? C'est simple: leurs projets sont généralement faciles à vérifier (vous pouvez travailler dans un environnement Visual Studio pour lequel PVS-Studio a un plugin pratique) et ils contiennent peu d'erreurs. C'est pourquoi l'algorithme de travail habituel est le suivant: rechercher et télécharger un projet open source depuis MS; vérifiez-le; choisissez des erreurs intéressantes; assurez-vous qu'il y en a peu; écrire un article sans oublier de féliciter les développeurs. Super! Gagnant-gagnant-gagnant: cela a pris un peu de temps, les patrons sont heureux de voir de nouveaux documents sur le blog, et le karma va bien. Mais cette fois "quelque chose s'est mal passé". Voyons ce que nous avons trouvé dans le code source de Windows Forms et si nous devons dire du bien de Microsoft cette fois.
PrésentationDébut décembre 2018, Microsoft a
annoncé la sortie de l'aperçu 1 de .NET Core 3. Un peu plus tôt (vers la mi-octobre), GitHub a commencé à divulguer activement les sources de
Windows Forms - la plate-forme d'interface utilisateur .NET Core pour créer des applications de bureau Windows . Vous pouvez voir les statistiques de validation
ici . Maintenant, n'importe qui peut télécharger le code source de WinForms pour examen.
J'ai également téléchargé les sources pour y rechercher des erreurs avec PVS-Studio. Le contrôle n'a pas posé de problème. Nous avions besoin de: Visual Studio 2019, .NET Core 3.0 SDK Preview, PVS-Studio. Et nous avons ici le journal des avertissements de l'analyseur.
Après avoir reçu le rapport PVS-Studio, je le trie généralement par numéros de diagnostic dans l'ordre croissant (la fenêtre avec le journal des messages PVS-Studio dans l'environnement Visual Studio propose différentes options de tri et de filtrage de la liste). Il vous permet de travailler avec des groupes d'erreurs similaires, ce qui simplifie considérablement l'analyse du code source. Je marque des erreurs intéressantes dans la liste avec une "étoile" et seulement ensuite, après avoir analysé tout le journal, j'écris des fragments de code et les décris. Puisqu'il y a généralement peu d'erreurs, je les "remue" en essayant de placer les plus intéressantes au début et à la fin de l'article. Mais cette fois, il s'est avéré qu'il y avait beaucoup d'erreurs (eh, l'intrigue n'a pas été sauvée depuis longtemps) et je vais les citer dans l'ordre des nombres de diagnostics.
Qu'avons-nous trouvé? 833 avertissements élevés et moyens (249 et 584, respectivement) ont été émis pour 540 000 lignes de code (sans compter les vides) dans des fichiers de 1670 cs. Et oui, traditionnellement, je ne vérifiais pas les tests et je ne prenais pas en compte les avertissements Low (il y en avait 215). Selon mes observations précédentes, les avertissements sont trop nombreux pour le projet MS. Mais tous les avertissements ne sont pas des erreurs.
Pour ce projet, le nombre de fausses alarmes était d'environ 30%. Dans environ 20% des cas, je ne pouvais tout simplement pas conclure exactement s'il s'agissait d'une erreur ou non car je ne connaissais pas assez bien le code. Et au moins 20% des erreurs que j'ai manquées peuvent être considérées comme "facteur humain": hâte, fatigue, etc. Soit dit en passant, l'effet inverse est également possible: certains déclencheurs de même type, dont le nombre pourrait atteindre 70-80, j'ai regardé "à côté d'un", ce qui pourrait parfois augmenter le nombre d'erreurs que je pensais réelles.
Quoi qu'il en soit, 30% des avertissements indiquent de vraies erreurs, ce qui est un pourcentage assez élevé si l'on tient compte du fait que l'analyseur n'était pas préconfiguré.
Donc, le nombre d'erreurs que j'ai réussi à trouver était d'environ 240, ce qui est dans la plage des statistiques données. Encore une fois, à mon avis, ce n'est pas le résultat le plus remarquable pour un projet MS (même s'il ne fera que 0,44 erreur pour 1000 lignes de code) et il y a probablement plus d'erreurs réelles dans le code WinForms également. Je suggère de considérer les raisons à la fin de l'article et voyons maintenant les erreurs les plus intéressantes.
Des erreursPVS-Studio:
V3003 L'utilisation du
modèle 'if (A) {...} else if (A) {...}' a été détectée. Il y a une probabilité de présence d'erreur logique. Vérifiez les lignes: 213, 224. ButtonStandardAdapter.cs 213
void PaintWorker(PaintEventArgs e, bool up, CheckState state) { up = up && state == CheckState.Unchecked; .... if (up & IsHighContrastHighlighted()) { .... } else if (up & IsHighContrastHighlighted()) { .... } else { .... } .... }
Si et
sinon si les blocs vérifient la même condition. Cela ressemble à du copier-coller. Est-ce une erreur? Si vous regardez la déclaration de la méthode
IsHighContrastHighlighted , vous pouvez en douter:
protected bool IsHighContrastHighlighted() { return SystemInformation.HighContrast && Application.RenderWithVisualStyles && (Control.Focused || Control.MouseIsOver || (Control.IsDefault && Control.Enabled)); }
La méthode peut probablement renvoyer différentes valeurs pour les appels séquentiels. Et ce qui se passe dans la méthode de l'appelant, bien sûr, semble étrange, mais a le droit d'exister. Cependant, je conseillerais aux auteurs de jeter un œil à ce fragment de code. Juste au cas où. C'est également un bon exemple de la difficulté de tirer des conclusions lors de l'analyse de code inconnu.
PVS-Studio:
V3004 L'
instruction 'then' est équivalente à l'instruction 'else'. RichTextBox.cs 1018
public int SelectionCharOffset { get { int selCharOffset = 0; .... NativeMethods.CHARFORMATA cf = GetCharFormat(true);
Et il y a certainement une erreur de copier-coller ici. Quelle que soit la condition, la variable
selCharOffset obtiendra toujours la même valeur.
Il existe deux autres erreurs de ce type dans le code WinForms:
- V3004 L'instruction 'then' est équivalente à l'instruction 'else'. SplitContainer.cs 1700
- V3004 L'instruction 'then' est équivalente à l'instruction 'else'. ToolstripProfessionalRenderer.cs 371
PVS-Studio:
V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 681, 680. ProfessionalColorTable.cs 681
internal void InitSystemColors(ref Dictionary<KnownColors, Color> rgbTable) { .... rgbTable[ProfessionalColorTable.KnownColors.msocbvcrCBBdrOuterDocked] = buttonFace; rgbTable[ProfessionalColorTable.KnownColors.msocbvcrCBBdrOuterDocked] = buttonShadow; .... }
La méthode remplit le dictionnaire
rgbTable . L'analyseur a pointé un fragment de code où différentes valeurs sont écrites deux fois sur la même clé dans l'ordre. Les choses iraient bien, mais il y a encore 16 de ces fragments dans cette méthode. Cela ne ressemble plus à une erreur unique en son genre. Mais pourquoi ils font cela reste un mystère pour moi. Je n'ai trouvé aucun signe de code généré automatiquement. Cela ressemble à ceci dans l'éditeur:
Je vais vous donner les dix premiers avertissements de la liste:
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 785, 784. ProfessionalColorTable.cs 785
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 787, 786. ProfessionalColorTable.cs 787
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 789, 788. ProfessionalColorTable.cs 789
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 791, 790. ProfessionalColorTable.cs 791
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 797, 796. ProfessionalColorTable.cs 797
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 799, 798. ProfessionalColorTable.cs 799
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 807, 806. ProfessionalColorTable.cs 807
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 815, 814. ProfessionalColorTable.cs 815
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 817, 816. ProfessionalColorTable.cs 817
- V3008 La variable se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 823, 822. ProfessionalColorTable.cs 823
PVS-Studio:
V3011 Deux conditions opposées ont été rencontrées. La deuxième condition est toujours fausse. Vérifiez les lignes: 5242, 5240. DataGrid.cs 5242
private void CheckHierarchyState() { if (checkHierarchy && listManager != null && myGridTable != null) { if (myGridTable == null)
L'opérateur de
retour ne sera jamais exécuté. Très probablement, la condition
myGridTable! = Null dans le bloc
if externe a été ajoutée ultérieurement lors du refactoring. Et maintenant, la vérification de
myGridTable == null n'a aucun sens. Pour améliorer la qualité du code, vous devez supprimer cette vérification.
PVS-Studio:
V3019 Il est possible qu'une variable incorrecte soit comparée à null après la conversion de type à l'aide du mot clé 'as'. Vérifiez les variables «gauche», «cscLeft». TypeCodeDomSerializer.cs 611
PVS-Studio:
V3019 Il est possible qu'une variable incorrecte soit comparée à null après la conversion de type à l'aide du mot clé 'as'. Vérifiez les variables «droite», «cscRight». TypeCodeDomSerializer.cs 615
public int Compare(object left, object right) { OrderedCodeStatementCollection cscLeft = left as OrderedCodeStatementCollection; OrderedCodeStatementCollection cscRight = right as OrderedCodeStatementCollection; if (left == null) { return 1; } else if (right == null) { return -1; } else if (right == left) { return 0; } return cscLeft.Order - cscRight.Order;
L'analyseur a généré deux avertissements pour la méthode
Compare à la fois. Quel est le problème? C'est que les valeurs
cscLeft et
cscRight ne sont pas vérifiées pour
null du tout. Ils peuvent obtenir cette valeur après une
conversion infructueuse vers le type
OrderedCodeStatementCollection . Ensuite, une exception sera levée dans la dernière expression de
retour . Cette situation est possible lorsque tous les contrôles de
gauche et de
droite passent et n'aboutissent pas à une sortie préalable de la méthode.
Pour corriger le code, vous devez utiliser
cscLeft / cscRight au lieu de
gauche / droite partout.
PVS-Studio:
V3020 Une «pause» inconditionnelle dans une boucle. SelectionService.cs 421
void ISelectionService.SetSelectedComponents( ICollection components, SelectionTypes selectionType) { ....
Ce fragment fait plutôt référence à "l'odeur de code". Il n'y a pas d'erreur ici. Mais des questions se posent quant à l'organisation de la boucle
foreach . Il est clair pourquoi il est nécessaire ici: en raison de la nécessité d'extraire des éléments de la collection, passés en tant que
ICollection . Mais pourquoi la boucle, initialement conçue pour une seule itération (la condition préalable est la présence d'un seul élément dans les
composants de la collection), nécessitait-elle un support supplémentaire tel que la
rupture ? Probablement, la réponse peut être considérée comme suit: "Historiquement, cela s'est produit." Le code a l'air moche.
PVS-Studio:
V3022 L' expression 'ocxState! = Null' est toujours vraie. AxHost.cs 2186
public State OcxState { .... set { .... if (value == null) { return; } .... ocxState = value; if (ocxState != null)
En raison d'une erreur logique, un "code mort" s'est produit dans ce fragment. Les expressions du bloc
else ne seront jamais exécutées.
PVS-Studio:
V3027 La variable 'e' a été utilisée dans l'expression logique avant d'être vérifiée par rapport à null dans la même expression logique. ImageEditor.cs 99
public override object EditValue(....) { .... ImageEditor e = ....; Type myClass = GetType(); if (!myClass.Equals(e.GetType()) && e != null && myClass.IsInstanceOfType(e)) { .... } .... }
La variable
e de la condition est d'abord utilisée puis vérifiée par rapport à
null . Bonjour,
NullReferenceException .
Encore une de ces erreurs:
PVS-Studio:
V3027 La variable 'dropDownItem' a été utilisée dans l'expression logique avant d'être vérifiée par rapport à null dans la même expression logique. ToolStripMenuItemDesigner.cs 1351
internal void EnterInSituEdit(ToolStripItem toolItem) { .... ToolStripDropDownItem dropDownItem = toolItem as ToolStripDropDownItem; if (!(dropDownItem.Owner is ToolStripDropDownMenu) && dropDownItem != null && dropDownItem.Bounds.Width < commitedEditorNode.Bounds.Width) { .... } .... }
La situation est similaire à la précédente mais avec la variable
dropDownItem . Je pense que de telles erreurs apparaissent à la suite d'une refonte insouciante. Probablement, une partie de la condition
! (DropDownItem.Owner est ToolStripDropDownMenu) a été ajoutée au code plus tard.
PVS-Studio:
contrôle récurrent
V3030 . La condition 'columnCount> 0' a déjà été vérifiée à la ligne 3900. ListView.cs 3903
internal ColumnHeader InsertColumn( int index, ColumnHeader ch, bool refreshSubItems) { ....
Une erreur qui peut sembler inoffensive. En effet, une vérification inutile est effectuée qui n'affecte pas la logique opérationnelle. Et parfois, cela se fait même lorsque vous devez vérifier à nouveau l'état d'un composant visuel, par exemple, obtenir le nombre d'entrées dans la liste. Mais dans ce cas, la variable
locale columnCount est vérifiée deux fois. C'est très suspect. Soit ils voulaient vérifier une autre variable, soit ils ont utilisé une mauvaise condition dans l'une des vérifications.
PVS-Studio:
V3061 Le paramètre 'lprcClipRect' est toujours réécrit dans le corps de la méthode avant d'être utilisé. WebBrowserSiteBase.cs 281
int UnsafeNativeMethods.IOleInPlaceSite.GetWindowContext( out UnsafeNativeMethods.IOleInPlaceFrame ppFrame, out UnsafeNativeMethods.IOleInPlaceUIWindow ppDoc, NativeMethods.COMRECT lprcPosRect, NativeMethods.COMRECT lprcClipRect, NativeMethods.tagOIFI lpFrameInfo) { ppDoc = null; ppFrame = Host.GetParentContainer(); lprcPosRect.left = Host.Bounds.X; lprcPosRect.top = Host.Bounds.Y; .... lprcClipRect = WebBrowserHelper.GetClipRect();
Une erreur non évidente. Oui, le paramètre
lprcClipRect est réellement initialisé avec une nouvelle valeur sans l'utiliser en aucune façon. Mais à quoi cela mène-t-il finalement? Je pense que quelque part dans le code appelant, la référence passée par ce paramètre restera inchangée, bien qu'elle ne soit pas censée l'être. Vraiment, appréciez le traitement d'autres variables dans cette méthode. Même son nom (préfixe "Get") indique qu'une certaine initialisation sera effectuée à l'intérieur de la méthode via les paramètres passés. Et il en est ainsi. Les deux premiers paramètres (
ppFrame et
ppDoc ) sont passés avec le modificateur
out et ils obtiennent de nouvelles valeurs. Les références
lprcPosRect et
lpFrameInfo sont utilisées pour accéder et initialiser les champs de classe. Seul
lprcClipRect se démarque. Le modificateur
out ou
ref est probablement requis pour ce paramètre.
PVS-Studio:
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,
L'analyseur soupçonnait que les deux derniers arguments étaient mélangés. Jetons un coup d'œil à la déclaration de la méthode
AdjustCellBorderStyle :
public virtual DataGridViewAdvancedBorderStyle AdjustCellBorderStyle( DataGridViewAdvancedBorderStyledataGridViewAdvancedBorderStyleInput, DataGridViewAdvancedBorderStyle dataGridViewAdvancedBorderStylePlaceholder, bool singleVerticalBorderAdded, bool singleHorizontalBorderAdded, bool isFirstDisplayedColumn, bool isFirstDisplayedRow) { .... }
On dirait une erreur. Oui, certains arguments sont souvent passés dans l'ordre inverse, par exemple pour échanger certaines variables. Mais je ne pense pas que ce soit le cas. Rien dans les méthodes de l'appelant ou de l'appelé n'indique ce modèle d'utilisation. Tout d'abord, les variables de type
booléen sont mélangées. Deuxièmement, les noms des méthodes sont également réguliers: pas de "Swap" ou "Reverse". D'ailleurs, ce n'est pas si difficile de se tromper comme ça. Les gens perçoivent souvent différemment l'ordre de la paire "ligne / colonne". Pour moi, par exemple, c'est la "ligne / colonne" qui est familière. Mais pour l'auteur de la méthode appelée
AdjustCellBorderStyle , évidemment, l'ordre le plus courant est "colonne / ligne".
PVS-Studio:
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); .... }
Rare erreur. L'ordre d'initialisation des champs de classe est mélangé. Pour calculer la valeur du champ
LOCALE_USER_DEFAULT, le champ
LANG_USER_DEFAULT est utilisé, qui n'est pas encore initialisé et a une valeur de 0. En passant, la variable
LANG_USER_DEFAULT n'est utilisée nulle part ailleurs dans le code. J'ai fait un effort supplémentaire et j'ai écrit un petit programme de console qui simule la situation. J'ai remplacé certaines constantes utilisées dans le code WinForms par leurs valeurs réelles:
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); } }
Par conséquent, la console affichera: 0.
Échangeons maintenant les déclarations des champs
LOCALE_USER_DEFAULT et
LANG_USER_DEFAULT . Le résultat de l'exécution du programme est le suivant: 1024. Je pense qu'il n'y a rien de plus à commenter ici.
PVS-Studio:
V3080 Déréférence nulle possible. Envisagez d'inspecter les «ces». CodeDomSerializerBase.cs 562
protected void DeserializeStatement( IDesignerSerializationManager manager, CodeStatement statement) { .... CodeExpressionStatement ces = statement as CodeExpressionStatement; if (ces != null) { .... } else { .... DeserializeExpression(manager, null, ces.Expression);
Le code qui devrait "planter" assez régulièrement, car vous pouvez entrer dans la branche
else juste au moment où la référence
ces est égale à
null .
Un autre exemple similaire:
PVS-Studio:
V3080 Déréférence nulle possible. Pensez à inspecter 'comboBox'. ComboBox.cs 6610
public void ValidateOwnerDrawRegions(ComboBox comboBox, ....) { .... if (comboBox != null) { return; } Rectangle topOwnerDrawArea = new Rectangle(0, 0, comboBox.Width, innerBorder.Top); .... }
Le code paradoxal. Apparemment, la
vérification if (comboBox! = Null) a été confondue avec
if (comboBox == null) . Et donc, nous aurons une autre
NullReferenceException.Nous avons considéré deux erreurs
V3080 assez évidentes où vous pouvez tracer visuellement une utilisation potentielle de référence nulle dans une méthode. Mais le diagnostic
V3080 est beaucoup plus efficace et peut trouver de telles erreurs pour les chaînes d'appels de méthode. Il n'y a pas si longtemps, nous avons considérablement amélioré le flux de données et les mécanismes d'analyse interprocédurale. Vous pouvez lire à ce sujet dans l'article "
Types de référence Nullable en C # 8.0 et analyse statique ". Mais voici ce genre d'erreur détectée dans WinForms:
PVS-Studio:
V3080 Déréférence nulle possible à l'intérieur de la méthode sur 'reader.NameTable'. Pensez à inspecter le 1er argument: contentReader. ResXResourceReader.cs 267
private void EnsureResData() { .... XmlTextReader contentReader = null; try { if (fileContents != null) { contentReader = new XmlTextReader(....); } else if (reader != null) { contentReader = new XmlTextReader(....); } else if (fileName != null || stream != null) { .... contentReader = new XmlTextReader(....); } SetupNameTable(contentReader);
Regardez ce qui arrive à la variable
contentReader dans le corps de la méthode. Après l'initialisation avec null, il sera à nouveau initialisé dans l'un des contrôles. Mais la série de vérifications ne se termine pas avec le bloc
else . Cela signifie que dans certains cas rares (ou en raison d'une refactorisation à l'avenir), la référence
peut toujours rester nulle. Ensuite, il sera transmis à la méthode
SetupNameTable où il est utilisé sans aucune vérification:
private void SetupNameTable(XmlReader reader) { reader.NameTable.Add(ResXResourceWriter.TypeStr); reader.NameTable.Add(ResXResourceWriter.NameStr); .... }
Il s'agit d'un code potentiellement dangereux.
Et encore une erreur où l'analyseur a dû passer par la chaîne d'appel pour détecter le problème:
PVS-Studio:
V3080 Déréférence nulle possible. Pensez à inspecter la «disposition». DockAndAnchorLayout.cs 156
private static Rectangle GetAnchorDestination( IArrangedElement element, Rectangle displayRect, bool measureOnly) { .... AnchorInfo layout = GetAnchorInfo(element); int left = layout.Left + displayRect.X; .... }
L'analyseur prétend qu'il est possible d'obtenir une référence nulle à partir de la méthode
GetAnchorInfo , ce qui provoquera une exception lors du calcul de la valeur
gauche . Passons en revue toute la chaîne d'appels et vérifions si c'est vrai:
private static AnchorInfo GetAnchorInfo(IArrangedElement element) { return (AnchorInfo)element.Properties.GetObject(s_layoutInfoProperty); } public object GetObject(int key) => GetObject(key, out _); public object GetObject(int key, out bool found) { short keyIndex = SplitKey(key, out short element); if (!LocateObjectEntry(keyIndex, out int index)) { found = false; return null; }
En effet, dans certains cas, la méthode
GetObject qui termine la chaîne d'appel retournera
null , qui sera transmise à la méthode appelante sans aucune vérification supplémentaire. Probablement, il est nécessaire de couvrir une telle situation dans la méthode
GetAnchorDestination .
Il y a beaucoup d'erreurs de ce type dans le code WinForms,
plus de 70 . Ils se ressemblent tous et je ne les décrirai pas dans l'article.
PVS-Studio:
V3091 Analyse empirique. Il est possible qu'une faute de frappe soit présente à l'intérieur du littéral de chaîne: "ShowCheckMargin". Le mot «ShowCheckMargin» est suspect. PropertyNames.cs 136
internal class PropertyNames { .... public static readonly string ShowImageMargin = "ShowCheckMargin"; ... public static readonly string ShowCheckMargin = "ShowCheckMargin"; .... }
Un bon exemple d'une erreur qui n'est pas si facile à trouver. Lors de l'initialisation des champs de classe, la même valeur est utilisée, bien que l'auteur du code n'en ait évidemment pas l'intention (le copier-coller est à blâmer). L'analyseur a tiré cette conclusion en comparant les noms des variables et les valeurs des chaînes attribuées. Je n'ai donné que des lignes avec des erreurs mais vous devriez vérifier à quoi cela ressemble dans l'éditeur de code:
La détection de ces erreurs est ce qui démontre toute la puissance et la durée d'attention sans fin des outils d'analyse statique.
PVS-Studio:
V3095 L'objet 'currentForm' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 3386, 3404. Application.cs 3386
private void RunMessageLoopInner(int reason, ApplicationContext context) { .... hwndOwner = new HandleRef( null, UnsafeNativeMethods.GetWindowLong( new HandleRef(currentForm, currentForm.Handle),
C'est classique. La variable
currentForm est utilisée sans aucun contrôle. Mais ensuite, il est vérifié pour
null dans le code. Dans ce cas, je peux vous conseiller d'être plus attentif lorsque vous travaillez avec des types de référence et d'utiliser également des analyseurs statiques :).
Encore une de ces erreurs:
PVS-Studio:
V3095 L'objet 'backgroundBrush' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 2331, 2334. DataGrid.cs 2331
public Color BackgroundColor { .... set { .... if (!value.Equals(backgroundBrush.Color))
Dans le code WinForms, j'ai rencontré
plus de 60 erreurs de ce type. À mon avis, tous sont plutôt critiques et nécessitent l'attention des développeurs. Mais il n'est plus aussi intéressant d'en parler dans l'article, je me limiterai donc aux deux mentionnés ci-dessus.
PVS-Studio:
V3125 L'objet '_propInfo' a été utilisé et a été vérifié par rapport à null dans différentes branches d'exécution. Vérifiez les lignes: 996, 982. Binding.cs 996
private void SetPropValue(object value) { .... if (....) { if .... else if (_propInfo != null) .... } else { _propInfo.SetValue(_control, value); } .... }
Par souci d'exhaustivité - également une sorte de classique, erreur
V3125 . La situation inverse. Au début, le développeur utilise une référence potentiellement
nulle en toute sécurité, après l'avoir vérifiée par rapport à null, mais arrête de le faire plus loin dans le code.
Et encore une de ces erreurs:
PVS-Studio:
V3125 L'objet 'propriétaire' a été utilisé après avoir été vérifié par rapport à null. Vérifiez les lignes: 64, 60. FlatButtonAppearance.cs 64
public int BorderSize { .... set { .... if (owner != null && owner.ParentInternal != null) { LayoutTransaction.DoLayoutIf(....); } owner.Invalidate();
Adorable Mais c'est le point de vue d'un chercheur extérieur. Après tout, l'analyseur a trouvé
plus de 50 de ces modèles dans le code WinForms en plus de ces deux
V3125 . Les développeurs ont beaucoup à travailler.
Et enfin, il y a une erreur intéressante, à mon avis.
PVS-Studio:
V3137 La variable 'hCurrentFont' est affectée mais n'est pas utilisée à la fin de la fonction. DeviceContext2.cs 241
sealed partial class DeviceContext : .... { WindowsFont selectedFont; .... internal void DisposeFont(bool disposing) { if (disposing) { DeviceContexts.RemoveDeviceContext(this); } if (selectedFont != null && selectedFont.Hfont != IntPtr.Zero) { IntPtr hCurrentFont = IntUnsafeNativeMethods.GetCurrentObject( new HandleRef(this, hDC), IntNativeMethods.OBJ_FONT); if (hCurrentFont == selectedFont.Hfont) {
Voyons ce qui a alerté l'analyseur et pourquoi cela peut indiquer un problème qu'une variable se voit attribuer une valeur, mais jamais utilisée dans le code.
Le fichier
DeviceContext2.cs contient une classe partielle. La méthode
DisposeFont est utilisée pour libérer des ressources après avoir travaillé avec des graphiques: le contexte de l'appareil et les polices. Pour une meilleure compréhension, j'ai donné toute la méthode
DisposeFont . Faites attention à la variable locale
hCurrentFont . Le problème est que la déclaration de cette variable dans la méthode masque le champ de classe du même nom. J'ai trouvé deux méthodes de la classe
DeviceContext où le
champ avec le nom
hCurrentFont est utilisé:
public IntPtr SelectFont(WindowsFont font) { .... hCurrentFont = font.Hfont; .... } public void ResetFont() { .... hCurrentFont = hInitialFont; }
Regardez la méthode
ResetFont . La dernière ligne
indique exactement ce que fait la méthode
DisposeFont dans le sous-bloc (c'est ce que l'analyseur pointe). Ce champ
hCurrentFont du même nom est déclaré dans une autre partie de la classe partielle dans le fichier
DeviceContext.cs :
sealed partial class DeviceContext : .... { .... IntPtr hInitialFont; .... IntPtr hCurrentFont;
Ainsi, une erreur évidente a été commise. Une autre question est dans son importance. Maintenant, à la suite du travail de la méthode
DisposeFont dans la section marquée avec le commentaire "sélectionner la police initiale dans", le champ
hCurrentFont ne sera pas initialisé. Je pense que seuls les auteurs du code peuvent donner un verdict exact.
ConclusionsDonc, cette fois, je vais devoir critiquer un peu la SEP. Dans WinForms, de nombreuses erreurs nécessitent une attention particulière de la part des développeurs. C'est peut-être la faute d'une hâte avec laquelle MS travaille sur .NET Core 3 et ses composants, y compris WinForms. À mon avis, le code WinForms est toujours "brut", mais j'espère que la situation changera pour le mieux bientôt.
La deuxième raison du grand nombre d'erreurs peut être que notre analyseur est simplement devenu meilleur pour les rechercher :).
Soit dit en passant, un article de mon collègue Sergey Vasiliev sera bientôt publié dans lequel il recherche et trouve beaucoup de problèmes dans le code des bibliothèques .NET Core. J'espère que son travail contribuera également à améliorer les caractéristiques de la plateforme .NET, car nous essayons toujours d'informer les développeurs sur les résultats de l'analyse de leurs projets.
Et pour ceux qui souhaitent améliorer leurs produits par eux-mêmes ou rechercher des erreurs dans les projets d'autres personnes, je vous suggère de
télécharger et d'essayer PVS-Studio .
Du code propre à tout le monde!