Artikel ini adalah hasil dari pengecekan proyek Avalonia UI menggunakan analisa statis PVS-Studio. Avalonia UI adalah open-source, cross-platform platform antarmuka pengguna berbasis XAML. Ini adalah salah satu proyek signifikan secara teknologi dalam sejarah .NET, karena memungkinkan Anda untuk membuat antarmuka lintas-platform berdasarkan pada sistem WPF. Saya harap artikel ini akan membantu penulis memperbaiki beberapa kesalahan dan meyakinkan mereka untuk menggunakan analisa statis di masa depan.
Tentang Avalonia UI
Proyek Avalonia UI (sebelumnya bernama Perspex) menyediakan kemampuan untuk membuat antarmuka pengguna yang berjalan pada Windows, Linux, dan MacOS. Ada juga dukungan eksperimental untuk Android dan iOS saat ini. Avalonia UI bukan pembungkus pembungkus, tetapi mengacu pada API asli. Berbeda dengan Formulir Xamarin, yang membungkus pembungkus Xamarin. Di salah satu video demo, saya dikejutkan oleh kemampuan untuk membawa kontrol ke konsol Debian. Selain itu, proyek ini menawarkan lebih banyak fitur untuk tata letak dan desain daripada desainer antarmuka konvensional, berkat penggunaan markup XAML.
Proyek yang sudah menggunakan Avalonia UI termasuk
AvalonStudio (IDE lintas-platform untuk pengembangan di C # dan C / C ++) dan
Core2D (editor diagram dan diagram 2D). Sebagai proyek komersial, Anda dapat membawa
Wasabi Wallet (dompet Bitcoin).
Perjuangan melawan kebutuhan untuk memiliki beberapa perpustakaan yang berbeda saat membuat aplikasi lintas platform sangat penting. Kami ingin membantu proyek, dan saya mengunduh proyek dan memeriksanya dengan penganalisa kami. Saya berharap bahwa penulis akan memperhatikan artikel ini dan membuat perubahan yang diperlukan pada kode, atau mungkin memperkenalkan analisis statis reguler ke dalam proses pengembangan. Untuk melakukan ini, mereka dapat mengambil keuntungan dari opsi lisensi bebas PVS-Studio untuk proyek sumber terbuka. Penggunaan penganalisis statis secara teratur membantu menghindari banyak masalah dan mengurangi biaya untuk mendeteksi dan memperbaiki banyak kesalahan.
Hasil Validasi
PVS-Studio Warning: V3001 Ada sub-ekspresi yang identik 'controlledFag' 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; .... }
Saya akan mulai secara simbolis dengan diagnostik C # pertama kami. Analyzer mendeteksi penggunaan aneh dari operator bitwise OR. Mari saya jelaskan pada angka apa yang terjadi di sini:
ekspresi
1100 0011 | 1111 0000 ^ 1111 0000
mirip dengan ini:
1100 0011 | 0000 0000
ATAU eksklusif ("^") memiliki prioritas lebih tinggi daripada ATAU bitwise ("|"). Kemungkinan besar, urutan operasi yang berbeda tersirat di sini. Dalam hal ini, Anda harus menggunakan ekspresi pertama dalam tanda kurung:
private void UpdateWMStyles(Action change) { .... style = (style | controlledFlags) ^ controlledFlags; .... }
Sebelum dua peringatan berikutnya saya harus mengakui: positif palsu. Ini disebabkan oleh penggunaan API publik, metode
TransformToVisual . Dalam kasus kami,
VisualRoot selalu menjadi induk dari
visual . Saya tidak mengerti ini ketika menganalisis respons, penulis proyek mengatakan kepada saya setelah menulis artikel. Jadi pengeditan yang diusulkan dalam artikel ini bukan untuk perlindungan terhadap kejatuhan yang sebenarnya, tetapi terhadap revisi potensial yang merusak logika ini.
PVS-Studio Warning: V3080 Kemungkinan null dereference dari 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 kecil. Penganalisa menganggap bahwa mendereferensi hasil panggilan ke TranslatePoint tidak aman. Lihatlah 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, ada
null kembali.
Metode ini memiliki 6 panggilan. Dalam tiga kasus, nilainya diperiksa, dan selebihnya PVS-Studio mendeteksi potensi dereferencing dan mengeluarkan peringatan. Saya mengutip yang pertama di atas, dan dua peringatan lainnya ada di sini:
- V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'p'. VisualExtensions.cs 35
- V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'controlPoint'. Scene.cs 176
Saya mengusulkan untuk
memperbaikinya dengan analogi dengan menambahkan
<Struct> Nullable. Pemeriksaan nilai 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 ....; }
PVS-Studio Warning: V3080 Kemungkinan null dereference dari nilai pengembalian metode. Pertimbangkan untuk memeriksa: TransformToVisual (...). ViewportManager.cs 381
Kasus yang sangat mirip dengan contoh sebelumnya:
private void OnEffectiveViewportChanged(TransformedBounds? bounds) { .... var transform = _owner.GetVisualRoot().TransformToVisual(_owner).Value; .... }
Metode
TransformToVisual terlihat seperti ini:
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 digunakan di sembilan tempat, ada cek di tujuh. Peringatan pertama untuk digunakan tanpa memeriksa lebih tinggi, dan yang terakhir ada di sini:
V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'transformasi'. MouseDevice.cs 80
Peringatan 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 aneh. Dalam enumerasi
NavigationDirection , ada 9 jenis dan
PageDown adalah yang terakhir dari mereka. Mungkin ini tidak selalu terjadi, atau apakah asuransi terhadap penampilan SUDDEN dari opsi rujukan baru. Menurut saya cek pertama sudah cukup di sini. Saya akan menyerahkan keputusan kepada penulis proyek.
Peringatan 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 }; }
Dalam hal ini, penganalisa menyarankan bahwa argumen kedua dan ketiga dari konstruktor bingung. Mari kita lihat konstruktor yang disebut:
public SelectionChangedEventArgs(RoutedEvent routedEvent, IList addedItems, IList removedItems) : base(routedEvent) { AddedItems = addedItems; RemovedItems = removedItems; }
Dua wadah tipe
IList diterima, mudah dicampur. Dilihat oleh komentar di awal kelas, ini adalah kesalahan dalam kode kontrol yang disalin dari Microsoft dan dimodifikasi di bawah Avalonia. Tetapi bagi saya itu bermanfaat untuk memperbaiki urutan argumen metode ini, setidaknya untuk tidak mencari kemungkinan kesalahan dalam diri Anda ketika laporan bug tiba.
Penganalisa menemukan tiga kesalahan serupa:
Peringatan PVS-Studio: V3066 Kemungkinan urutan argumen yang salah diteruskan ke 'konstruktor SelectionChangedEventArgs': 'dihapus' dan 'ditambahkan'. AutoCompleteBox.cs 707
OnSelectionChanged(new SelectionChangedEventArgs(SelectionChangedEvent, removed, added));
Pilihan konstruktor yang
samaChangedEventArgs.Peringatan 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 operasi dalam 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 melihat bahwa di
ItemsRepeaterElementIndexChangedEventArgs dan dalam metode
Pembaruan , argumen
oldIndex dan
newIndex memiliki urutan berbeda:
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 kodenya ditulis oleh programmer yang berbeda, dan untuk yang satu lebih penting apa yang terjadi, dan untuk yang lain - apa yang akan terjadi :)
Seperti dalam kasus sebelumnya, Anda tidak harus mengeditnya segera, Anda perlu memeriksa apakah memang ada kesalahan.
PVS-Studio Warning: 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); } }
Implementasi metode
ThenBy yang sangat menarik. Antarmuka
IEnumerable , dari mana argumen
seq diwarisi
, memiliki metode
ThenBy ; Saya kira penggunaannya tersirat. Seperti ini:
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); } }
Peringatan 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 percaya bahwa
indeks dapat memiliki nilai -1. Variabel diperoleh dari metode
FindClosestBeforeKeyFrame , lihat:
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 dapat kita lihat, kondisi diperiksa dalam loop dan nilai iterator sebelumnya dikembalikan. Kondisi ini cukup sulit untuk diverifikasi, dan saya tidak bisa mengatakan dengan tepat apa itu
CueValue , tetapi menurut uraiannya dibutuhkan nilai dari 0,0 hingga 1,0. Kita dapat mengatakan sesuatu tentang
waktu , ini adalah
animationTime dari metode panggilan, jelas lebih dari nol dan kurang dari satu. Jika tidak, eksekusi program akan berjalan di cabang yang berbeda. Jika ini adalah metode yang dipanggil untuk membuat animasi, maka situasinya terlihat seperti bug mengambang yang bagus. Saya akan menambahkan cek untuk hasil
FindClosestBeforeKeyFrame jika penanganan khusus diperlukan dalam kasus ini. Atau - jika elemen pertama tidak memenuhi beberapa kondisi lain - hapus dari loop. Tidak tahu bagaimana semua ini harus bekerja, saya akan memilih opsi kedua sebagai contoh koreksi:
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."); }
Peringatan PVS-Studio: V3117 Parameter konstruktor 'ponsel' 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; }
Contoh yang baik dari perbedaan antara operasi penganalisa dan tinjauan kode manual. Tiga belas argumen konstruktor, satu tidak digunakan. Bahkan, Visual Studio juga mencatat argumen yang tidak digunakan, tetapi pada peringatan tingkat ketiga (mereka sering dinonaktifkan). Dalam hal ini, ini adalah kesalahan yang jelas, karena kelas juga memiliki tiga belas properti untuk setiap argumen, dan tidak ada nilai yang diberikan di mana pun di
Ponsel . Pengeditan sudah jelas, saya tidak akan membawanya.
Peringatan 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; .... }
Penganalisa menganggap berbahaya untuk melakukan dereferensi hasil pemanggilan
CreateContainer .
Lihatlah 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; } }
Penganalisis dapat melihat penugasan
nol ke variabel bahkan ketika melewati nilai melalui rantai lima puluh metode. Tapi dia tidak bisa mengatakan apakah eksekusi akan dilakukan di utas ini setidaknya sekali. Ya, dan saya juga tidak bisa, secara umum ... Panggilan metode hilang di antara metode yang ditimpa dan virtual. Jadi saya sarankan agar aman dengan cek tambahan:
protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); if(tabItem == null) return null; tabItem.ParentTabControl = Owner; .... }
Peringatan PVS-Studio: V3142 Kode yang tidak dapat dideteksi terdeteksi. Mungkin saja ada kesalahan. DevTools.xaml.cs 91
Saya tidak akan menulis banyak kode di sini untuk membuat intrik, saya akan mengatakan segera: peringatan itu salah. Penganalisa melihat panggilan ke metode yang melempar pengecualian tanpa syarat. Ini dia:
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"); }
Mustahil untuk tidak memperhatikan tiga puluh lima peringatan (!) Tentang kode yang tidak dapat dijangkau yang terletak setelah panggilan ke metode ini. Saya bertanya kepada salah satu pengembang proyek: bagaimana cara kerjanya? Dan mereka memberi tahu saya cara mengganti panggilan dari satu metode dengan panggilan yang lain menggunakan perpustakaan
Mono.Cecil . Ini memungkinkan Anda untuk mengganti panggilan secara langsung dalam kode IL.
Penganalisa tidak mendukung perpustakaan ini, oleh karena itu, kami memiliki banyak kesalahan positif, jadi lebih baik untuk menonaktifkan diagnostik ini pada proyek ini. Agak memalukan untuk mengakui, sayalah yang melakukan diagnosis ini ... tetapi, seperti alat apa pun, analisis statis perlu dikonfigurasikan.
Misalnya, kami sedang mengembangkan diagnostik tentang konversi jenis yang tidak aman. Dan itu memberikan operasi kurang sedikit dari seribu pada proyek game, di mana kontrol mengetik dilakukan di sisi mesin.
Peringatan PVS-Studio: V3009 Aneh bahwa metode ini selalu mengembalikan nilai yang sama dan '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 selalu mengembalikan
true . Mungkin tujuan metode ini telah berubah sejak tanda tangan ditulis, tetapi kemungkinan besar ini adalah kesalahan. Ini juga kelas kontrol yang disalin dari Microsoft, dilihat dari komentar di awal kelas. Menurut pendapat saya,
DataGrid biasanya merupakan salah satu kontrol yang paling tidak stabil, dan bagi saya itu layak untuk dipikirkan apakah perlu untuk mengkonfirmasi gulir jika tidak memenuhi persyaratan?
Kesimpulan
Beberapa kesalahan ini diperkenalkan ke dalam proyek bukan oleh pengembang UI Avalonia sendiri, tetapi oleh kode yang disalin dari kontrol WPF. Namun, untuk pengguna antarmuka, sumber kesalahan biasanya tidak masalah. Antarmuka yang macet atau tidak berfungsi merusak pendapat seluruh program.
Dalam artikel yang saya sebutkan kebutuhan untuk mengkonfigurasi analisa, ada positif palsu yang tidak dapat dihindari karena prinsip-prinsip pengoperasian algoritma analisis statis. Siapa pun yang terbiasa dengan
masalah berhenti menyadari keterbatasan matematika ketika bekerja dengan kode lain. Tetapi dalam kasus ini kita berbicara tentang menonaktifkan satu diagnostik dari hampir seratus setengah. Jadi kita tidak berbicara tentang hilangnya makna dalam analisis statis (atau pertanyaan itu tidak sepadan). Selain itu, diagnosis ini mungkin memiliki respons yang baik, hanya saja lebih sulit untuk menemukan di antara massa positif palsu.
Pastikan untuk mencatat kualitas tinggi dari kode proyek! Saya berharap para pengembang akan menjaga kecepatan dan tingkat kualitas kode. Sayangnya, semakin besar proyeknya, semakin banyak bug di dalamnya. Salah satu cara yang mungkin untuk mengurangi bug adalah dengan mengkonfigurasi CI \ CD dengan benar, dengan koneksi analisis statis dan dinamis. Dan jika Anda ingin menyederhanakan pekerjaan dengan proyek besar dan mengurangi jumlah waktu yang dibutuhkan untuk debugging,
unduh dan coba PVS-Studio!
Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Alexander Senichkin.
Kontribusi Kecil Kami untuk Avalonia UI's Fight for Fewer Platforms .