Artikel ini adalah ulasan bug yang ditemukan di proyek Avalonia UI dengan analisa statis PVS-Studio. Avalonia UI adalah framework UI open-source cross-platform berbasis XAML. Ini adalah salah satu proyek yang paling signifikan secara teknologi dalam sejarah. NET karena memungkinkan pengembang untuk membuat antarmuka lintas platform berdasarkan pada sistem WPF. Kami berharap penulis proyek akan menemukan artikel ini membantu dalam memperbaiki beberapa bug, dan cukup meyakinkan untuk menjadikan analisis statis bagian dari proses pengembangan mereka.
Tentang Avalonia UI
Avalonia UI (sebelumnya dikenal sebagai Perspex) memungkinkan pengembang untuk membuat antarmuka pengguna yang dapat berjalan di Windows, Linux, dan MacOS. Sebagai fitur eksperimental, ia juga menyediakan dukungan Android dan iOS. Avalonia UI bukan pembungkus di sekitar pembungkus lain, seperti Xamarin Forms, yang membungkus pembungkus Xamarin, tetapi langsung mengakses API asli. Saat menonton salah satu video demo, saya heran mengetahui bahwa Anda dapat menampilkan kontrol ke konsol Debian. Selain itu, berkat penggunaan bahasa markup XAML, Avalonia UI menyediakan lebih banyak kemampuan desain dan tata letak dibandingkan dengan konstruktor UI lainnya.
Untuk beberapa contoh, Avalonia UI digunakan di
AvalonStudio (IDE lintas-platform untuk pengembangan perangkat lunak C # dan C / C ++) dan
Core2D (editor diagram 2D).
Wasabi Wallet (a bitcoin wallet) adalah contoh perangkat lunak komersial yang memanfaatkan Avalonia UI.
Pertarungan melawan perlunya menjaga banyak perpustakaan saat membuat aplikasi lintas platform sangat penting. Kami ingin membantu penulis Avalonia UI dengan itu, jadi saya mengunduh kode sumber proyek dan memeriksanya dengan penganalisa kami. Saya berharap mereka akan melihat artikel ini dan melakukan perbaikan yang disarankan dan bahkan mulai menggunakan analisis statis secara teratur sebagai bagian dari proses pengembangan mereka. Ini dapat dengan mudah dilakukan berkat opsi lisensi gratis PVS-Studio yang tersedia untuk pengembang open-source. Menggunakan analisis statis secara teratur membantu menghindari banyak masalah dan membuat deteksi dan perbaikan bug jauh lebih murah.
Hasil analisis
Pesan diagnostik PVS-Studio: V3001 Ada sub-ekspresi yang identik '
tag kendali' di sebelah kiri dan di sebelah kanan operator '^'. WindowImpl.cs 975TwitterClientMessageHandler.cs 52
private void UpdateWMStyles(Action change) { .... var style = (WindowStyles)GetWindowLong(....); .... style = style | controlledFlags ^ controlledFlags; .... }
Untuk menambahkan beberapa simbolisme, mari kita mulai dengan diagnostik C # pertama kami. Penganalisa telah mendeteksi ekspresi aneh dengan operator bitwise OR. Biarkan saya jelaskan ini menggunakan angka:
ekspresi
1100 0011 | 1111 0000 ^ 1111 0000
setara dengan
1100 0011 | 0000 0000
Diutamakan dari OR eksklusif ("^") lebih tinggi daripada bitwise OR ("|"). Si programmer mungkin tidak bermaksud memesan ini. Kode dapat diperbaiki dengan melampirkan ekspresi pertama dalam tanda kurung:
private void UpdateWMStyles(Action change) { .... style = (style | controlledFlags) ^ controlledFlags; .... }
Adapun dua peringatan berikutnya, saya harus akui: ini adalah positif palsu. Anda lihat, para pengembang menggunakan API publik dari metode
TransformToVisual . Dalam hal ini,
VisualRoot selalu menjadi elemen induk untuk
visual . Saya tidak mengerti ketika memeriksa peringatan itu; hanya setelah saya menyelesaikan artikel itulah salah satu penulis proyek memberi tahu saya tentang itu. Oleh karena itu, perbaikan yang disarankan di bawah ini sebenarnya bertujuan melindungi kode terhadap kemungkinan modifikasi yang melanggar logika ini daripada kerusakan yang sebenarnya.
Pesan diagnostik PVS-Studio: V3080 Kemungkinan null dereference nilai pengembalian metode. Pertimbangkan untuk memeriksa: 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; }
Metode ini kecil. Penganalisa percaya bahwa dereferensi nilai yang dikembalikan oleh panggilan
TranslatePoint tidak aman. Mari kita lihat metode ini:
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; }
Memang, itu bisa mengembalikan
nol .
Metode ini disebut enam kali: tiga kali dengan cek nilai yang dikembalikan, dan tiga lainnya tanpa cek, sehingga memicu peringatan tentang potensi dereference. Yang pertama adalah yang di atas, dan di sini ada dua yang lain:
- V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'p'. VisualExtensions.cs 35
- V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'controlPoint'. Scene.cs 176
Saya menyarankan untuk memperbaiki bug ini mengikuti pola yang digunakan dalam versi yang aman, yaitu dengan menambahkan
Nullable <Struct> .HasValue memeriksa di dalam metode
PointToClient :
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 ....; }
Pesan diagnostik PVS-Studio: V3080 Kemungkinan null dereference nilai pengembalian metode. Pertimbangkan untuk memeriksa: TransformToVisual (...). ViewportManager.cs 381
Bug ini sangat mirip dengan yang sebelumnya:
private void OnEffectiveViewportChanged(TransformedBounds? bounds) { .... var transform = _owner.GetVisualRoot().TransformToVisual(_owner).Value; .... }
Ini adalah kode dari metode
TransformToVisual :
public static Matrix? TransformToVisual(this IVisual from, IVisual to) { var common = from.FindCommonVisualAncestor(to); if (common != null) { .... } return null; }
Omong-omong, metode
FindCommonVisualAncestor memang dapat mengembalikan
nol sebagai nilai default untuk jenis referensi:
public static IVisual FindCommonVisualAncestor(this IVisual visual, IVisual target) { Contract.Requires<ArgumentNullException>(visual != null); return ....FirstOrDefault(); }
Metode
TransformToVisual disebut sembilan kali, dengan hanya tujuh pemeriksaan. Panggilan pertama dengan dereferensi tidak aman adalah yang di atas, dan ini yang kedua:
V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'transformasi'. MouseDevice.cs 80
Pesan diagnostik PVS-Studio: Ekspresi
V3022 selalu benar. Mungkin operator '&&' harus digunakan di sini. NavigationDirection.cs 89
public static bool IsDirectional(this NavigationDirection direction) { return direction > NavigationDirection.Previous || direction <= NavigationDirection.PageDown; }
Pemeriksaan ini aneh. Enumerasi
NavigationDirection berisi 9 jenis, dengan jenis
PageDown menjadi yang terakhir. Mungkin tidak selalu seperti itu, atau mungkin ini adalah perlindungan terhadap penambahan SUDDEN dari opsi arah baru. Menurut pendapat saya, cek pertama harus cukup. Bagaimanapun, mari serahkan ini pada penulis untuk memutuskan.
Pesan diagnostik PVS-Studio: V3066 Kemungkinan urutan argumen yang salah diteruskan ke konstruktor 'SelectionChangedEventArgs': 'dihapusSelectedItems' dan 'ditambahkanSelectedItems'. DataGridSelectedItemsCollection.cs 338
internal SelectionChangedEventArgs GetSelectionChangedEventArgs() { .... return new SelectionChangedEventArgs (DataGrid.SelectionChangedEvent, removedSelectedItems, addedSelectedItems) { Source = OwningGrid }; }
Penganalisa memperingatkan tentang urutan yang salah dari argumen kedua dan ketiga dari konstruktor. Mari kita lihat konstruktor itu:
public SelectionChangedEventArgs(RoutedEvent routedEvent, IList addedItems, IList removedItems) : base(routedEvent) { AddedItems = addedItems; RemovedItems = removedItems; }
Dibutuhkan dua wadah tipe
IList sebagai argumen, yang membuatnya sangat mudah untuk menuliskannya dalam urutan yang salah. Sebuah komentar di awal kelas menunjukkan bahwa ini adalah kesalahan dalam kode kontrol yang dipinjam dari Microsoft dan dimodifikasi untuk digunakan di Avalonia. Tapi saya masih bersikeras untuk memperbaiki urutan argumen jika hanya untuk menghindari mendapatkan laporan bug dan membuang waktu mencari bug dalam kode Anda sendiri.
Ada tiga kesalahan jenis ini:
Pesan diagnostik PVS-Studio: V3066 Kemungkinan urutan argumen yang salah diteruskan ke 'konstruktor SelectionChangedEventArgs': 'dihapus' dan 'ditambahkan'. AutoCompleteBox.cs 707
OnSelectionChanged(new SelectionChangedEventArgs(SelectionChangedEvent, removed, added));
Itu adalah konstruktor
SelectionChangedEventArgs yang sama
.Pesan diagnostik PVS-Studio V3066 :
- Kemungkinan urutan argumen yang salah diteruskan ke konstruktor 'ItemsRepeaterElementIndexChangedEventArgs': 'oldIndex' dan 'newIndex'. ItemsRepeater.cs 532
- Kemungkinan urutan argumen yang salah diteruskan ke metode 'Perbarui': 'oldIndex' dan 'newIndex'. ItemsRepeater.cs 536
Dua peringatan pada satu metode panggilan acara.
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); } ..... } }
Penganalisa memperhatikan bahwa argumen
oldIndex dan
newIndex ditulis dalam urutan berbeda dalam kedua metode
ItemsRepeaterElementIndexChangedEventArgs dan
Pembaruan :
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; }
Mungkin kode ini sedang ditulis oleh programmer yang berbeda, salah satunya lebih tertarik pada masa lalu, dan yang lain di masa depan :)
Sama seperti masalah sebelumnya, ini tidak membutuhkan perbaikan segera; belum ditentukan apakah kode ini sebenarnya salah.
Pesan diagnostik PVS-Studio: V3004 Pernyataan 'then' setara dengan pernyataan '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); } }
Ini adalah implementasi yang sangat aneh dari metode
ThenBy . Antarmuka
IEnumerable , di mana argumen
seq diwarisi dari, berisi metode
ThenBy , yang tampaknya dimaksudkan untuk digunakan dengan cara berikut:
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); } }
Pesan diagnostik PVS-Studio: V3106 Nilai indeks negatif yang mungkin. Nilai indeks 'indeks' bisa mencapai -1. 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]; } .... } .... }
Penganalisa yakin bahwa variabel
indeks dapat berakhir dengan nilai -1. Variabel ini diberikan nilai yang dikembalikan oleh metode
FindClosestBeforeKeyFrame , jadi mari kita melihatnya:
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."); }
Seperti yang Anda lihat, loop berisi kondisi yang diikuti oleh pernyataan pengembalian yang mengembalikan nilai iterator sebelumnya. Sulit untuk memeriksa apakah kondisi ini benar, dan saya tidak dapat memastikan dengan pasti nilai apa yang akan dimiliki
CueValue , tetapi deskripsi menunjukkan bahwa diperlukan nilai dari 0,0 hingga 1,0. Tapi kita masih bisa mengatakan beberapa kata tentang
waktu : itu adalah variabel
animationTime yang diteruskan ke metode pemanggilan, dan itu pasti lebih besar dari nol dan kurang dari satu. Jika tidak, eksekusi akan mengikuti cabang yang berbeda. Jika metode ini digunakan untuk animasi, situasi ini terlihat seperti Heisenbug yang layak. Saya akan merekomendasikan memeriksa nilai yang dikembalikan oleh
FindClosestBeforeKeyFrame jika
kasing ini memerlukan beberapa perlakuan khusus atau menghapus elemen pertama dari loop jika tidak memenuhi beberapa persyaratan lain. Saya tidak tahu bagaimana tepatnya semua ini bekerja, jadi saya akan memilih solusi kedua sebagai contoh:
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."); }
Pesan diagnostik PVS-Studio: V3117 Parameter ponsel 'telepon' tidak digunakan. Country.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; }
Ini adalah contoh yang baik tentang bagaimana analisis statis lebih baik daripada ulasan kode. Konstruktor disebut dengan tiga belas argumen, salah satunya tidak digunakan. Sebenarnya, Visual Studio juga bisa mendeteksinya, tetapi hanya dengan bantuan diagnostik tingkat ketiga (yang sering dimatikan). Kami jelas berurusan dengan bug di sini karena kelas ini juga mengandung tiga belas properti - satu per argumen - tetapi tidak ada tugas untuk variabel
Ponsel . Karena perbaikannya sudah jelas, saya tidak akan menjelaskannya.
Pesan diagnostik PVS-Studio: V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'tabItem'. TabItemContainerGenerator.cs 22
protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); tabItem.ParentTabControl = Owner; .... }
Penganalisis menganggap dereferensi nilai yang dikembalikan oleh metode
CreateContainer tidak aman. Mari kita lihat metode ini:
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 dapat melacak penugasan
nol bahkan melalui rantai lima puluh metode, tetapi tidak bisa mengatakan apakah eksekusi akan pernah mengikuti cabang itu. Saya juga tidak bisa, dalam hal ini ... Panggilan hilang di antara metode yang ditimpa dan virtual, jadi saya hanya menyarankan menulis cek tambahan untuk berjaga-jaga:
protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); if(tabItem == null) return; tabItem.ParentTabControl = Owner; .... }
Pesan diagnostik PVS-Studio: V3142 Kode tidak dapat dideteksi terdeteksi. Mungkin saja ada kesalahan. DevTools.xaml.cs 91
Tidak ada gunanya mengutip terlalu banyak kode yang mencoba menjaga ketegangan; Saya akan segera memberi tahu Anda: peringatan ini adalah positif palsu. Penganalisis mendeteksi panggilan metode yang melempar pengecualian tanpa syarat:
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"); }
Tiga puluh lima (!) Peringatan tentang kode yang tidak dapat dijangkau setelah panggilan ke metode ini terlalu banyak untuk diabaikan, jadi saya bertanya kepada salah satu pengembang apa yang terjadi di sini. Dia mengatakan kepada saya bahwa mereka menggunakan teknik, di mana Anda mengganti panggilan ke satu metode dengan panggilan ke metode lain menggunakan perpustakaan
Mono.Cecil . Perpustakaan ini memungkinkan Anda untuk mengganti panggilan tepat di kode IL.
Penganalisis kami tidak mendukung perpustakaan ini, karena itu jumlah positif palsu yang sangat besar. Ini berarti diagnostik ini harus dimatikan saat memeriksa Avalonia UI. Rasanya agak canggung, tetapi saya harus mengakui bahwa sayalah yang membuat diagnostik ini ... Tapi, seperti alat lain, penganalisa statis memerlukan penyetelan yang bagus.
Misalnya, saat ini kami sedang mengerjakan diagnostik yang mendeteksi konversi tipe yang tidak aman. Ini menghasilkan sekitar seribu positif palsu pada proyek game di mana pengecekan tipe dilakukan di sisi mesin.
Pesan diagnostik PVS-Studio: V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. 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; }
Metode ini mengembalikan
true setiap saat. Mungkin tujuannya telah berubah sejak pertama kali ditulis, tetapi lebih mirip bug. Dilihat oleh komentar di awal kelas, ini adalah kelas kontrol lain yang dipinjam dari Microsoft. Jika Anda bertanya kepada saya,
DataGrid adalah salah satu kontrol yang paling tidak stabil, jadi mungkin itu bukan ide yang baik untuk mengkonfirmasi gulir ketika tidak memenuhi persyaratan.
Kesimpulan
Beberapa bug yang dijelaskan di atas dipinjam bersama dengan kode yang disalin dari kontrol WPF, dan penulis Avalonia UI tidak ada hubungannya dengan mereka. Tapi itu tidak membuat perbedaan bagi pengguna: antarmuka crash atau glitching meninggalkan kesan buruk pada kualitas keseluruhan program.
Saya menyebutkan perlunya penyempurnaan penganalisis: false positive hanya tidak dapat dihindari karena prinsip kerja di belakang algoritma analisis statis. Mereka yang terbiasa dengan
masalah penghentian tahu bahwa ada kendala matematika dalam memproses satu bagian kode dengan yang lain. Namun, dalam kasus ini, kita berbicara tentang menonaktifkan satu diagnostik dari hampir seratus setengah. Jadi, tidak ada masalah hilangnya makna dalam kasus analisis statis. Selain itu, diagnostik ini juga bisa menghasilkan peringatan yang menunjuk pada bug asli, tetapi itu akan sulit untuk diperhatikan di antara banyak positif palsu.
Saya harus menyebutkan kualitas luar biasa dari proyek UI Avalonia! Saya berharap para pengembang akan tetap seperti itu. Sayangnya, jumlah bug tumbuh seiring dengan ukuran program. Penyempurnaan yang bijaksana dari sistem CI \ CD, yang didukung dengan analisis statis dan dinamis, adalah salah satu cara untuk mencegah bug. Dan jika Anda ingin membuat pengembangan proyek besar lebih mudah dan menghabiskan lebih sedikit waktu untuk debugging,
unduh dan coba PVS-Studio!