Geekly articles weekly

Memeriksa kode sumber Roslyn

PVS-Studio vs Roslyn

Dari waktu ke waktu, kami kembali ke proyek yang sebelumnya kami uji dengan PVS-Studio dan menulis artikel tentang itu. Ada dua alasan untuk melakukan ini. Pertama, untuk memahami seberapa baik analisa kami. Kedua, untuk melacak apakah penulis proyek memperhatikan artikel kami, serta laporan kesalahan yang biasanya kami berikan kepada mereka. Tentu saja, kesalahan dapat diperbaiki tanpa partisipasi kami. Tapi itu selalu baik ketika tepatnya upaya kami membantu membuat proyek menjadi lebih baik. Roslyn tidak terkecuali. Artikel ulasan sebelumnya tentang proyek ini berasal dari 23 Desember 2015. Ini cukup lama, mengingat jalur yang telah ditempuh oleh analis kami dalam pengembangannya selama ini. Bagi kami secara pribadi, Roslyn juga memiliki minat tambahan dengan fakta bahwa inti dari C # analyzer PVS-Studio didasarkan padanya. Oleh karena itu, kami sangat tertarik dengan kualitas kode untuk proyek ini. Kami akan mengatur pemeriksaan kedua dan mencari tahu apa yang baru dan menarik (tapi semoga tidak ada yang signifikan) PVS-Studio dapat temukan di sana.

Roslyn (atau .NET Compiler Platform) mungkin akrab bagi banyak pembaca kami. Singkatnya, ini adalah kumpulan kompiler sumber terbuka dan API untuk analisis kode untuk bahasa C # dan Visual Basic .NET. Kode sumber untuk proyek tersedia di GitHub .

Saya tidak akan memberikan uraian terperinci tentang platform ini, tetapi saya akan merekomendasikan kepada semua orang yang tertarik dengan artikel oleh kolega saya Sergey Vasiliev, " Pengantar Roslyn. Menggunakan alat analisis statis untuk mengembangkan ." Dari artikel ini Anda dapat belajar tidak hanya tentang fitur arsitektur Roslyn, tetapi juga bagaimana tepatnya kita menggunakan platform ini.

Seperti yang saya sebutkan sebelumnya, lebih dari tiga tahun telah berlalu sejak penulisan artikel terakhir oleh rekan saya Andrei Karpov tentang cek Roslyn " rilis Tahun Baru PVS-Studio 6.00: memeriksa Roslyn ". Selama ini, penganalisa C # PVS-Studio telah memperoleh banyak fitur baru. Secara umum, artikel Andrey adalah semacam "bola uji", karena C # analyzer hanya ditambahkan ke PVS-Studio saat itu. Meskipun demikian, meskipun demikian, dalam proyek berkualitas tinggi tanpa syarat, Roslyn berhasil menemukan kesalahan yang menarik. Apa yang telah berubah dalam alat analisis untuk kode C # sejauh ini, yang berpotensi memungkinkan analisis yang lebih dalam?

Selama beberapa waktu terakhir, inti penganalisa dan infrastruktur telah berkembang. Dukungan ditambahkan untuk Visual Studio 2017 dan Roslyn 2.0, serta integrasi mendalam dengan MSBuild. Anda dapat membaca lebih lanjut tentang pendekatan kami untuk integrasi dengan MSBuild dan tentang alasan yang membuat kami menerimanya dalam artikel oleh rekan saya Pavel Yeremeyev, " Dukungan untuk Visual Studio 2017 dan Roslyn 2.0 di PVS-Studio: kadang-kadang menggunakan solusi yang sudah jadi tidak semudah kelihatannya sekilas . "

Sekarang kami secara aktif bekerja pada transisi ke Roslyn 3.0 sesuai dengan skema yang sama yang kami awalnya mendukung Visual Studio 2017, yaitu, melalui toolset kami sendiri, yang datang dalam kit distribusi PVS-Studio dengan "rintisan" dalam bentuk file MSBuild.exe kosong. Terlepas dari kenyataan bahwa itu terlihat seperti "penopang" (API MSBuild tidak terlalu ramah untuk digunakan kembali dalam proyek-proyek patry ketiga karena portabilitas yang rendah dari perpustakaan), pendekatan ini telah membantu kami menghidupkan kembali beberapa pembaruan Roslyn yang relatif tanpa rasa sakit selama kehidupan Visual Studio 2017. dan sekarang, meskipun dengan sejumlah besar overlay, selamat dari upgrade ke Visual Studio 2019, serta mempertahankan kompatibilitas mundur penuh dan kinerja pada sistem dengan versi MSBuild yang lebih lama.

Inti penganalisa juga telah mengalami sejumlah perbaikan. Salah satu inovasi utama adalah analisis antar-prosedur yang lengkap, dengan mempertimbangkan nilai input dan output metode, dengan mempertimbangkan, tergantung pada parameter ini, jangkauan cabang-cabang eksekusi dan poin pengembalian.

Tugas melacak parameter di dalam metode sudah hampir selesai, sambil mempertahankan anotasi otomatis untuk apa yang terjadi dengan parameter ini di sana (misalnya, dereferencing yang berpotensi berbahaya). Ini akan memungkinkan untuk diagnostik apa pun yang menggunakan mekanisme aliran data untuk memperhitungkan situasi berbahaya yang terjadi ketika meneruskan parameter ke suatu metode. Sebelumnya, ketika menganalisis tempat berbahaya seperti itu, peringatan tidak dibuat, karena kami tidak bisa mengetahui semua nilai input yang mungkin untuk metode tersebut. Sekarang kita dapat mendeteksi bahaya, karena di semua tempat di mana metode ini dipanggil, parameter input ini akan diperhitungkan.

Catatan: Anda dapat membiasakan diri dengan mekanisme utama alat analisis, seperti aliran data dan lainnya, dari artikel " Teknologi yang Digunakan dalam Penganalisis Kode PVS-Studio untuk Menemukan Kesalahan dan Kerentanan Potensial ".

Analisis antar-prosedur dalam PVS-Studio C # tidak dibatasi oleh parameter input atau kedalaman. Satu-satunya batasan adalah metode virtual di kelas yang tidak ditutup untuk warisan dan jatuh ke rekursi (kita akan berhenti ketika kita melihat pada stack panggilan berulang ke metode yang sudah dihitung). Selain itu, metode rekursif itu sendiri pada akhirnya akan dihitung dengan asumsi bahwa nilai kembalinya rekursi sendiri tidak diketahui.

Inovasi besar lainnya dalam penganalisis C # adalah kemungkinan dereferensi penunjuk yang berpotensi nol. Sebelumnya, analisa bersumpah pada pengecualian referensi nol yang mungkin jika yakin bahwa di semua cabang eksekusi nilai variabel akan menjadi nol. Tentu saja, ia terkadang salah, sehingga diagnostik V3080 sebelumnya disebut referensi nol potensial.

Sekarang penganalisa mengingat bahwa variabel bisa nol di salah satu cabang eksekusi (misalnya, dalam kondisi tertentu jika). Jika dia melihat akses ke variabel seperti itu tanpa memeriksa, dia akan mendapatkan pesan V3080, tetapi pada tingkat kepentingan yang lebih rendah daripada jika dia melihat nol di semua cabang. Dalam kombinasi dengan peningkatan analisis antarproedural, mekanisme semacam itu memungkinkan menemukan kesalahan yang sangat sulit dideteksi. Contohnya adalah rangkaian panjang pemanggilan metode, yang terakhir yang tidak Anda kenal, dan yang, misalnya, mengembalikan nol dalam keadaan tertentu, tetapi Anda tidak melindungi diri dari ini karena Anda sama sekali tidak mengetahuinya. Dalam hal ini, penganalisa hanya bersumpah ketika secara tepat melihat penugasan nol. Menurut pendapat kami, ini secara kualitatif membedakan pendekatan kami dari inovasi C # 8.0 sebagai jenis referensi yang dapat dibatalkan, yang, pada kenyataannya, bermuara pada pengaturan pemeriksaan nol dalam setiap metode. Kami menawarkan alternatif - untuk melakukan pemeriksaan hanya di mana null benar-benar dapat datang, dan penganalisa kami sekarang dapat mencari situasi seperti itu.

Jadi, tanpa penundaan, mari beralih ke "tanya jawab" - menganalisis hasil pemeriksaan Roslyn. Pertama, mari kita lihat kesalahan yang ditemukan berkat inovasi yang dijelaskan di atas. Secara umum, beberapa peringatan dikeluarkan untuk kode Roslyn kali ini. Saya pikir ini disebabkan oleh kenyataan bahwa platform berkembang sangat aktif (basis kode saat ini berada di sekitar 2.770.000 baris kode, tidak termasuk yang kosong), dan kami belum melakukan analisis proyek ini untuk waktu yang lama. Namun, tidak ada begitu banyak kesalahan kritis, yaitu mereka menarik untuk artikel ini. Dan ya, ada beberapa tes di Roslyn yang saya, seperti biasa, tidak termasuk dalam pengujian.

Saya akan mulai dengan kesalahan V3080, dengan tingkat kekritisan sedang, di mana penganalisa mendeteksi kemungkinan akses melalui tautan nol, tetapi tidak dalam semua kasus yang mungkin (cabang kode).

Kemungkinan null dereference - Medium

V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'saat ini'. CSharpSyntaxTreeFactoryService.PositionalSyntaxReference.cs 70

private SyntaxNode GetNode(SyntaxNode root) { var current = root; .... while (current.FullSpan.Contains(....)) // <= { .... var nodeOrToken = current.ChildThatContainsPosition(....); .... current = nodeOrToken.AsNode(); // <= } .... } public SyntaxNode AsNode() { if (_token != null) { return null; } return _nodeOrParent; }

Pertimbangkan metode GetNode . Penganalisa menganggap bahwa akses dengan referensi nol dimungkinkan dalam kondisi blok sementara . Di dalam tubuh blok sementara , variabel saat ini akan diberi nilai - hasil dari eksekusi metode AsNode . Dan nilai ini dalam beberapa kasus akan menjadi nol . Contoh yang baik dari analisis antar prosedur dalam aksi.

Sekarang pertimbangkan kasus serupa di mana analisis antar-prosedur dilakukan melalui dua pemanggilan metode.

V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'direktori'. CommonCommandLineParser.cs 911

 private IEnumerable<CommandLineSourceFile> ExpandFileNamePattern(string path, string baseDirectory, ....) { string directory = PathUtilities.GetDirectoryName(path); .... var resolvedDirectoryPath = (directory.Length == 0) ? // <= baseDirectory : FileUtilities.ResolveRelativePath(directory, baseDirectory); .... } public static string GetDirectoryName(string path) { return GetDirectoryName(path, IsUnixLikePlatform); } internal static string GetDirectoryName(string path, bool isUnixLike) { if (path != null) { .... } return null; }

Variabel direktori dalam tubuh metode ExpandFileNamePattern mendapatkan nilai dari metode GetDirectoryName (string) . Itu, pada gilirannya, akan mengembalikan hasil dari metode GetDirectoryName (string, bool) yang kelebihan beban, nilainya mungkin nol . Karena lebih jauh dalam tubuh metode ExpandFileNamePattern variabel direktori digunakan tanpa memeriksa awal untuk kesetaraan nol , kita dapat berbicara tentang legitimasi peringatan oleh penganalisis. Ini adalah desain yang berpotensi tidak aman.

Sepotong kode dengan kesalahan V3080, lebih tepatnya, segera dengan dua kesalahan yang dikeluarkan untuk satu baris kode. Di sini, analisis antar-prosedur tidak diperlukan.

V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'spanStartLocation'. TestWorkspace.cs 574

V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'spanEndLocationExclusive'. TestWorkspace.cs 574

 private void MapMarkupSpans(....) { .... foreach (....) { .... foreach (....) { .... int? spanStartLocation = null; int? spanEndLocationExclusive = null; foreach (....) { if (....) { if (spanStartLocation == null && positionInMarkup <= markupSpanStart && ....) { .... spanStartLocation = ....; } if (spanEndLocationExclusive == null && positionInMarkup <= markupSpanEndExclusive && ....) { .... spanEndLocationExclusive = ....; break; } .... } .... } tempMappedMarkupSpans[key]. Add(new TextSpan( spanStartLocation.Value, // <= spanEndLocationExclusive.Value - // <= spanStartLocation.Value)); } } .... }

Variabel spanStartLocation dan spanEndLocationExclusive adalah tipe nullable int dan diinisialisasi ke nol . Lebih lanjut dalam kode mereka dapat diberi nilai, tetapi hanya jika kondisi tertentu terpenuhi. Dalam beberapa kasus, nilainya akan tetap sama dengan nol . Lebih lanjut dalam kode, variabel-variabel ini diakses dengan referensi tanpa terlebih dahulu memeriksa kesetaraan nol , seperti yang ditunjukkan oleh penganalisa.

Kode Roslyn mengandung beberapa kesalahan seperti itu, lebih dari 100. Seringkali pola kesalahan ini sama. Ada beberapa metode umum yang berpotensi menghasilkan null . Hasil dari metode ini digunakan di banyak tempat, kadang-kadang melalui puluhan panggilan metode menengah atau pemeriksaan tambahan. Penting untuk dipahami bahwa kesalahan ini tidak fatal, tetapi mereka berpotensi menyebabkan akses melalui tautan nol. Dan untuk mendeteksi kesalahan seperti itu sangat sulit. Oleh karena itu, dalam beberapa kasus, Anda harus mempertimbangkan refactoring kode, di mana pengecualian akan dilemparkan jika metode mengembalikan nol . Jika tidak, Anda hanya dapat mengamankan kode Anda dengan total cek, yang cukup membosankan dan tidak dapat diandalkan. Tentu saja, dalam setiap kasus, keputusan harus dibuat berdasarkan karakteristik proyek.

Catatan Itu terjadi bahwa saat ini tidak ada situasi (input data) di mana metode mengembalikan nol dan tidak ada kesalahan nyata. Namun, kode semacam itu masih tidak dapat diandalkan, karena semuanya dapat berubah ketika perubahan dilakukan pada kode tersebut.

Untuk menutup topik dengan V3080 , mari kita lihat kesalahan yang jelas dengan tingkat kepercayaan tinggi, ketika akses melalui tautan nol lebih mungkin atau bahkan tidak terhindarkan.

Kemungkinan null dereference - Tinggi

V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'collectionType.Type'. AbstractConvertForToForEachCodeRefactoringProvider.cs 137

 public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) { .... var collectionType = semanticModel.GetTypeInfo(....); if (collectionType.Type == null && collectionType.Type.TypeKind == TypeKind.Error) { return; } .... }

Karena kesalahan ketik dalam kondisi (alih-alih operator || yang kami gunakan && ), kode tidak berfungsi sebagaimana dimaksud, dan variabel collectionType.Type akan diakses jika nol . Kondisi tersebut harus diperbaiki sebagai berikut:

 if (collectionType.Type == null || collectionType.Type.TypeKind == TypeKind.Error) ....

Omong-omong, varian kedua dari pengembangan acara juga dimungkinkan: di bagian pertama, kondisinya digabungkan oleh operator == dan ! = . Maka kode yang diperbaiki adalah:

 if (collectionType.Type != null && collectionType.Type.TypeKind == TypeKind.Error) ....

Versi kode ini kurang logis, tetapi juga memperbaiki kesalahan. Keputusan akhir tergantung pada penulis proyek.

Kesalahan serupa lainnya.

V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'tindakan'. TextViewWindow_InProc.cs 372

 private Func<IWpfTextView, Task> GetLightBulbApplicationAction(....) { .... if (action == null) { throw new InvalidOperationException( $"Unable to find FixAll in {fixAllScope.ToString()} code fix for suggested action '{action.DisplayText}'."); } .... }

Kesalahan dibuat saat menulis pesan untuk pengecualian. Pada saat yang sama, upaya dilakukan untuk mengakses properti action.DisplayText melalui variabel tindakan , yang jelas nol .

Dan kesalahan terakhir adalah V3080 High level.

V3080 Kemungkinan null dereference. Pertimbangkan untuk memeriksa 'tipe'. ObjectFormatterHelpers.cs 91

 private static bool IsApplicableAttribute( TypeInfo type, TypeInfo targetType, string targetTypeName) { return type != null && AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName; }

Metode ini kecil, jadi saya memberikan seluruh kodenya. Kondisi di blok kembali salah. Dalam beberapa kasus, dimungkinkan untuk melempar NullReferenceException saat mengakses type.FullName . Saya menggunakan tanda kurung (mereka tidak akan mengubah perilaku di sini) untuk mengklarifikasi situasi:

 return (type != null && AreEquivalent(targetType, type)) || (targetTypeName != null && type.FullName == targetTypeName);

Begitulah, sesuai dengan prioritas operasi, kode ini akan berfungsi. Jika variabel type adalah null , kita masuk ke cek lain, di mana, memastikan variabel targetTypeName adalah null , kami menggunakan referensi tipe nol. Anda dapat memperbaiki kode, misalnya, seperti ini:

 return type != null && (AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName);

Saya pikir di situlah Anda dapat menyelesaikan studi tentang kesalahan V3080 dan melihat apa lagi yang berhasil ditemukan oleh penganalisa PVS-Studio dalam kode Roslyn.

Salah ketik

V3005 Variabel 'SourceCodeKind' ditugaskan untuk dirinya sendiri. DynamicFileInfo.cs 17

 internal sealed class DynamicFileInfo { .... public DynamicFileInfo( string filePath, SourceCodeKind sourceCodeKind, TextLoader textLoader, IDocumentServiceProvider documentServiceProvider) { FilePath = filePath; SourceCodeKind = SourceCodeKind; // <= TextLoader = textLoader; DocumentServiceProvider = documentServiceProvider; } .... }

Karena nama variabel yang gagal, salah ketik dibuat di konstruktor kelas DynamicFileInfo . Bidang SourceCodeKind diberikan nilainya sendiri, alih-alih menggunakan parameter sourceCodeKind . Untuk meminimalkan kemungkinan kesalahan tersebut, disarankan untuk menggunakan awalan garis bawah untuk nama parameter dalam kasus tersebut. Saya akan memberikan contoh versi kode yang diperbaiki:

 public DynamicFileInfo( string _filePath, SourceCodeKind _sourceCodeKind, TextLoader _textLoader, IDocumentServiceProvider _documentServiceProvider) { FilePath = _filePath; SourceCodeKind = _sourceCodeKind; TextLoader = _textLoader; DocumentServiceProvider = _documentServiceProvider; }

Kecerobohan

V3006 Objek telah dibuat tetapi tidak sedang digunakan. Kata kunci 'melempar' bisa hilang: lempar InvalidOperationException (FOO) baru. ProjectBuildManager.cs 61

 ~ProjectBuildManager() { if (_batchBuildStarted) { new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }

Di bawah kondisi tertentu, destruktor harus melempar pengecualian, tetapi ini tidak terjadi, dan objek pengecualian hanya dibuat. Kata kunci lemparan dihilangkan. Versi kode yang diperbaiki:

 ~ProjectBuildManager() { if (_batchBuildStarted) { throw new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }

Masalah bekerja dengan destruktor di C # dan melemparkan pengecualian dari mereka adalah topik untuk diskusi terpisah, yang berada di luar cakupan artikel ini.

Ketika hasilnya tidak penting

Sejumlah peringatan V3009 telah diterima untuk metode yang mengembalikan nilai yang sama dalam semua kasus. Kadang-kadang ini tidak kritis, atau kode kembali tidak diperiksa dalam kode panggilan. Saya melewatkan peringatan seperti itu. Tetapi beberapa kode terasa mencurigakan bagi saya. Saya akan mengutip salah satu dari mereka:

V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. GoToDefinitionCommandHandler.cs 62

 internal bool TryExecuteCommand(....) { .... using (context.OperationContext.AddScope(....)) { if (....) { return true; } } .... return true; }

Metode TryExecuteCommand hanya mengembalikan true , dan tidak lain kecuali true . Pada saat yang sama, nilai pengembalian terlibat dalam beberapa pemeriksaan dalam kode panggilan:

 public bool ExecuteCommand(....) { .... if (caretPos.HasValue && TryExecuteCommand(....)) { .... } .... }

Sulit untuk mengatakan betapa berbahayanya perilaku ini. Tetapi jika hasilnya tidak diperlukan, mungkin perlu mengganti jenis kembali dengan void dan membuat sedikit perubahan pada metode pemanggilan. Ini akan membuat kode lebih mudah dimengerti dan aman.

Peringatan serupa lainnya:

  • V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. CommentUncommentSelectionCommandHandler.cs 86
  • V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. RenameTrackingTaggerProvider.RenameTrackingCommitter.cs 99
  • V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. JsonRpcClient.cs 138
  • V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. AbstractFormatEngine.OperationApplier.cs 164
  • V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama 'salah'. TriviaDataFactory.CodeShapeAnalyzer.cs 254
  • V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. ObjectList.cs 173
  • V3009 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama dari 'benar'. ObjectList.cs 249

Tidak dicentang

V3019 Mungkin variabel yang salah dibandingkan dengan nol setelah konversi jenis menggunakan kata kunci 'sebagai'. Periksa variabel 'nilai', 'valueToSerialize'. RoamingVisualStudioProfileOptionPersister.cs 277

 public bool TryPersist(OptionKey optionKey, object value) { .... var valueToSerialize = value as NamingStylePreferences; if (value != null) { value = valueToSerialize.CreateXElement().ToString(); } .... }

Nilai variabel adalah tipe NamingStylePreferences . Masalahnya mengikuti cek ini. Bahkan jika variabel nilai bukan nol, ini tidak menjamin bahwa konversi tipe berhasil dan valueToSerialize tidak mengandung nol . NullReferenceException dapat dilempar . Kode perlu diperbaiki sebagai berikut:

 var valueToSerialize = value as NamingStylePreferences; if (valueToSerialize != null) { value = valueToSerialize.CreateXElement().ToString(); }

Dan satu lagi kesalahan serupa.

V3019 Mungkin variabel yang salah dibandingkan dengan nol setelah konversi jenis menggunakan kata kunci 'sebagai'. Periksa variabel 'columnState', 'columnState2'. StreamingFindUsagesPresenter.cs 181

 private void SetDefinitionGroupingPriority(....) { .... foreach (var columnState in ....) { var columnState2 = columnState as ColumnState2; if (columnState?.Name == // <= StandardTableColumnDefinitions2.Definition) { newColumns.Add(new ColumnState2( columnState2.Name, // <= ....)); } .... } .... }

Variabel columnState adalah tipe ColumnState2 . Namun, hasil operasi, variabel columnState2 , tidak diperiksa lebih lanjut untuk null . Sebagai gantinya, variabel columnState diperiksa menggunakan pernyataan null bersyarat. Apa bahayanya kode ini? Seperti dalam contoh sebelumnya, ketik casting menggunakan operator as mungkin gagal, dan variabel columnState2 akan menjadi nol , yang selanjutnya akan melempar pengecualian. Ngomong-ngomong, kesalahan ketik mungkin bisa disalahkan. Perhatikan kondisi di blok if . Mungkin alih-alih columnState? .Name mereka ingin menulis columnState2? .Name . Ini sangat mungkin diberikan nama variabel yang agak disayangkan columnState dan columnState2.

Cek berlebihan

Sejumlah besar peringatan (lebih dari 100) dikeluarkan untuk konstruksi yang tidak kritis, tetapi berpotensi tidak aman terkait dengan cek yang berlebihan. Sebagai contoh, saya akan memberikan salah satunya.

Ekspresi V3022 'navInfo == null' selalu salah. AbstractSyncClassViewCommandHandler.cs 101

 public bool ExecuteCommand(....) { .... IVsNavInfo navInfo = null; if (symbol != null) { navInfo = libraryService.NavInfoFactory.CreateForSymbol(....); } if (navInfo == null) { navInfo = libraryService.NavInfoFactory.CreateForProject(....); } if (navInfo == null) // <= { return true; } .... } public IVsNavInfo CreateForSymbol(....) { .... return null; } public IVsNavInfo CreateForProject(....) { return new NavInfo(....); }

Mungkin tidak ada kesalahan nyata di sini. Hanya alasan yang bagus untuk menunjukkan kombinasi teknologi "analisis antar-proses + analisis aliran data" dalam tindakan. Penganalisa menganggap bahwa cek kedua navInfo == null adalah mubazir. Memang, sebelum itu, nilai untuk menetapkan navInfo akan diperoleh dari libraryService.NavInfoFactory.CreateForProject , yang akan membangun dan mengembalikan objek baru dari kelas NavInfo . Tapi bukan nol sama sekali. Pertanyaannya adalah, mengapa penganalisis tidak menghasilkan peringatan untuk pemeriksaan pertama navInfo == null ? Ada penjelasan untuk ini. Pertama, jika variabel simbol ternyata nol , maka nilai navInfo akan tetap menjadi referensi nol. Kedua, bahkan jika navInfo mendapatkan nilai dari metode libraryService.NavInfoFactory.CreateForSymbol , nilai ini mungkin nol . Jadi pemeriksaan pertama navInfo == null benar-benar diperlukan.

Cek tidak cukup

Dan sekarang situasinya adalah kebalikan dari yang di atas. Beberapa peringatan V3042 diterima untuk kode yang dapat diakses dengan referensi nol. Selain itu, hanya satu atau dua cek kecil yang dapat memperbaiki semuanya.

Pertimbangkan satu bagian kode yang menarik yang mengandung dua kesalahan seperti itu.

V3042 Kemungkinan NullReferenceException. '?.' dan '.' operator digunakan untuk mengakses anggota objek 'penerima' Binder_Expressions.cs 7770

V3042 Kemungkinan NullReferenceException. '?.' dan '.' operator digunakan untuk mengakses anggota objek 'penerima' Binder_Expressions.cs 7776

 private BoundExpression GetReceiverForConditionalBinding( ExpressionSyntax binding, DiagnosticBag diagnostics) { .... BoundExpression receiver = this.ConditionalReceiverExpression; if (receiver?.Syntax != // <= GetConditionalReceiverSyntax(conditionalAccessNode)) { receiver = BindConditionalAccessReceiver(conditionalAccessNode, diagnostics); } var receiverType = receiver.Type; // <= if (receiverType?.IsNullableType() == true) { .... } receiver = new BoundConditionalReceiver(receiver.Syntax, 0, // <= receiverType ?? CreateErrorType(), hasErrors: receiver.HasErrors) // <= { WasCompilerGenerated = true }; return receiver; }

Variabel penerima bisa nol . Penulis kode mengetahui hal ini karena ia menggunakan operator null bersyarat dalam kondisi blok if untuk mengakses receiver? .Sinktax . Lebih lanjut dalam kode, penerima variabel digunakan tanpa pemeriksaan untuk mengakses receiver. Jenis , penerima . Sintaks dan penerima . Memiliki Kesalahan . Kesalahan ini perlu diperbaiki:

 private BoundExpression GetReceiverForConditionalBinding( ExpressionSyntax binding, DiagnosticBag diagnostics) { .... BoundExpression receiver = this.ConditionalReceiverExpression; if (receiver?.Syntax != GetConditionalReceiverSyntax(conditionalAccessNode)) { receiver = BindConditionalAccessReceiver(conditionalAccessNode, diagnostics); } var receiverType = receiver?.Type; if (receiverType?.IsNullableType() == true) { .... } receiver = new BoundConditionalReceiver(receiver?.Syntax, 0, receiverType ?? CreateErrorType(), hasErrors: receiver?.HasErrors) { WasCompilerGenerated = true }; return receiver; }

Anda juga perlu memastikan bahwa konstruktor BoundConditionalReceiver mendukung mendapatkan nilai nol untuk parameternya, atau melakukan refactoring tambahan.

Kesalahan serupa lainnya:

  • V3042 Kemungkinan NullReferenceException. '?.' dan '.' operator digunakan untuk mengakses anggota objek 'mengandungType' SyntaxGeneratorExtensions_Negate.cs 240
  • V3042 Kemungkinan NullReferenceException. '?.' dan '.' operator digunakan untuk mengakses anggota objek 'ekspresi' ExpressionSyntaxExtensions.cs 349
  • V3042 Kemungkinan NullReferenceException. '?.' dan '.' operator digunakan untuk mengakses anggota objek 'ekspresi' ExpressionSyntaxExtensions.cs 349

Kesalahan dalam kondisi

V3057 Fungsi 'Substring' dapat menerima nilai '-1' sementara nilai non-negatif diharapkan. Periksa argumen kedua. CommonCommandLineParser.cs 109

 internal static bool TryParseOption(....) { .... if (colon >= 0) { name = arg.Substring(1, colon - 1); value = arg.Substring(colon + 1); } .... }

Jika variabel titik dua adalah 0, yang memungkinkan suatu kondisi dalam kode, metode Substring akan melempar ArgumentOutOfRangeException . Diperlukan koreksi:

 if (colon > 0)

Mungkin salah ketik

V3065 Parameter 't2' tidak digunakan di dalam tubuh metode. CSharpCodeGenerationHelpers.cs 84

 private static TypeDeclarationSyntax ReplaceUnterminatedConstructs(....) { .... var updatedToken = lastToken.ReplaceTrivia(lastToken.TrailingTrivia, (t1, t2) => { if (t1.Kind() == SyntaxKind.MultiLineCommentTrivia) { var text = t1.ToString(); .... } else if (t1.Kind() == SyntaxKind.SkippedTokensTrivia) { return ReplaceUnterminatedConstructs(t1); } return t1; }); .... }

Dua parameter dilewatkan ke ekspresi lambda: t1 dan t2. Namun, hanya t1 yang digunakan. Itu terlihat mencurigakan, mengingat betapa mudahnya melakukan kesalahan saat menggunakan variabel dengan nama-nama ini.

Kecerobohan

V3083 Doa yang tidak aman dari acara 'TagsChanged', NullReferenceException dimungkinkan. Pertimbangkan menugaskan acara ke variabel lokal sebelum menjalankannya. PreviewUpdater.Tagger.cs 37

 public void OnTextBufferChanged() { if (PreviewUpdater.SpanToShow != default) { if (TagsChanged != null) { var span = _textBuffer.CurrentSnapshot.GetFullSpan(); TagsChanged(this, new SnapshotSpanEventArgs(span)); // <= } } }

Acara TagsChanged dipicu tidak aman. Antara memeriksa kesetaraan nol dan memanggil suatu acara, mereka mungkin punya waktu untuk berhenti berlangganan darinya, maka pengecualian akan dilemparkan. Selain itu, di badan blok if , sesaat sebelum acara dipanggil, beberapa operasi lain dilakukan. Saya menyebut kesalahan ini "Ketidakpedulian", karena di tempat lain dalam kode mereka bekerja dengan acara ini lebih akurat, misalnya, seperti ini:

 private void OnTrackingSpansChanged(bool leafChanged) { var handler = TagsChanged; if (handler != null) { var snapshot = _buffer.CurrentSnapshot; handler(this, new SnapshotSpanEventArgs(snapshot.GetFullSpan())); } }

Menggunakan variabel handler opsional menghilangkan masalah. Dalam metode OnTextBufferChanged , Anda perlu mengedit untuk operasi aman yang sama dengan acara tersebut.

Rentang berpotongan

V3092 Range intersection dimungkinkan dalam ekspresi bersyarat. Contoh: if (A> 0 && A <5) {...} lain jika (A> 3 && A <9) {...}. ILBuilderEmit.cs 677

 internal void EmitLongConstant(long value) { if (value >= int.MinValue && value <= int.MaxValue) { .... } else if (value >= uint.MinValue && value <= uint.MaxValue) { .... } else { .... } }

Untuk pemahaman yang lebih baik, saya akan menulis ulang fragmen kode ini, mengganti nama konstan dengan nilai aktualnya:

 internal void EmitLongConstant(long value) { if (value >= -2147483648 && value <= 2147483648) { .... } else if (value >= 0 && value <= 4294967295) { .... } else { .... } }

Mungkin, tidak ada kesalahan nyata, tetapi kondisinya terlihat aneh. Bagian kedua ( jika tidak ) akan dilakukan hanya untuk rentang nilai dari 2147483648 + 1 hingga 4294967295.

Beberapa lagi dari peringatan ini:

  • V3092 Range intersection dimungkinkan dalam ekspresi bersyarat. Contoh: if (A> 0 && A <5) {...} lain jika (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 109
  • V3092 Range intersection dimungkinkan dalam ekspresi bersyarat. Contoh: if (A> 0 && A <5) {...} lain jika (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 66

Lebih lanjut tentang pemeriksaan kesetaraan nol (atau ketiadaan)

Beberapa kesalahan V3095 tentang memeriksa variabel untuk null setelah menggunakannya. Yang pertama ambigu, pertimbangkan kodenya.

V3095 Objek 'displayName' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 498, 503. FusionAssemblyIdentity.cs 498

 internal static IAssemblyName ToAssemblyNameObject(string displayName) { if (displayName.IndexOf('\0') >= 0) { return null; } Debug.Assert(displayName != null); .... }

Diasumsikan bahwa referensi displayName mungkin nol. Untuk melakukan ini, periksa Debug.Assert . Tidak jelas mengapa ia pergi setelah menggunakan string. Perlu juga dicatat bahwa untuk konfigurasi lain selain Debug, kompiler akan menghapus Debug. Masukkan dari kode sama sekali. Apakah ini berarti bahwa hanya untuk Debug dimungkinkan untuk mendapatkan referensi nol? Dan jika ini tidak demikian, mengapa tidak memeriksa string. IsNullOrEmpty (string) , misalnya. Ini adalah pertanyaan untuk pembuat kode.

Kesalahan berikut ini lebih jelas.

V3095 Objek 'scriptArgsOpt' digunakan sebelum diverifikasi terhadap null. Periksa baris: 321, 325. CommonCommandLineParser.cs 321

 internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt.Add(arg); // <= continue; } if (scriptArgsOpt != null) { .... } .... } }

Saya pikir kode ini tidak memerlukan penjelasan. Saya akan memberikan versi yang diperbaiki:

 internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt?.Add(arg); continue; } if (scriptArgsOpt != null) { .... } .... } }

Kode Roslyn menemukan 15 kesalahan lainnya:

  • V3095 Objek 'LocalFunctions' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 289, 317. ControlFlowGraphBuilder.RegionBuilder.cs 289
  • V3095 Objek 'resolution.OverloadResolutionResult' digunakan sebelum diverifikasi terhadap null. Periksa baris: 579, 588. Binder_Invocation.cs 579
  • V3095 Objek 'resolution.MethodGroup' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 592, 621. Binder_Invocation.cs 592
  • V3095 Objek 'touchFilesLogger' digunakan sebelum diverifikasi terhadap null. Periksa baris: 111, 126. CSharpCompiler.cs 111
  • V3095 Objek 'newExceptionRegionsOpt' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 736, 743. AbstrakEditAndLanjutkanAnalyzer.cs 736
  • V3095 Objek 'simbol' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 422, 427. AbstractGenerateConstructorService.Editor.cs 422
  • V3095 Objek '_state.BaseTypeOrInterfaceOpt' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 132, 140. AbstractGenerateTypeService.GenerateNamedType.cs 132
  • V3095 Objek 'elemen' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 232, 233. ProjectUtil.cs 232
  • V3095 Objek 'bahasa' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 22, 28. ExportCodeCleanupProvider.cs 22
  • V3095 Objek 'memberType' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 183, 184. SyntaxGeneratorExtensions_CreateGetHashCodeMethod.cs 183
  • V3095 Objek 'validTypeDeclarations' digunakan sebelum diverifikasi terhadap nol. Check lines: 223, 228. SyntaxTreeExtensions.cs 223
  • V3095 The 'text' object was used before it was verified against null. Check lines: 376, 385. MSBuildWorkspace.cs 376
  • V3095 The 'nameOrMemberAccessExpression' object was used before it was verified against null. Check lines: 206, 223. CSharpGenerateTypeService.cs 206
  • V3095 The 'simpleName' object was used before it was verified against null. Check lines: 83, 85. CSharpGenerateMethodService.cs 83
  • V3095 The 'option' object was used before it was verified against null. Check lines: 23, 28. OptionKey.cs 23

Pertimbangkan kesalahan V3105 . Di sini kita menggunakan operator null bersyarat saat menginisialisasi variabel, dan selanjutnya dalam kode variabel digunakan tanpa memeriksa kesetaraan nol .

Kesalahan berikutnya ditandai segera oleh dua peringatan.

V3105 Variabel 'documentId' digunakan setelah ditugaskan melalui operator kondisional nol. NullReferenceException dimungkinkan. CodeLensReferencesService.cs 138

V3105 Variabel 'documentId' digunakan setelah ditugaskan melalui operator kondisional nol. NullReferenceException dimungkinkan. CodeLensReferencesService.cs 139

 private static async Task<ReferenceLocationDescriptor> GetDescriptorOfEnclosingSymbolAsync(....) { .... var documentId = solution.GetDocument(location.SourceTree)?.Id; return new ReferenceLocationDescriptor( .... documentId.ProjectId.Id, documentId.Id, ....); }

DocumentId variabel dapat diinisialisasi ke nol . Akibatnya, pembuatan ReferenceLocationDescriptor akan berakhir dengan melemparkan pengecualian. Kode perlu diperbaiki:

 return new ReferenceLocationDescriptor( .... documentId?.ProjectId.Id, documentId?.Id, ....);

Selain itu, lebih lanjut dalam kode, perlu untuk memberikan kemungkinan kesetaraan variabel nol yang diteruskan ke konstruktor.

Kesalahan serupa lainnya dalam kode:

  • V3105 Variabel 'simbol' digunakan setelah ditugaskan melalui operator kondisional nol. NullReferenceException dimungkinkan. SymbolFinder_Hierarchy.cs 44
  • V3105 Variabel 'simbol' digunakan setelah ditugaskan melalui operator kondisional nol. NullReferenceException dimungkinkan. SymbolFinder_Hierarchy.cs 51

Prioritas dan tanda kurung

V3123 Mungkin operator '?:' Bekerja dengan cara yang berbeda dari yang diharapkan. Prioritasnya lebih rendah daripada prioritas operator lain dalam kondisinya. Edit.cs 70

 public bool Equals(Edit<TNode> other) { return _kind == other._kind && (_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode) && (_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode); }

Kondisi di blok kembali tidak dihitung sama sekali seperti yang dipikirkan pengembang. Diasumsikan bahwa kondisi pertama adalah _kind == other._kin d (oleh karena itu, pembungkus baris dilakukan setelah kondisi ini), dan kemudian blok kondisi dengan Operator " ? " Akan dihitung secara berurutan . Faktanya, kondisi pertama adalah _kind == other._kind && (_oldNode == null) . Ini karena operator && memiliki prioritas lebih tinggi daripada Operator " ? ". Untuk memperbaiki kesalahan, perlu tanda kurung semua ekspresi operator " ? ":

 return _kind == other._kind && ((_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode)) && ((_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode));

Ini menyimpulkan deskripsi kesalahan yang ditemukan.

Kesimpulan

Terlepas dari sejumlah besar kesalahan yang dapat saya deteksi, dalam hal ukuran kode proyek Roslyn (2.770.000 baris), ini akan menjadi jumlah yang sangat kecil. Seperti Andrei di artikel sebelumnya, saya juga siap untuk mengenali kualitas tinggi dari proyek ini.

Saya ingin mencatat bahwa pemeriksaan kode sesekali tidak ada hubungannya dengan metodologi analisis statis dan praktis tidak membawa manfaat apa pun. Analisis statis harus diterapkan secara teratur, dan bukan dari kasus ke kasus. Maka banyak kesalahan akan diperbaiki pada tahap paling awal, dan, oleh karena itu, biaya memperbaikinya akan sepuluh kali lebih rendah. Gagasan ini dijelaskan secara lebih rinci dalam artikel kecil ini , yang saya minta Anda untuk membiasakan diri.

Anda dapat secara independen mencari beberapa kesalahan lagi baik dalam proyek yang dipertimbangkan, dan yang lainnya. Untuk melakukan ini, Anda hanya perlu mengunduh dan mencoba penganalisa kami.



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Sergey Khrenov. Memeriksa Kode Sumber Roslyn .

Source: https://habr.com/ru/post/id446592/

More articles:


All Articles