Dieser Artikel ist das Ergebnis der Überprüfung des Avalonia-UI-Projekts mit dem statischen Analysegerät PVS-Studio. Die Avalonia-Benutzeroberfläche ist eine plattformübergreifende XAML-basierte Open-Source-Benutzeroberflächenplattform. Dies ist eines der technologisch bedeutenden Projekte in der Geschichte von .NET, da Sie plattformübergreifende Schnittstellen auf der Basis des WPF-Systems erstellen können. Ich hoffe, dass dieser Artikel den Autoren hilft, einige Fehler zu korrigieren und sie davon zu überzeugen, in Zukunft statische Analysegeräte zu verwenden.
Über Avalonia UI
Das Avalonia-UI-Projekt (früher Perspex genannt) bietet die Möglichkeit, Benutzeroberflächen für Windows, Linux und MacOS zu erstellen. Momentan gibt es auch experimentelle Unterstützung für Android und iOS. Die Avalonia-Benutzeroberfläche ist kein Wrapper über Wrappern, sondern bezieht sich auf die native API. Im Gegensatz zu Xamarin Forms, das Xamarin-Wrapper umhüllt. In einem der Demo-Videos war ich beeindruckt von der Möglichkeit, die Steuerung der Debian-Konsole zu übernehmen. Darüber hinaus bietet das Projekt dank der Verwendung von XAML-Markup mehr Funktionen für Layout und Design als herkömmliche Schnittstellendesigner.
Zu den Projekten, die Avalonia UI bereits verwenden, gehören
AvalonStudio (eine plattformübergreifende IDE für die Entwicklung in C # und C / C ++) und
Core2D (Editor für 2D-Diagramme und Diagramme). Als kommerzielles Projekt können Sie
Wasabi Wallet (Bitcoin Wallet) mitbringen.
Der Kampf gegen die Notwendigkeit, mehrere verschiedene Bibliotheken für die Erstellung einer plattformübergreifenden Anwendung zu haben, ist von großer Bedeutung. Wir wollten dem Projekt helfen, und ich lud das Projekt herunter und überprüfte es mit unserem Analysegerät. Ich hoffe, dass die Autoren diesem Artikel Aufmerksamkeit schenken und die notwendigen Änderungen am Code vornehmen oder möglicherweise regelmäßige statische Analysen in den Entwicklungsprozess einführen. Dazu können sie die kostenlose Lizenzierungsoption von PVS-Studio für Open Source-Projekte nutzen. Die regelmäßige Verwendung eines statischen Analysegeräts hilft, viele Probleme zu vermeiden und die Kosten für die Erkennung und Behebung vieler Fehler zu senken.
Validierungsergebnisse
PVS-Studio Warnung: 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; .... }
Ich werde symbolisch mit unserer ersten C # -Diagnose beginnen. Der Analysator hat eine merkwürdige Verwendung des bitweisen ODER-Operators festgestellt. Lassen Sie mich anhand der Zahlen erklären, was hier passiert:
Ausdruck
1100 0011 | 1111 0000 ^ 1111 0000
ähnlich wie folgt:
1100 0011 | 0000 0000
Ein exklusives ODER ("^") hat eine höhere Priorität als ein bitweises ODER ("|"). Höchstwahrscheinlich wurde hier eine andere Reihenfolge der Operationen impliziert. In diesem Fall sollten Sie den ersten Ausdruck in Klammern setzen:
private void UpdateWMStyles(Action change) { .... style = (style | controlledFlags) ^ controlledFlags; .... }
Vor den nächsten beiden Warnungen muss ich zugeben: die Fehlalarme. Dies liegt an der Verwendung der öffentlichen API, der
TransformToVisual- Methode. In unserem Fall ist
VisualRoot immer das übergeordnete
Element von
Visual . Ich habe dies bei der Analyse der Antwort nicht verstanden, teilte mir der Autor des Projekts nach dem Schreiben des Artikels mit. Die im Artikel vorgeschlagenen Änderungen dienen also nicht dem Schutz vor einem tatsächlichen Sturz, sondern vor einer möglichen Überarbeitung, die diese Logik verletzt.
PVS-Studio Warnung: 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; }
Eine kleine Methode. Der Analyzer ist der Ansicht, dass die Dereferenzierung des Ergebnisses eines Aufrufs von TranslatePoint unsicher ist. Schauen Sie sich diese Methode an:
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 gibt es eine Rückkehr
null .
Diese Methode hat 6 Aufrufe. In drei Fällen wird der Wert überprüft und im Übrigen erkennt PVS-Studio mögliche Dereferenzierungen und gibt Warnungen aus. Ich habe das erste oben zitiert und die anderen beiden Warnungen sind hier:
- 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, es analog zu
beheben , indem ich die
Nullable <Struct> .HasValue- Prüfung in der
PointToClient- Methode
hinzufüge : 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 Warnung: V3080 Mögliche Null-Dereferenzierung des Methodenrückgabewerts. Betrachten Sie Folgendes: TransformToVisual (...). ViewportManager.cs 381
Ein Fall, der dem vorherigen Beispiel sehr ähnlich ist:
private void OnEffectiveViewportChanged(TransformedBounds? bounds) { .... var transform = _owner.GetVisualRoot().TransformToVisual(_owner).Value; .... }
Die
TransformToVisual- Methode sieht folgendermaßen aus:
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 an neun Stellen verwendet, an sieben Stellen werden Prüfungen durchgeführt. Die erste Warnung, die ohne Überprüfung verwendet werden muss, ist höher und die letzte ist hier:
V3080 Mögliche Null-Dereferenzierung. Betrachten Sie die Überprüfung von "Transformation". MouseDevice.cs 80
PVS-Studio Warnung: 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; }
Seltsame Prüfung. In der
NavigationDirection- Enumeration gibt es 9 Typen, und
PageDown ist der letzte von ihnen. Vielleicht war dies nicht immer der Fall, oder es handelt sich um eine Versicherung gegen das plötzliche Auftreten neuer Überweisungsoptionen. Es scheint mir, dass der erste Scheck hier ausreicht. Die Entscheidung überlasse ich den Autoren des Projekts.
Warnung PVS-Studio: 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 }; }
In diesem Fall schlug der Analysator vor, das zweite und dritte Argument des Konstruktors zu verwechseln. Schauen wir uns den aufgerufenen Konstruktor an:
public SelectionChangedEventArgs(RoutedEvent routedEvent, IList addedItems, IList removedItems) : base(routedEvent) { AddedItems = addedItems; RemovedItems = removedItems; }
Es werden zwei Behälter vom Typ
IList akzeptiert, die leicht zu mischen sind. Nach dem Kommentar zu Beginn der Klasse zu urteilen, handelt es sich um einen Fehler im Kontrollcode, der von Microsoft kopiert und unter Avalonia geändert wurde. Aber es scheint mir, dass es sich lohnt, die Reihenfolge der Argumente der Methode zu korrigieren, zumindest nicht nach einem möglichen Fehler in sich selbst zu suchen, wenn ein Fehlerbericht kommt.
Der Analysator hat drei weitere ähnliche Fehler gefunden:
Warnung PVS-Studio: V3066 Mögliche falsche Reihenfolge der Argumente, die an den Konstruktor 'SelectionChangedEventArgs' übergeben wurden: 'removed' und 'added'. AutoCompleteBox.cs 707
OnSelectionChanged(new SelectionChangedEventArgs(SelectionChangedEvent, removed, added));
Selber Konstruktor
SelectionChangedEventArgs.PVS-Studio V3066 Warnungen :
- 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 Operationen in einer Methode eines Ereignisaufrufs.
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
sowohl in ItemsRepeaterElementIndexChangedEventArgs als auch in der
Update- Methode die Argumente
oldIndex und
newIndex eine unterschiedliche Reihenfolge haben:
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 der Code von verschiedenen Programmierern geschrieben, und für einen ist es wichtiger, was passiert ist, und für den anderen - was wird passieren :)
Wie im vorherigen Fall sollten Sie es nicht sofort bearbeiten, sondern prüfen, ob wirklich ein Fehler vorliegt.
PVS-Studio Warnung: 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); } }
Eine äußerst interessante Implementierung der
ThenBy- Methode. Die
IEnumerable- Schnittstelle, von der das
seq- Argument geerbt wird
, verfügt über eine
ThenBy- Methode. Ich nehme an, dass seine Verwendung impliziert wurde. So:
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); } }
Warnung PVS-Studio: 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 geht davon aus, dass der
Index einen Wert von -1 haben kann. Die Variable wird von der
FindClosestBeforeKeyFrame- Methode abgerufen.
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 wir sehen können, wird die Bedingung in der Schleife überprüft und der vorherige Wert des Iterators wird zurückgegeben. Der Zustand ist ziemlich schwer zu überprüfen und ich kann nicht genau sagen, was
CueValue ist , aber der Beschreibung nach nimmt er einen Wert von 0,0 bis 1,0 an. Wir können etwas über die
Zeit sagen, das ist
animationTime von der aufrufenden Methode, es ist definitiv mehr als null und weniger als eins. Andernfalls würde die Programmausführung in einem anderen Zweig ablaufen. Wenn dies die Methoden sind, die zum Rendern der Animation aufgerufen werden, sieht die Situation wie ein guter schwebender Fehler aus. Ich würde eine Überprüfung für das Ergebnis von
FindClosestBeforeKeyFrame hinzufügen, wenn in diesem Fall eine spezielle Behandlung erforderlich ist. Oder - wenn das erste Element einige andere Bedingungen nicht erfüllt - entfernen Sie es aus der Schleife. Da ich nicht weiß, wie das alles funktionieren soll, werde ich die zweite Option als Beispiel für die Korrektur wählen:
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 Warnung: Der 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; }
Ein gutes Beispiel für den Unterschied zwischen dem Betrieb des Analysegeräts und der manuellen Codeüberprüfung. Dreizehn Konstruktorargumente, eines davon nicht verwendet. Tatsächlich stellt Visual Studio auch ein nicht verwendetes Argument fest, jedoch auf der dritten Warnstufe (sie sind häufig deaktiviert). In diesem Fall ist dies ein klarer Fehler, da die Klasse auch dreizehn Eigenschaften für jedes Argument hat und in
Phones nirgendwo ein Wert zugewiesen ist. Bearbeitung ist selbstverständlich, ich werde es nicht bringen.
PVS-Studio Warnung: 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 hielt es für gefährlich, das Ergebnis des Aufrufs von
CreateContainer zu dereferenzieren.
Schauen Sie sich diese Methode an:
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; } }
Der Analysator kann die Zuweisung von
Null zu einer Variablen sehen, selbst wenn ein Wert durch eine Kette von fünfzig Methoden geleitet wird. Er kann jedoch nicht sagen, ob dieser Thread mindestens einmal ausgeführt wird. Ja, und ich konnte es auch im Allgemeinen nicht ... Methodenaufrufe gehen zwischen überschriebenen und virtuellen Methoden verloren. Ich schlage daher vor, mit einer zusätzlichen Überprüfung nur auf Nummer sicher zu gehen:
protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); if(tabItem == null) return null; tabItem.ParentTabControl = Owner; .... }
PVS-Studio Warnung: V3142 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. DevTools.xaml.cs 91
Ich werde hier nicht viel Code schreiben, um Intrigen zu erzeugen, ich werde sofort sagen: Die Warnung ist falsch. Der Analysator hat einen Aufruf einer Methode gesehen, die eine bedingungslose Ausnahme auslöst. Da ist er:
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"); }
Es war unmöglich, fünfunddreißig Warnungen (!) Über den nicht erreichbaren Code, der sich nach Aufrufen dieser Methode befindet, nicht zu beachten. Ich habe einen der Entwickler des Projekts gefragt: Wie funktioniert es? Und sie erzählten mir von einer Möglichkeit, Aufrufe einer Methode durch Aufrufe anderer zu ersetzen, die die
Mono.Cecil- Bibliothek verwenden. Sie können Anrufe direkt im IL-Code ersetzen.
Der Analyzer unterstützt diese Bibliothek nicht, daher gibt es viele Fehlalarme. Es ist daher besser, diese Diagnose für dieses Projekt zu deaktivieren. Es ist etwas peinlich zuzugeben, dass ich diese Diagnose gestellt habe ... aber wie jedes Tool muss auch die statische Analyse konfiguriert werden.
Beispielsweise entwickeln wir derzeit Diagnosen zur Konvertierung unsicherer Typen. Und es gibt etwas weniger als tausend Operationen auf dem Spielprojekt, bei denen die Tippsteuerung auf der Motorseite ausgeführt wird.
PVS-Studio Warnung: 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 immer
true zurück . Vielleicht hat sich der Zweck der Methode geändert, seit die Signatur geschrieben wurde, aber höchstwahrscheinlich handelt es sich um einen Fehler. Dies ist auch die von Microsoft kopierte Kontrollklasse, gemessen am Kommentar am Beginn der Klasse. Meiner Meinung nach ist
DataGrid in der Regel eines der instabilsten Steuerelemente. Ich halte es für erwägenswert. Ist es erforderlich, den Bildlauf zu bestätigen, wenn die Bedingungen nicht erfüllt werden?
Fazit
Einige dieser Fehler wurden nicht von den Avalonia-UI-Entwicklern selbst in das Projekt eingefügt, sondern von Code, der aus den WPF-Steuerelementen kopiert wurde. Für den Benutzer der Schnittstelle spielt die Fehlerquelle jedoch in der Regel keine Rolle. Ein abgestürztes oder fehlerhaftes Interface verdirbt die Meinung des gesamten Programms.
In dem Artikel, den ich erwähnte, dass der Analysator konfiguriert werden muss, gibt es Fehlalgorithmen, die aufgrund der Funktionsweise statischer Analysealgorithmen unvermeidbar sind. Jeder, der mit dem
Problem des Anhaltens vertraut
ist, kennt die mathematischen Einschränkungen beim Arbeiten mit anderem Code. In diesem Fall geht es jedoch darum, eine Diagnose von fast einhunderteinhalb zu deaktivieren. Wir sprechen also nicht über den Bedeutungsverlust in der statischen Analyse (oder die Frage ist es nicht wert). Außerdem hätte diese Diagnose gut ansprechen können. Es ist nur schwieriger, sie unter der Masse der falsch-positiven Ergebnisse zu finden.
Achten Sie auf die hohe Qualität des Projektcodes! Ich hoffe, die Entwickler werden das Tempo und die Qualität des Codes beibehalten. Je größer das Projekt, desto mehr Fehler sind leider enthalten. Eine der Möglichkeiten zur Reduzierung von Fehlern besteht in der korrekten Konfiguration von CI \ CD durch die Verknüpfung statischer und dynamischer Analysen. Und wenn Sie die Arbeit mit großen Projekten vereinfachen und den Zeitaufwand für das Debuggen verringern möchten,
laden Sie PVS-Studio
herunter und probieren Sie es aus !
Wenn Sie diesen Artikel mit einem englischsprachigen Publikum teilen möchten, verwenden Sie bitte den Link zur Übersetzung: Alexander Senichkin.
Unser kleiner Beitrag zum Kampf der Avalonia-Benutzeroberfläche für weniger Plattformen .