Dieser Artikel ist eine Übersicht über die Fehler, die im Avalonia-UI-Projekt mit dem statischen Analysegerät PVS-Studio gefunden wurden. Avalonia UI ist ein plattformübergreifendes Open-Source-XAML-basiertes UI-Framework. Dies ist eines der technologisch bedeutendsten Projekte in der Geschichte von .NET, da Entwickler plattformübergreifende Schnittstellen auf der Basis des WPF-Systems erstellen können. Wir hoffen, dass die Autoren des Projekts diesen Artikel hilfreich finden, um einige der Fehler zu beheben und überzeugend genug sind, um statische Analysen in ihren Entwicklungsprozess einzubeziehen.
Über Avalonia UI
Mit Avalonia UI (früher als Perspex bekannt) können Entwickler Benutzeroberflächen erstellen, die unter Windows, Linux und MacOS ausgeführt werden können. Als experimentelles Feature bietet es auch Unterstützung für Android und iOS. Die Avalonia-Benutzeroberfläche ist kein Wrapper für andere Wrapper wie Xamarin Forms, der Xamarin-Wrapper umschließt, sondern greift direkt auf die native API zu. Beim Anschauen eines der Demo-Videos war ich erstaunt, dass Sie ein Steuerelement auf der Debian-Konsole ausgeben können. Dank der Verwendung der XAML-Auszeichnungssprache bietet die Avalonia-Benutzeroberfläche im Vergleich zu anderen Benutzeroberflächenkonstruktoren mehr Design- und Layoutfunktionen.
Avalonia UI wird unter anderem in
AvalonStudio (einer plattformübergreifenden IDE für die C # - und C / C ++ - Softwareentwicklung) und
Core2D (einem 2D-Diagrammeditor) verwendet.
Wasabi Wallet (eine Bitcoin-Brieftasche) ist ein Beispiel für kommerzielle Software, die die Avalonia-Benutzeroberfläche verwendet.
Der Kampf gegen die Notwendigkeit, beim Erstellen einer plattformübergreifenden Anwendung eine Reihe von Bibliotheken zu führen, ist äußerst wichtig. Wir wollten den Autoren der Avalonia-Benutzeroberfläche dabei helfen, also lud ich den Quellcode des Projekts herunter und überprüfte ihn mit unserem Analyser. Ich hoffe, dass sie diesen Artikel sehen und die vorgeschlagenen Korrekturen vornehmen und sogar regelmäßig statische Analysen als Teil ihres Entwicklungsprozesses verwenden. Dies ist dank der kostenlosen Lizenzierungsoption von PVS-Studio für Open-Source-Entwickler problemlos möglich. Die regelmäßige Verwendung statischer Analysen hilft, viele Probleme zu vermeiden und das Erkennen und Beheben von Fehlern viel billiger zu machen.
Analyseergebnisse
PVS-Studio-Diagnosemeldung: V3001 Links und rechts vom Operator '^' befinden sich identische Unterausdrücke 'controlledFlags'. WindowImpl.cs 975TwitterClientMessageHandler.cs 52
private void UpdateWMStyles(Action change) { .... var style = (WindowStyles)GetWindowLong(....); .... style = style | controlledFlags ^ controlledFlags; .... }
Beginnen wir mit unserer ersten C # -Diagnose, um eine gewisse Symbolik hinzuzufügen. Der Analysator hat einen merkwürdigen Ausdruck mit dem bitweisen Operator OR erkannt. Lassen Sie mich dies mit Zahlen erklären:
der Ausdruck
1100 0011 | 1111 0000 ^ 1111 0000
ist äquivalent zu
1100 0011 | 0000 0000
Die Priorität des exklusiven ODER ("^") ist höher als die des bitweisen ODER ("|"). Der Programmierer hatte diese Reihenfolge wahrscheinlich nicht vorgesehen. Der Code kann durch Einschließen des ersten Ausdrucks in Klammern festgelegt werden:
private void UpdateWMStyles(Action change) { .... style = (style | controlledFlags) ^ controlledFlags; .... }
Was die nächsten beiden Warnungen angeht, muss ich zugeben: Dies sind falsch positive Ergebnisse. Sie sehen, die Entwickler verwenden die öffentliche API der
TransformToVisual- Methode. In diesem Fall ist
VisualRoot immer ein übergeordnetes Element für
Visual . Das habe ich bei der Prüfung der Warnung nicht verstanden. Erst nachdem ich den Artikel fertiggestellt hatte, erzählte mir einer der Autoren des Projekts davon. Daher zielen die unten vorgeschlagenen Korrekturen darauf ab, den Code vor möglichen Änderungen zu schützen, die diese Logik verletzen, und nicht auf einen tatsächlichen Absturz.
PVS-Studio-Diagnosemeldung: V3080 Mögliche Null-Dereferenzierung des Methodenrückgabewerts. Betrachten Sie Folgendes: TranslatePoint (...). VisualExtensions.cs 23
public static Point PointToClient(this IVisual visual, PixelPoint point) { var rootPoint = visual.VisualRoot.PointToClient(point); return visual.VisualRoot.TranslatePoint(rootPoint, visual).Value; }
Diese Methode ist klein. Der Analysator ist der Ansicht, dass die Dereferenzierung des durch den Aufruf von
TranslatePoint zurückgegebenen Werts unsicher ist. Werfen wir einen Blick auf diese Methode:
public static Point? TranslatePoint(this IVisual visual, Point point, IVisual relativeTo) { var transform = visual.TransformToVisual(relativeTo); if (transform.HasValue) { return point.Transform(transform.Value); } return null; }
In der Tat könnte es
null zurückgeben .
Diese Methode wird sechsmal aufgerufen: dreimal mit einer Überprüfung des zurückgegebenen Werts und dreimal ohne Überprüfung, wodurch die Warnung vor einer möglichen Dereferenzierung ausgelöst wird. Das erste ist das obige und hier sind die beiden anderen:
- V3080 Mögliche Null-Dereferenzierung. Sehen Sie sich 'p' an. VisualExtensions.cs 35
- V3080 Mögliche Null-Dereferenzierung. Betrachten Sie die Inspektion von 'controlPoint'. Scene.cs 176
Ich schlage vor, diese Fehler nach dem in den sicheren Versionen verwendeten Muster zu beheben, dh durch Hinzufügen einer
Nullable <Struct> .HasValue- Prüfung in der
PointToClient- Methode:
public static Point PointToClient(this IVisual visual, PixelPoint point) { var rootPoint = visual.VisualRoot.PointToClient(point); if (rootPoint.HasValue) return visual.VisualRoot.TranslatePoint(rootPoint, visual).Value; else throw ....; }
PVS-Studio-Diagnosemeldung: V3080 Mögliche Null-Dereferenzierung des Methodenrückgabewerts. Betrachten Sie Folgendes: TransformToVisual (...). ViewportManager.cs 381
Dieser Fehler ist dem vorherigen sehr ähnlich:
private void OnEffectiveViewportChanged(TransformedBounds? bounds) { .... var transform = _owner.GetVisualRoot().TransformToVisual(_owner).Value; .... }
Dies ist der Code der
TransformToVisual- Methode:
public static Matrix? TransformToVisual(this IVisual from, IVisual to) { var common = from.FindCommonVisualAncestor(to); if (common != null) { .... } return null; }
Übrigens kann die
FindCommonVisualAncestor- Methode tatsächlich
null als Standardwert für Referenztypen zurückgeben:
public static IVisual FindCommonVisualAncestor(this IVisual visual, IVisual target) { Contract.Requires<ArgumentNullException>(visual != null); return ....FirstOrDefault(); }
Die
TransformToVisual- Methode wird neunmal mit nur sieben Überprüfungen aufgerufen. Der erste Aufruf mit unsicherer Dereferenzierung ist der obige und der zweite:
V3080 Mögliche Null-Dereferenzierung. Betrachten Sie die Überprüfung von "Transformation". MouseDevice.cs 80
PVS-Studio Diagnosemeldung: V3022 Ausdruck ist immer wahr. Wahrscheinlich sollte hier der Operator '&&' verwendet werden. NavigationDirection.cs 89
public static bool IsDirectional(this NavigationDirection direction) { return direction > NavigationDirection.Previous || direction <= NavigationDirection.PageDown; }
Diese Prüfung ist seltsam. Die
NavigationDirection- Enumeration enthält 9 Typen, wobei der
PageDown- Typ der letzte ist. Vielleicht war es nicht immer so, oder vielleicht ist dies ein Schutz gegen das Hinzufügen neuer Richtungsoptionen durch SUDDEN. Meiner Meinung nach sollte die erste Überprüfung ausreichen. Wie auch immer, überlassen wir dies den Autoren, um zu entscheiden.
PVS-Studio-Diagnosemeldung: V3066 Mögliche falsche Reihenfolge der an den Konstruktor 'SelectionChangedEventArgs' übergebenen Argumente: 'removedSelectedItems' und 'addedSelectedItems'. DataGridSelectedItemsCollection.cs 338
internal SelectionChangedEventArgs GetSelectionChangedEventArgs() { .... return new SelectionChangedEventArgs (DataGrid.SelectionChangedEvent, removedSelectedItems, addedSelectedItems) { Source = OwningGrid }; }
Der Analysator warnt vor der falschen Reihenfolge der zweiten und dritten Argumente des Konstruktors. Werfen wir einen Blick auf diesen Konstruktor:
public SelectionChangedEventArgs(RoutedEvent routedEvent, IList addedItems, IList removedItems) : base(routedEvent) { AddedItems = addedItems; RemovedItems = removedItems; }
Es werden zwei Container vom Typ
IList als Argumente verwendet, wodurch es sehr einfach ist, sie in der falschen Reihenfolge zu schreiben. Ein Kommentar am Anfang der Klasse deutet darauf hin, dass dies ein Fehler im Code des Steuerelements ist, das von Microsoft ausgeliehen und für die Verwendung in Avalonia geändert wurde. Aber ich würde immer noch darauf bestehen, die Argumentationsreihenfolge zu korrigieren, um zu vermeiden, dass ein Fehlerbericht darüber erstellt wird und Zeit damit verschwendet wird, nach einem Fehler in Ihrem eigenen Code zu suchen.
Es gab drei weitere Fehler dieses Typs:
PVS-Studio-Diagnosemeldung: V3066 Mögliche falsche Reihenfolge der an den Konstruktor 'SelectionChangedEventArgs' übergebenen Argumente: 'entfernt' und 'hinzugefügt'. AutoCompleteBox.cs 707
OnSelectionChanged(new SelectionChangedEventArgs(SelectionChangedEvent, removed, added));
Es ist derselbe Konstruktor
SelectionChangedEventArgs.PVS-Studio Diagnosemeldungen V3066 :
- Mögliche falsche Reihenfolge der an den Konstruktor 'ItemsRepeaterElementIndexChangedEventArgs' übergebenen Argumente: 'oldIndex' und 'newIndex'. ItemsRepeater.cs 532
- Mögliche falsche Reihenfolge der an die 'Update'-Methode übergebenen Argumente:' oldIndex 'und' newIndex '. ItemsRepeater.cs 536
Zwei Warnungen bei einer Ereignisaufrufmethode.
internal void OnElementIndexChanged(IControl element, int oldIndex, int newIndex) { if (ElementIndexChanged != null) { if (_elementIndexChangedArgs == null) { _elementIndexChangedArgs = new ItemsRepeaterElementIndexChangedEventArgs(element, oldIndex, newIndex); } else { _elementIndexChangedArgs.Update(element, oldIndex, newIndex); } ..... } }
Der Analysator hat festgestellt, dass die Argumente
oldIndex und
newIndex in beiden Methoden
ItemsRepeaterElementIndexChangedEventArgs und
Update in einer anderen Reihenfolge geschrieben sind:
internal ItemsRepeaterElementIndexChangedEventArgs( IControl element, int newIndex, int oldIndex) { Element = element; NewIndex = newIndex; OldIndex = oldIndex; } internal void Update(IControl element, int newIndex, int oldIndex) { Element = element; NewIndex = newIndex; OldIndex = oldIndex; }
Vielleicht wurde dieser Code von verschiedenen Programmierern geschrieben, von denen sich einer mehr für die Vergangenheit und der andere für die Zukunft interessierte :)
Genau wie in der vorigen Ausgabe ist hier keine sofortige Korrektur erforderlich. es muss noch festgestellt werden, ob dieser Code tatsächlich fehlerhaft ist.
PVS-Studio Diagnosemeldung: V3004 Die Anweisung 'then' entspricht der Anweisung 'else'. DataGridSortDescription.cs 235
public override IOrderedEnumerable<object> ThenBy(IOrderedEnumerable<object> seq) { if (_descending) { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } else { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } }
Dies ist eine ziemlich merkwürdige Implementierung der
ThenBy- Methode. Die
IEnumerable- Schnittstelle, von der das
seq- Argument geerbt wird, enthält die
ThenBy- Methode, die anscheinend folgendermaßen verwendet werden sollte:
public override IOrderedEnumerable<object> ThenBy(IOrderedEnumerable<object> seq) { if (_descending) { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } else { return seq.ThenBy(o => GetValue(o), InternalComparer); } }
PVS-Studio Diagnosemeldung: V3106 Möglicher negativer Indexwert. Der Wert von 'index' index könnte -1 erreichen. Animator.cs 68
protected T InterpolationHandler(double animationTime, T neutralValue) { .... if (kvCount > 2) { if (animationTime <= 0.0) { .... } else if (animationTime >= 1.0) { .... } else { int index = FindClosestBeforeKeyFrame(animationTime); firstKeyframe = _convertedKeyframes[index]; } .... } .... }
Der Analysator ist sich sicher, dass die
Indexvariable den Wert -1 haben kann. Dieser Variablen wird der von der
FindClosestBeforeKeyFrame- Methode zurückgegebene Wert zugewiesen.
private int FindClosestBeforeKeyFrame(double time) { for (int i = 0; i < _convertedKeyframes.Count; i++) if (_convertedKeyframes[i].Cue.CueValue > time) return i - 1; throw new Exception("Index time is out of keyframe time range."); }
Wie Sie sehen, enthält die Schleife eine Bedingung, auf die eine return-Anweisung folgt, die den vorherigen Wert des Iterators zurückgibt. Es ist schwierig zu überprüfen, ob diese Bedingung erfüllt ist, und ich kann nicht sicher sagen, welchen Wert
CueValue haben wird, aber die Beschreibung legt nahe, dass es einen Wert von 0,0 bis 1,0 annimmt. Wir können jedoch noch ein paar Worte zur
Zeit sagen: Es handelt sich um die an die aufrufende Methode übergebene
animationTime- Variable, die definitiv größer als null und kleiner als eins ist. Andernfalls würde die Ausführung einem anderen Zweig folgen. Wenn diese Methoden für Animationen verwendet werden, ähnelt diese Situation einem anständigen Heisenbug. Ich würde empfehlen, den von
FindClosestBeforeKeyFrame zurückgegebenen Wert zu
überprüfen, wenn in diesem Fall eine spezielle Behandlung erforderlich ist, oder das erste Element aus der Schleife zu entfernen, wenn andere Bedingungen nicht erfüllt sind. Ich weiß nicht, wie genau das alles funktionieren soll, also würde ich die zweite Lösung als Beispiel nehmen:
private int FindClosestBeforeKeyFrame(double time) { for (int i = 1; i < _convertedKeyframes.Count; i++) if (_convertedKeyframes[i].Cue.CueValue > time) return i - 1; throw new Exception("Index time is out of keyframe time range."); }
PVS-Studio Diagnosemeldung: V3117 Konstruktorparameter 'phones' wird nicht verwendet. Land.cs 25
public Country(string name, string region, int population, int area, double density, double coast, double? migration, double? infantMorality, int gdp, double? literacy, double? phones, double? birth, double? death) { Name = name; Region = region; Population = population; Area = area; PopulationDensity = density; CoastLine = coast; NetMigration = migration; InfantMortality = infantMorality; GDP = gdp; LiteracyPercent = literacy; BirthRate = birth; DeathRate = death; }
Dies ist ein gutes Beispiel dafür, wie statische Analysen besser sind als Codeüberprüfungen. Der Konstruktor wird mit dreizehn Argumenten aufgerufen, von denen eines nicht verwendet wird. Tatsächlich konnte Visual Studio es auch erkennen, jedoch nur mithilfe von Diagnosen der dritten Ebene (die häufig deaktiviert sind). Wir haben es hier definitiv mit einem Fehler zu tun, da die Klasse auch dreizehn Eigenschaften enthält - eine pro Argument -, aber es gibt keine Zuweisung zur Variablen
Phones . Da das Problem offensichtlich ist, werde ich es nicht näher erläutern.
PVS-Studio Diagnosemeldung: V3080 Mögliche Null-Dereferenzierung. Sehen Sie sich 'tabItem' an. TabItemContainerGenerator.cs 22
protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); tabItem.ParentTabControl = Owner; .... }
Der Analyzer betrachtet die Dereferenzierung des von der
CreateContainer- Methode zurückgegebenen
Werts als unsicher. Werfen wir einen Blick auf diese Methode:
protected override IControl CreateContainer(object item) { var container = item as T; if (item == null) { return null; } else if (container != null) { return container; } else { .... return result; } }
PVS-Studio kann eine Zuweisung von
Null selbst durch eine Kette von fünfzig Methoden verfolgen, kann jedoch nicht sagen, ob die Ausführung jemals diesem Zweig folgen würde. Ich könnte es auch nicht ... Die Aufrufe gehen zwischen überschriebenen und virtuellen Methoden verloren, daher würde ich einfach vorschlagen, einen zusätzlichen Scheck für den Fall zu schreiben:
protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); if(tabItem == null) return; tabItem.ParentTabControl = Owner; .... }
PVS-Studio Diagnosemeldung: V3142 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. DevTools.xaml.cs 91
Es macht keinen Sinn, zu viel Code zu zitieren, um die Spannung aufrechtzuerhalten. Ich sage es Ihnen gleich: Diese Warnung ist falsch positiv. Der Analysator hat einen Aufruf der Methode festgestellt, der eine bedingungslose Ausnahme auslöst:
public static void Load(object obj) { throw new XamlLoadException($"No precompiled XAML found for {obj.GetType()}, make sure to specify x:Class and include your XAML file as AvaloniaResource"); }
Fünfunddreißig (!) Warnungen über nicht erreichbaren Code nach den Aufrufen dieser Methode waren zu viel, um ignoriert zu werden, und ich fragte einen der Entwickler, was hier vor sich ging. Er erzählte mir, dass sie eine Technik verwendeten, bei der Sie Aufrufe an eine Methode durch Aufrufe an andere Methoden ersetzen, indem Sie die
Mono.Cecil- Bibliothek verwenden. Mit dieser Bibliothek können Sie Anrufe direkt im IL-Code ersetzen.
Unser Analyser unterstützt diese Bibliothek nicht, daher die große Menge an falschen Positiven. Dies bedeutet, dass diese Diagnose deaktiviert werden sollte, wenn die Avalonia-Benutzeroberfläche überprüft wird. Es fühlt sich etwas umständlich an, aber ich muss zugeben, dass ich es bin, der diese Diagnose erstellt hat ... Aber wie jedes andere Tool muss auch ein statischer Analysator etwas genauer eingestellt werden.
Zum Beispiel arbeiten wir derzeit an einer Diagnose, die unsichere Typkonvertierungen erkennt. Bei einem Spielprojekt, bei dem die Typprüfung auf der Seite der Engine durchgeführt wird, werden ungefähr tausend Fehlalarme ausgegeben.
PVS-Studio-Diagnosemeldung: V3009 Es ist merkwürdig, dass diese Methode immer den gleichen Wert von 'true' zurückgibt. DataGridRows.cs 412
internal bool ScrollSlotIntoView(int slot, bool scrolledHorizontally) { if (....) { .... if (DisplayData.FirstScrollingSlot < slot && DisplayData.LastScrollingSlot > slot) { return true; } else if (DisplayData.FirstScrollingSlot == slot && slot != -1) { .... return true; } .... } .... return true; }
Die Methode gibt die ganze Zeit
true zurück . Vielleicht hat sich sein Zweck geändert, seit es zum ersten Mal geschrieben wurde, aber es sieht eher nach einem Bug aus. Nach dem Kommentar zu Beginn der Klasse zu urteilen, handelt es sich um eine weitere Kontrollklasse, die von Microsoft ausgeliehen wurde. Wenn Sie mich fragen, ist
DataGrid eines der am wenigsten stabilen Steuerelemente.
Daher ist es möglicherweise keine gute Idee, den
Bildlauf zu bestätigen, wenn die Bedingungen nicht erfüllt sind.
Fazit
Einige der oben beschriebenen Fehler wurden zusammen mit dem aus den WPF-Steuerelementen kopierten Code ausgeliehen, und die Autoren der Avalonia-Benutzeroberfläche haben nichts damit zu tun. Für den Benutzer macht es jedoch keinen Unterschied: Eine nicht funktionierende oder fehlerhafte Benutzeroberfläche hinterlässt einen schlechten Eindruck von der Gesamtqualität des Programms.
Ich erwähnte die Notwendigkeit der Feinabstimmung des Analysegeräts: False Positives sind aufgrund der Arbeitsweise der statischen Analysealgorithmen unvermeidlich. Diejenigen, die mit dem
Problem des
Anhaltens vertraut sind
, wissen, dass es mathematische Einschränkungen bei der Verarbeitung eines Codeteils mit einem anderen gibt. In diesem Fall geht es jedoch darum, eine Diagnose von fast einhunderteinhalb zu deaktivieren. Bei statischen Analysen besteht also kein Problem des Bedeutungsverlustes. Außerdem könnte diese Diagnose auch Warnungen auslösen, die auf echte Bugs hinweisen, aber diese sind unter Tonnen von False Positives kaum zu bemerken.
Ich muss die bemerkenswerte Qualität des Avalonia UI-Projekts erwähnen! Ich hoffe die Entwickler werden es so halten. Leider wächst die Anzahl der Fehler mit der Größe des Programms. Eine kluge Feinabstimmung der CI \ CD-Systeme, unterstützt durch statische und dynamische Analysen, ist eine der Möglichkeiten, um Fehler in Schach zu halten. Und wenn Sie die Entwicklung großer Projekte vereinfachen und weniger Zeit für das Debuggen aufwenden möchten,
laden Sie PVS-Studio
herunter und probieren Sie es aus !