Geekly articles weekly

Memeriksa Kode Sumber Roslyn

PVS-Studio vs Roslyn

Sekali-sekali kita kembali ke proyek yang sebelumnya telah kita periksa menggunakan PVS-Studio, yang menghasilkan deskripsi mereka di berbagai artikel. Dua alasan membuat comeback ini menarik bagi kami. Pertama, kesempatan untuk menilai kemajuan alat analisis kami. Kedua, memantau umpan balik dari penulis proyek untuk artikel kami dan laporan kesalahan, yang biasanya kami berikan kepada mereka. Tentu saja, kesalahan dapat diperbaiki tanpa partisipasi kami. Namun, selalu menyenangkan ketika upaya kami membantu membuat proyek lebih baik. Roslyn tidak terkecuali. Artikel sebelumnya tentang pemeriksaan proyek ini tanggal kembali ke 23 Desember 2015. Ini cukup lama, mengingat kemajuan yang telah dibuat oleh analis kami sejak saat itu. Karena inti C # dari analisa PVS-Studio didasarkan pada Roslyn, itu memberi kami minat tambahan dalam proyek ini. Sebagai hasilnya, kami sama tertarik dengan mustard tentang kualitas kode proyek ini. Sekarang mari kita coba sekali lagi dan cari tahu beberapa masalah baru dan menarik (tapi mari kita berharap tidak ada yang signifikan) yang dapat ditemukan oleh PVS-Studio.

Banyak pembaca kami mungkin sangat menyadari Roslyn (atau .NET Compiler Platform). Singkatnya, ini adalah kumpulan kompiler open source dan API untuk analisis kode bahasa C # dan Visual Basic .NET dari Microsoft. Kode sumber proyek tersedia di GitHub .

Saya tidak akan memberikan penjelasan rinci tentang platform ini dan saya akan merekomendasikan memeriksa artikel oleh rekan saya Sergey Vasiliev " Pengantar Roslyn dan penggunaannya dalam pengembangan program " untuk semua pembaca yang tertarik. Dari artikel ini, Anda dapat mengetahui tidak hanya tentang fitur arsitektur Roslyn, tetapi bagaimana tepatnya kami menggunakan platform ini.

Seperti yang saya sebutkan sebelumnya, sudah lebih dari 3 tahun sejak kolega saya Andrey Karpov menulis artikel terakhir tentang cek Roslyn " New Year PVS-Studio 6.00 Release: Scanning Roslyn ". Sejak itu, penganalisa C # PVS-Studio telah mendapatkan banyak fitur baru. Sebenarnya, artikel Andrey adalah uji kasus, karena pada saat itu penganalisa C # baru saja ditambahkan di PVS-Studio. Meskipun demikian, kami berhasil mendeteksi kesalahan dalam proyek Roslyn, yang tentu saja berkualitas tinggi. Jadi apa yang telah berubah dalam alat analisis untuk kode C # pada saat ini yang memungkinkan kami melakukan analisis yang lebih mendalam?

Sejak itu, inti dan infrastruktur telah berkembang. Kami menambahkan dukungan untuk Visual Studio 2017 dan Roslyn 2.0, dan integrasi mendalam dengan MSBuild. Artikel oleh rekan saya Paul Eremeev " Dukungan Visual Studio 2017 dan Roslyn 2.0 di PVS-Studio: kadang-kadang itu tidak mudah untuk menggunakan solusi yang sudah jadi seperti yang tampak " menggambarkan pendekatan kami untuk integrasi dengan MSBuild dan alasan untuk keputusan ini.

Saat ini kami secara aktif bekerja untuk pindah ke Roslyn 3.0 dengan cara yang sama seperti kami awalnya mendukung Visual Studio 2017. Ini membutuhkan penggunaan toolset kami sendiri, termasuk dalam distribusi PVS-Studio sebagai "rintisan", yang merupakan MSBuild kosong File .exe. Terlepas dari kenyataan bahwa itu terlihat seperti "penopang" (MSBuild API tidak terlalu ramah untuk digunakan kembali dalam proyek-proyek pihak ketiga karena portabilitas perpustakaan rendah), pendekatan semacam itu telah membantu kami untuk secara relatif mulus mengatasi beberapa pembaruan Roslyn dalam hal Visual Studio 2017. Sampai sekarang ini membantu (bahkan dengan beberapa tantangan) untuk melewati pembaruan Visual Studio 2019 dan mempertahankan kompatibilitas mundur penuh dan kinerja untuk sistem dengan versi MSBuild yang lebih lama.

Inti penganalisa juga telah mengalami sejumlah perbaikan. Salah satu fitur utama adalah analisis interprocedural lengkap dengan pertimbangan nilai-nilai metode input dan output, mengevaluasi (tergantung pada parameter ini) jangkauan cabang eksekusi dan titik pengembalian.

Kami sedang dalam perjalanan untuk menyelesaikan tugas pemantauan parameter di dalam metode (misalnya, dereferensi yang berpotensi berbahaya) bersama dengan menyimpan anotasi otomatis mereka. Untuk diagnostik yang menggunakan mekanisme aliran data, ini akan memungkinkan mempertimbangkan situasi berbahaya, terjadi ketika melewati parameter dalam suatu metode. Sebelum ini, ketika menganalisis tempat berbahaya seperti itu, peringatan tidak dibuat, karena kami tidak bisa tahu tentang semua nilai input yang mungkin dalam metode seperti itu. Sekarang kita dapat mendeteksi bahaya, karena di semua tempat memanggil metode ini, parameter input ini akan diperhitungkan.

Catatan: Anda dapat membaca tentang mekanisme analisa dasar, seperti aliran data dan lainnya dalam artikel " Teknologi yang digunakan dalam penganalisa kode PVS-Studio untuk menemukan bug dan potensi kerentanan ".

Analisis antar-prosedur dalam PVS-Studio C # tidak dibatasi oleh parameter input, maupun kedalaman. Satu-satunya batasan adalah metode virtual di kelas, terbuka untuk warisan, serta masuk ke rekursi (analisis berhenti ketika tersandung panggilan berulang metode yang sudah dievaluasi). Dengan demikian, metode rekursif itu sendiri pada akhirnya akan dievaluasi dengan asumsi bahwa nilai pengembalian rekursi tidak diketahui.

Fitur baru yang hebat lainnya dalam penganalisis C # telah mempertimbangkan kemungkinan dereferensi penunjuk yang berpotensi nol. Sebelum itu, penganalisa mengeluh tentang kemungkinan pengecualian referensi nol, dipastikan bahwa di semua cabang eksekusi nilai variabel akan menjadi nol. Tentu saja, itu salah dalam beberapa kasus, itu sebabnya diagnostik V3080 sebelumnya disebut referensi nol potensial.

Sekarang penganalisa menyadari fakta bahwa variabel bisa nol di salah satu cabang eksekusi (misalnya, di bawah kondisi if tertentu). Jika ia melihat akses ke variabel seperti itu tanpa centang, itu akan mengeluarkan peringatan V3080, tetapi pada tingkat kepastian yang lebih rendah, daripada jika ia melihat nol di semua cabang. Seiring dengan peningkatan analisis antarproedural, mekanisme semacam itu memungkinkan menemukan kesalahan yang sangat sulit dideteksi. Berikut ini sebuah contoh - bayangkan rantai panjang pemanggilan metode, yang terakhir tidak Anda kenal. Dalam keadaan tertentu, ia mengembalikan nol di blok tangkap , tetapi Anda belum terlindungi dari ini, karena Anda belum tahu. Dalam hal ini, penganalisa hanya mengeluh, ketika ia benar-benar melihat penugasan nol. Dalam pandangan kami, ini secara kualitatif membedakan pendekatan kami dari fitur C # 8.0 sebagai referensi jenis yang dapat dibatalkan, yang, pada kenyataannya, terbatas pada pengaturan pemeriksaan untuk null untuk setiap metode. Namun, kami menyarankan alternatif - untuk melakukan pemeriksaan hanya di tempat-tempat di mana null benar-benar dapat terjadi, dan penganalisa kami sekarang dapat mencari kasus-kasus tersebut.

Jadi, jangan menunda poin utama terlalu lama dan menyalahkan orang lain - menganalisis hasil pemeriksaan Roslyn. Pertama, mari kita pertimbangkan kesalahan yang ditemukan karena fitur-fitur yang dijelaskan di atas. Singkatnya, ada cukup banyak peringatan untuk kode Roslyn kali ini. Saya pikir ini terkait dengan fakta bahwa platform sangat aktif berkembang (pada titik ini basis kode sekitar 2.770.000 baris tidak termasuk kosong), dan kami belum menganalisis proyek ini lama. Namun demikian, tidak ada begitu banyak kesalahan kritis, sedangkan mereka yang paling menarik untuk artikel ini. Seperti biasa, saya mengecualikan tes dari cek, ada cukup banyak di Roslyn.

Saya akan mulai dengan kesalahan V3080 tingkat menengah kepastian, di mana penganalisa telah mendeteksi kemungkinan akses dengan referensi nol, tetapi tidak dalam semua kasus yang mungkin (kode cabang).

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; }

Mari kita pertimbangkan metode GetNode . Alat analisis menyarankan akses dengan referensi nol dimungkinkan dalam kondisi blok sementara . Variabel diberikan nilai dalam tubuh blok sementara , yang merupakan hasil dari metode AsNode . Dalam beberapa kasus, nilai ini akan menjadi nol . Contoh yang baik dari analisis antar prosedur dalam aksi.

Sekarang mari kita 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, mengembalikan hasil metode GetDirectoryName kelebihan (string, bool) yang nilainya bisa nol . Karena direktori variabel digunakan tanpa pemeriksaan pendahuluan untuk null dalam tubuh metode ExpandFileNamePattern - kita dapat menyatakan penganalisa dengan benar tentang mengeluarkan peringatan. Ini adalah konstruksi yang berpotensi tidak aman.

Fragmen kode lain dengan kesalahan V3080, lebih tepatnya, dengan dua kesalahan, dikeluarkan untuk satu baris kode. Analisis antar-prosedur tidak diperlukan di sini.

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 dari tipe int nullable dan diinisialisasi dengan nol . Lebih lanjut di sepanjang kode mereka dapat diberi nilai, tetapi hanya dalam kondisi tertentu. Dalam beberapa kasus, nilainya tetap nol . Setelah itu, variabel-variabel ini diakses dengan referensi tanpa pemeriksaan awal untuk null, yang menunjukkan penganalisa.

Kode Roslyn mengandung cukup banyak kesalahan seperti itu, lebih dari 100. Seringkali pola kesalahan ini sama. Ada beberapa jenis metode umum, yang berpotensi mengembalikan nol . 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 dengan referensi nol. Meskipun mendeteksi kesalahan seperti itu cukup menantang. Itu sebabnya dalam beberapa kasus seseorang harus mempertimbangkan kode refactoring, dalam hal ini jika null kembali, metode ini akan mengeluarkan pengecualian. Jika tidak, Anda dapat mengamankan kode Anda hanya dengan pemeriksaan umum yang cukup melelahkan dan terkadang tidak dapat diandalkan. Bagaimanapun, jelas bahwa setiap kasus spesifik memerlukan solusi berdasarkan spesifikasi proyek.

Catatan Kebetulan, bahwa pada titik tertentu tidak ada kasus seperti itu (input data), ketika metode mengembalikan nol dan tidak ada kesalahan aktual. Namun, kode semacam itu masih tidak dapat diandalkan, karena semuanya dapat berubah ketika memperkenalkan beberapa perubahan kode.

Untuk menjatuhkan subjek V3080 , mari kita lihat kesalahan yang jelas dari tingkat kepastian tinggi, ketika akses dengan referensi nol adalah yang paling mungkin atau bahkan tak 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 ( && digunakan sebagai ganti operator || ), kode bekerja secara berbeda dari yang dimaksudkan dan akses ke variabel collectionType.Type akan dieksekusi ketika itu nol . Kondisi tersebut harus diperbaiki sebagai berikut:

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

Ngomong-ngomong, hal-hal dapat terungkap dengan cara lain: di bagian pertama kondisi operator == dan ! = Kacau . Maka kode yang benar akan terlihat seperti ini:

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

Versi kode ini kurang logis, tetapi juga memperbaiki kesalahan. Solusi akhir ada pada penulis proyek untuk memutuskan.

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 membuat pesan untuk pengecualian. Ini diikuti oleh upaya untuk mengakses properti action.DisplayText melalui variabel tindakan , yang dikenal sebagai nol .

Di sinilah kesalahan V3080 terakhir dari tingkat Tinggi.

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; }

Metodenya cukup kecil, jadi saya kutip sepenuhnya. Kondisi di blok kembali salah. Dalam beberapa kasus, ketika mengakses type.FullName , pengecualian dapat terjadi. Saya akan menggunakan tanda kurung untuk memperjelas (mereka tidak akan mengubah perilaku):

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

Menurut operasi yang diutamakan, kode akan bekerja persis seperti ini. Jika variabel tipe adalah nol , kita akan jatuh ke cek lain, di mana kita akan menggunakan referensi tipe nol, setelah memeriksa variabel targetTypeName untuk null . Kode mungkin diperbaiki, misalnya, sebagai berikut:

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

Saya pikir, itu sudah cukup untuk meninjau kesalahan V3080. Sekarang saatnya untuk melihat hal-hal menarik lainnya yang berhasil ditemukan oleh alat analisa PVS-Studio.

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 kegagalan penamaan variabel, kesalahan ketik dibuat di konstruktor kelas DynamicFileInfo . Bidang SourceCodeKind diberi nilainya sendiri alih-alih menggunakan parameter sourceCodeKind . Untuk meminimalkan kemungkinan kesalahan tersebut, kami sarankan Anda menggunakan awalan garis bawah ke nama parameter dalam kasus tersebut. Berikut adalah contoh versi kode yang diperbaiki:

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

Ketidaksengajaan

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, destructor harus melempar eksepsi, tetapi itu tidak terjadi ketika objek eksepsi hanya dibuat. Kata kunci lemparan tidak terjawab. Ini adalah versi kode yang diperbaiki:

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

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

Ketika hasilnya tidak penting

Metode, yang menerima nilai yang sama dalam semua kasus, memicu sejumlah peringatan V3009 . Dalam beberapa kasus bisa jadi tidak kritis atau nilai kembaliannya tidak dicentang dalam kode panggilan. Saya melewatkan peringatan seperti itu. Tetapi beberapa cuplikan kode tampak mencurigakan. Ini salah satunya:

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 mengembalikan apa pun kecuali benar . Dalam melakukannya, dalam kode panggilan nilai yang dikembalikan terlibat dalam beberapa pemeriksaan.

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

Sulit untuk mengatakan dengan tepat sejauh mana perilaku seperti itu berbahaya. Tetapi jika hasilnya tidak diperlukan, mungkin jenis nilai kembali harus diubah menjadi batal dan orang harus mengedit kecil dalam metode panggilan. Ini akan membuat kode lebih mudah dibaca dan aman.

Peringatan penganalisa serupa:

  • 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

Memeriksa hal yang salah

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(); } .... }

Variabel nilai dilemparkan ke tipe NamingStylePreferences . Masalahnya ada di cek yang mengikuti ini. Bahkan jika variabel nilai bukan nol, itu tidak menjamin bahwa casting tipe berhasil dan valueToSerialize tidak mengandung nol . Kemungkinan melempar pengecualian NullReferenceException . Kode perlu diperbaiki sebagai berikut:

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

Bug serupa lainnya:

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 dilemparkan ke jenis ColumnState2 . Namun, hasil operasi, yang merupakan variabel columnState2, tidak diperiksa untuk nol lebih lanjut. Sebagai gantinya, variabel columnState diperiksa menggunakan operator null bersyarat. Mengapa kode ini berbahaya? Sama seperti pada contoh sebelumnya, casting dengan operator as mungkin gagal dan variabel akan menjadi nol yang akan menghasilkan pengecualian. Ngomong-ngomong, kesalahan ketik bisa disalahkan di sini. Lihatlah kondisi di blok if .

Mungkin, bukannya columnState? .Nama penulis ingin menulis columnState2? .Name . Sangat mungkin, mengingat nama variabel yang agak salah, columnState dan columnState2.

Cek berlebihan

Cukup banyak peringatan (lebih dari 100) dikeluarkan pada konstruksi yang tidak kritis, tetapi berpotensi tidak aman terkait dengan cek yang berlebihan. Misalnya, ini 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 bug yang sebenarnya di sini. Itu hanya alasan yang bagus untuk menunjukkan "analisis antar-proses + analisis aliran data" yang bekerja dengan baik. Alat analisis menyarankan cek kedua navInfo == null berlebihan. Memang, sebelum itu nilai yang ditetapkan untuk navInfo akan diperoleh dari metode libraryService.NavInfoFactory.CreateForProject , yang akan membangun dan mengembalikan objek baru dari kelas NavInfo . Tidak mungkin itu akan mengembalikan nol . Di sini muncul pertanyaan, mengapa analis tidak mengeluarkan peringatan untuk cek pertama navInfo == null ? Ada beberapa alasan. Pertama, jika variabel simbol nol , nilai navInfo akan tetap menjadi referensi nol juga. Kedua, bahkan jika navInfo mendapatkan nilai dari metode ibraryService.NavInfoFactory.CreateForSymbol , nilai ini juga bisa menjadi nol . Jadi, pemeriksaan pertama navInfo == null benar-benar diperlukan.

Cek tidak mencukupi

Sekarang situasi sebaliknya dari yang dibahas di atas. Beberapa peringatan V3042 dipicu untuk kode, di mana akses dengan referensi nol dimungkinkan. Bahkan satu atau dua cek kecil bisa memperbaiki semuanya.

Mari kita pertimbangkan fragmen kode lain yang menarik, yang memiliki 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 mungkin nol. Penulis kode mengetahui hal ini, karena ia menggunakan operator null bersyarat dalam kondisi blok if untuk mengakses receiver ? . Sintaks . Selanjutnya variabel penerima digunakan tanpa pemeriksaan untuk mengakses receiver. Jenis , penerima . Sintaks dan penerima . Memiliki Kesalahan . Kesalahan ini harus 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; }

Kita juga harus memastikan bahwa konstruktor mendukung mendapatkan nilai nol untuk parameternya atau kita perlu 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); } .... }

Dalam hal jika variabel usus besar adalah 0, yang baik-baik saja sesuai dengan kondisi dalam kode, metode Substring akan melempar pengecualian. Ini harus diperbaiki:

 if (colon > 0)

Kemungkinan 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; }); .... }

Ekspresi lambda menerima dua parameter: t1 dan t2. Namun, hanya t1 yang digunakan. Itu terlihat mencurigakan, dengan mempertimbangkan fakta betapa mudahnya melakukan kesalahan ketika menggunakan variabel dengan nama-nama tersebut.

Ketidaksengajaan

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 dipanggil dengan cara yang tidak aman. Antara memeriksa nol dan menjalankan acara, seseorang dapat berhenti berlangganan darinya, maka pengecualian akan dilemparkan. Selanjutnya, operasi lain dilakukan di tubuh blok if tepat sebelum memanggil acara. Saya menyebut kesalahan ini "Kelalaian", karena acara ini ditangani lebih hati-hati di tempat lain, sebagai berikut:

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

Penggunaan variabel penangan tambahan mencegah masalah. Dalam metode OnTextBufferChanged, kita harus mengedit agar dapat menangani acara dengan aman.

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, izinkan saya menulis ulang kode ini, mengubah nama konstanta 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 ( selain itu ) akan dieksekusi hanya untuk rentang dari 2147483648 +1 untuk 4294967295.

Beberapa peringatan serupa lainnya:

  • 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 cek nol (atau kurang dari itu)

Beberapa kesalahan V3095 pada pemeriksaan variabel untuk null tepat setelah penggunaannya. Yang pertama ambigu, mari kita 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 displayName referensi bisa nol. Untuk ini, cek Debug.Assert dilakukan. Tidak jelas mengapa ia pergi setelah menggunakan string. Ini juga harus diperhitungkan bahwa untuk konfigurasi yang berbeda dari Debug, kompiler akan menghapus Debug . Masukkan sama sekali . Apakah itu berarti mendapatkan referensi nol hanya mungkin untuk Debug? Jika tidak demikian, mengapa penulis melakukan pemeriksaan string. IsNullOrEmpty (string) , misalnya. Ini adalah pertanyaan untuk pembuat kode.

Kesalahan berikut 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 apa pun. Biarkan saya memberi Anda versi tetap:

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

Dalam kode Roslyn, ada 15 kesalahan serupa 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. Periksa baris: 223, 228. SyntaxTreeExtensions.cs 223
  • V3095 Objek 'teks' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 376, 385. MSBuildWorkspace.cs 376
  • V3095 Objek 'nameOrMemberAccessExpression' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 206, 223. CSharpGenerateTypeService.cs 206
  • V3095 Objek 'simpleName' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 83, 85. CSharpGenerateMethodService.cs 83
  • V3095 Objek 'opsi' digunakan sebelum diverifikasi terhadap nol. Periksa baris: 23, 28. OptionKey.cs 23

Mari kita pertimbangkan kesalahan V3105 . Di sini operator nol bersyarat digunakan ketika menginisialisasi variabel, tetapi selanjutnya variabel digunakan tanpa memeriksa nol .

Dua peringatan menunjukkan kesalahan berikut:

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, ....); }

Variabel documentId dapat diinisialisasi dengan nol . Akibatnya, membuat objek ReferenceLocationDescriptor akan menghasilkan melempar pengecualian. Kode harus diperbaiki:

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

Pengembang juga harus mencakup kemungkinan variabel, diteruskan ke konstruktor, menjadi nol.

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 dievaluasi bukan seperti yang dimaksudkan pengembang. Diasumsikan bahwa kondisi pertama adalah _kind == other._kin d, (ini sebabnya setelah kondisi ini ada jeda baris), dan setelah itu blok kondisi dengan operator " ? " Akan dievaluasi secara berurutan. Faktanya, kondisi pertama adalah _kind == other._kind && (_oldNode == null) . Ini disebabkan oleh fakta bahwa operator && memiliki prioritas lebih tinggi daripada operator " ? ". Untuk memperbaikinya, pengembang harus mengambil semua ekspresi operator " ? " Dalam tanda kurung:

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

Itu menyimpulkan deskripsi saya tentang kesalahan yang ditemukan.

Kesimpulan

Terlepas dari sejumlah besar kesalahan, yang saya berhasil temukan, dalam hal ukuran kode proyek Roslyn (2.770.000 baris), itu tidak terlalu banyak. Seperti yang ditulis Andrey dalam artikel sebelumnya, saya juga siap untuk mengakui kualitas tinggi dari proyek ini.

Saya ingin mencatat bahwa pemeriksaan kode sesekali tidak ada hubungannya dengan metodologi analisis statis dan hampir tidak membantu. Analisis statis harus diterapkan secara teratur, dan tidak berdasarkan kasus per kasus. Dengan cara ini, banyak kesalahan akan diperbaiki pada tahap paling awal, dan karenanya biaya untuk memperbaikinya akan sepuluh kali lebih sedikit. Gagasan ini dituangkan secara lebih rinci dalam catatan kecil ini, silakan, periksa.

Anda dapat memeriksa sendiri beberapa kesalahan di proyek ini dan di yang lain. Untuk melakukan ini, Anda hanya perlu mengunduh dan mencoba penganalisa kami.

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

More articles:


All Articles