TOP 10 kesalahan ditemukan dalam proyek C # pada tahun 2020

gambar1.png


Akhirnya, tahun 2020 yang sulit akan segera berakhir, yang berarti sudah waktunya untuk mempertimbangkan! Selama tahun ini, tim PVS-Studio telah menulis banyak artikel yang membahas berbagai kesalahan yang ditemukan oleh penganalisis dalam proyek sumber terbuka. Anda dapat melihat yang paling menarik dari mereka di sini, dalam kesalahan TOP yang ditemukan di proyek C # pada tahun 2020. Selamat melihat!



Bagaimana bagian atas dibentuk



Daftar ini berisi pemicu paling menarik, menurut saya, yang saya dan kolega tulis di artikel untuk tahun 2020. Kriteria pemilihan yang penting adalah tingkat keyakinan bahwa kesalahan memang telah dibuat dalam fragmen kode terkait. Dan, tentu saja, ketika memilih, serta menempatkan tempat, 'ketertarikan' pemicuan diperhitungkan, tetapi ini adalah pendapat subjektif saya - Anda selalu dapat menantangnya di komentar.



Saya mencoba membuat yang teratas menjadi beragam: baik dalam hal pesan PVS-Studio, dan dalam hal proyek yang peringatan kode dikeluarkan. Daftar tersebut mencakup deteksi pada sumber dari 8 proyek terverifikasi. Pada saat yang sama, aturan diagnostik hampir tidak pernah diulang - Anda dapat bertemu dua kali di sini hanya V3022 dan V3106 (dan tidak, itu bukan buatan saya, tapi ternyata ini adalah favorit saya). Jadi, variasi dijamin di sini :).



Baiklah, mari kita mulai! 10 besar!



Tempat 10 - Lisensi lama baru



Tanggapan teratas kami terbuka dari sebuah artikel oleh satu orang yang sangat baik tentang memeriksa proyek C # untuk Linux dan macOS, di mana proyek RavenDB digunakan sebagai contoh:



private static void UpdateEnvironmentVariableLicenseString(....)
{
  ....
  if (ValidateLicense(newLicense, rsaParameters, oldLicense) == false)
    return;
  ....
}
      
      





Peringatan Penganalisis : V3066 Kemungkinan urutan argumen salah diteruskan ke metode 'ValidateLicense': 'newLicense' dan 'oldLicense'. LicenseHelper.cs (177) Raven.Server



Tampaknya, ada apa di sini? Kode dikompilasi dengan cukup baik. Mengapa penganalisis memutuskan bahwa perlu mentransfer dahuluLicense lama , lalu baruLicense ? Anda dapat menebaknya, bukan? Mari kita lihat deklarasi ValidateLicense :



private static bool ValidateLicense(License oldLicense, 
                                    RSAParameters rsaParameters, 
                                    License newLicense)
      
      





Wow, itu benar - pertama yang lama masuk ke parameter, dan baru kemudian lisensi baru. Katakan padaku, dapatkah analisis dinamismu menangkap ini? :)



Bagaimanapun, pemicuannya menarik. Mungkin urutannya tidak terlalu penting di sana, tetapi lebih baik periksa ulang fragmen seperti itu, setuju?



Tempat ke-9 - 'FirstOrDefault' dan 'null' yang tidak terduga



Di urutan ke-9 adalah respon dari artikel " Mainkan di" osu! ", Jangan lupakan kesalahan ", yang ditulis di awal tahun berjalan:



public ScoreInfo CreateScoreInfo(RulesetStore rulesets)
{
  var ruleset = rulesets.GetRuleset(OnlineRulesetID);

  var mods = Mods != null ? ruleset.CreateInstance() 
                                   .GetAllMods().Where(....)
                                   .ToArray() : Array.Empty<Mod>();
  ....
}
      
      





Lihat kesalahannya? Dan dia adalah! Apa yang dikatakan penganalisis?



Peringatan Analyzer : V3146 [CWE-476] Kemungkinan dereferensi null dari 'ruleset'. 'FirstOrDefault' dapat mengembalikan nilai nol default. APILegacyScoreInfo.cs 24



Ya, ya, sekali lagi saya tidak memberikan semua informasi yang diperlukan sekaligus. Faktanya, Anda benar-benar tidak dapat melihat sesuatu yang mencurigakan dalam kode ini. Lagipula, FirstOrDefault , yang diberitahukan oleh penganalisis, ada dalam definisi metode GetRuleset :



public RulesetInfo GetRuleset(int id) => 
  AvailableRulesets.FirstOrDefault(....);
      
      





Bisnis yang buruk! Metode ini akan mengembalikan RulesetInfo jika menemukan yang cocok. Dan jika tidak? Dengan tenang mengembalikan nol . Dan itu akan menembak sudah di tempat di mana hasil panggilan akan digunakan. Dalam kasus ini, saat memanggil ruleset.CreateInstance () .



Mungkin timbul pertanyaan: bagaimana jika tidak pernah ada nol ? Bagaimana jika koleksi selalu berisi elemen yang dibutuhkan? Nah, jika pengembang yakin tentang ini, mengapa tidak menggunakan First, bukan FirstOrDefault ?



Tempat 8 - Halo dari Python



Pemicu terakhir dari tiga yang pertama dikeluarkan untuk kode proyek RunUO. Artikel tentang cara memeriksanya ditulis pada bulan Februari dan tersedia di sini .



Fragmen yang ditemukan sangat mencurigakan, meskipun sulit untuk memastikan apakah itu adalah kesalahan:



public override void OnCast()
{
  if ( Core.AOS )
  {
    damage = m.Hits / 2;

    if ( !m.Player )
      damage = Math.Max( Math.Min( damage, 100 ), 15 );
      damage += Utility.RandomMinMax( 0, 15 );
  }
  else { .... }
}
      
      





Analyzer Peringatan : V3043 Logika operasional kode tidak sesuai dengan formatnya. Pernyataan itu menjorok ke kanan, tetapi selalu dieksekusi. Ada kemungkinan tanda kurung kurawal tidak ada. Earthquake.cs 57



Itu benar - ini tentang indentasi! Tampaknya garis kerusakan + = Utility.RandomMinMax (0, 15) seharusnya dieksekusi hanya jika m.Player adalah palsu... Kode Python akan bekerja dengan cara yang sama, di mana lekukan ditulis tidak hanya untuk kecantikan, tetapi juga untuk mendefinisikan logika aplikasi. Tetapi kompilator C # memiliki pendapat berbeda tentang masalah ini! Saya ingin tahu apa pendapat developernya?



Secara umum, dalam situasi ini ada 2 pilihan. Baik kawat gigi keriting benar-benar hilang di sini, dan semuanya berfungsi dengan tidak benar, atau semuanya berfungsi dengan benar, tetapi seiring waktu, pasti akan ada orang yang akan menganggap ini sebagai kesalahan dan "memperbaikinya".



Mungkin saya salah, dan memang ada kalanya tidak masalah menulis sesuatu seperti itu. Jika Anda mengetahui kasus-kasus seperti itu, silakan tulis komentar - saya akan sangat tertarik untuk mempelajari kasus-kasus seperti itu.



Tempat ke-7 - Sempurna atau Sempurna - itulah pertanyaannya!



Semakin sulit mendistribusikan peringatan di beberapa tempat, dan kita beralih ke alarm kedua dalam daftar ini dari artikel tentang osu! ...



Berapa lama waktu yang Anda butuhkan untuk melihat kesalahan tersebut?



protected override void CheckForResult(....)
{
  ....
  ApplyResult(r =>
  {
    if (   holdNote.hasBroken
        && (result == HitResult.Perfect || result == HitResult.Perfect))
      result = HitResult.Good;
    ....
  });
}
      
      





Peringatan Analyzer : V3001 Ada hasil sub-ekspresi identik '== HitResult.Perfect' di kiri dan kanan '||' operator. DrawableHoldNote.cs 266 Tidak



banyak, menurut saya, Anda hanya perlu membaca peringatan PVS-Studio. Pengembang yang menggunakan analisis statis biasanya melakukan ini :). Orang bisa berdebat tentang tempat sebelumnya di atas, tetapi di sini kesalahannya jelas. Sulit untuk mengatakan elemen tertentu dari pencacahan HitResult mana yang seharusnya digunakan di sini daripada yang kedua Sempurna (baik, atau bukan yang pertama), tetapi pada akhirnya ada sesuatu yang salah ditulis dengan jelas. Tidak apa-apa - kesalahan telah ditemukan, yang berarti dapat diperbaiki dengan mudah.



Tempat ke-6 - lulus nol (tidak)!



Tempat ke-6 adalah respons yang sangat keren terhadap kode dari proyek Open XML SDK. Anda dapat membaca artikel tentang memeriksanya di sini .



Pengembang ingin mengimplementasikan properti yang tidak akan mengembalikan null , meskipun ditetapkan secara langsung. Dan itu sangat bagus ketika Anda dapat yakin bahwa nol tidak akan ditulis dalam keadaan apa pun. Sayangnya ini bukan situasinya sama sekali:



internal string RawOuterXml
{
  get => _rawOuterXml;

  set
  {
    if (string.IsNullOrEmpty(value))
    {
      _rawOuterXml = string.Empty;
    }

    _rawOuterXml = value;
  }
}
      
      





Peringatan Parser : V3008 Variabel '_rawOuterXml' diberi nilai dua kali berturut-turut. Mungkin ini salah. Baris centang: 164, 161. OpenXmlElement.cs 164



Ternyata di _rawOuterXml akan terekam nilai terlepas dari apakah null atau tidak. Jika Anda melihat kode ini dengan lalai, Anda mungkin berpikir bahwa null tidak akan pernah ditulis ke properti ini - lagipula, ada pemeriksaan! Nah, jika demikian, maka di bawah pohon Anda tidak dapat menemukan hadiah, tetapi NullReferenceException yang tidak terduga :(



Tempat ke-5 - Dibidik dari larik dengan larik di dalamnya



5 pemicu teratas tahun 2020 dikeluarkan oleh penganalisis untuk proyek TensorFlow.NET yang saya uji secara pribadi. Dengan mengklik tautan , Anda dapat membaca artikel tentang memeriksa proyek ini (oh, dan saya sering melihat semua orang di sana).



Ngomong-ngomong, jika Anda suka melihat contoh kesalahan menarik dari proyek C # nyata, saya sarankan Anda berlangganan ke twitter saya . Di sana saya berencana untuk memposting pemicu penasaran dan fragmen kode yang hanya menarik, banyak di antaranya, sayangnya, tidak akan dimasukkan dalam artikel. Saya akan senang melihat Anda! :)



Nah, sekarang mari kita lanjutkan ke pemicuan:



public TensorShape(int[][] dims)
{
  if(dims.Length == 1)
  {
    switch (dims[0].Length)
    {
      case 0: shape = new Shape(new int[0]); break;
      case 1: shape = Shape.Vector((int)dims[0][0]); break;
      case 2: shape = Shape.Matrix(dims[0][0], dims[1][2]); break; // <=
      default: shape = new Shape(dims[0]); break;
    }
  }
  else
  {
    throw new NotImplementedException("TensorShape int[][] dims");
  }
}
      
      





Peringatan penganalisis : V3106 Mungkin indeks di luar batas. Indeks '1' mengarah ke luar batas 'redup'. TensorShape.cs 107



Faktanya, sangat sulit untuk memilih di mana harus meletakkan pemicu ini, karena ini sangat menarik, tetapi yang lain tidak jauh ketinggalan dalam hal ini. Jadi, mari kita cari tahu apa yang terjadi di sini.



Jika jumlah array dalam redup tidak sama dengan 1, maka pengecualian tipe NotImplementedException akan ditampilkan . Dan apa yang akan terjadi jika reduptepatnya satu larik? Jumlah elemen dalam 'larik dalam larik' ini dicentang. Perhatikan apa yang terjadi jika 2. Sebagai salah satu argumen untuk konstruktor Shape.Matrix , secara tidak terduga, dims [1] [2] dilewatkan . Sekarang mari kita ingat - berapa banyak elemen yang ada di array dims ?



Benar, tepat satu - kami baru saja memeriksanya! Upaya untuk mendapatkan elemen kedua dari larik, yang hanya berisi satu elemen, akan memunculkan pengecualian IndexOutOfRangeException . Jelas sebuah kesalahan. Tetapi apakah ada cara yang jelas untuk memperbaikinya?



Hal pertama yang terlintas dalam pikiran adalah perubahan meredupkan [1] [2] menjadi meredup [0] [2] . Akankah kesalahan itu hilang? Tidak peduli bagaimana itu! Akan ada pengecualian yang sama lagi. Tapi kali ini akan dihubungkan dengan fakta bahwa dalam kasus ini-cabang jumlah elemen di dims [0] sama dengan 2. Apakah pengembang membuat dua kesalahan dalam indeks berturut-turut? Atau haruskah variabel lain digunakan di sana? Siapa tahu ... Tugas penganalisis adalah menemukan kesalahan, dan orang yang membuatnya atau rekannya harus memperbaikinya.



Tempat keempat - Properti suatu objek yang tidak ada



Pemicu lain mengenai bagian atas ini dari artikel tentang pemeriksaan OpenRA . Mungkin layak mendapatkan lebih, tetapi atas kehendak takdir, pemicu ini berada di posisi ke-4. Yah, itu juga cukup bagus! Mari kita lihat kesalahan apa yang dapat dideteksi PVS-Studio kali ini:



public ConnectionSwitchModLogic(....)
{
  ....
  var logo = panel.GetOrNull<RGBASpriteWidget>("MOD_ICON");
  if (logo != null)
  {
    logo.GetSprite = () =>
    {
      ....
    };
  }

  if (logo != null && mod.Icon == null)                    // <=
  {
    // Hide the logo and center just the text
    if (title != null)
      title.Bounds.X = logo.Bounds.Left;

    if (version != null)
      version.Bounds.X = logo.Bounds.X;
    width -= logo.Bounds.Width;
  }
  else
  {
    // Add an equal logo margin on the right of the text
    width += logo.Bounds.Width;                           // <=
  }
  ....
}
      
      





Peringatan Analyzer : V3125 Objek 'logo' digunakan setelah diverifikasi terhadap null. Periksa baris: 236, 222. ConnectionLogic.cs 236



Apa yang harus Anda perhatikan di sini? Nah, sebagai permulaan, logo mungkin bisa berisi null . Ini diisyaratkan oleh pemeriksaan konstan dan nama metode GetOrNull , yang mengembalikan nilai yang ditulis ke logo . Jika demikian, mari pikirkan tentang apa yang terjadi jika GetOrNull benar-benar mengembalikan null . Awalnya semuanya beres, tapi kemudian kondisinya dicentang logo! = null && mod.Icon == null . Seperti yang bisa Anda bayangkan, hasilnya adalah transisi ke lain -cabang dan ... Mencoba mengakses properti Bounds dari variabel di mana null ditulis , dan kemudian - bdysh! NullReferenceException mengetuk pintu.



Tempat ketiga - elemen Schrödinger



Terakhir, kita sampai pada tiga finalis teratas. 3 bug teratas untuk tahun 2020 ditemukan di proyek Nethermind, tentang verifikasi di mana artikel ditulis dengan judul menarik " Kode dalam satu baris atau memeriksa Nethermind menggunakan PVS-Studio C # untuk Linux ". Kesalahannya sangat sederhana, tetapi tidak terlihat oleh mata manusia, terutama jika Anda mempertimbangkan ukuran proyek. Apakah menurut Anda pemicu ini layak untuk ditempatkan?



public ReceiptsMessage Deserialize(byte[] bytes)
{
  if (bytes.Length == 0 && bytes[0] == Rlp.OfEmptySequence[0])
    return new ReceiptsMessage(null);
    ....
}
      
      





Peringatan penganalisis : V3106 Mungkin indeks di luar batas. Indeks '0' menunjuk di luar batas 'byte'. Nethermind.Network ReceiptsMessageSerializer.cs 50



Mungkin bisa mengambil barang pertama di kotak kosong akan keren, tapi di sini keinginan seperti itu hanya akan menyebabkan IndexOutOfRangeException dilemparkan . Hanya satu hal kecil - kesalahan kecil di operator, dan aplikasi sudah tidak berfungsi dengan benar, atau bahkan mungkin macet.



Tentunya, Anda harus menggunakan '||' daripada '&&'. Kesalahan logika seperti itu tidak jarang terjadi, terutama dalam desain yang kompleks. Oleh karena itu, cukup nyaman untuk memeriksa momen seperti itu dalam mode otomatis.



Juara 2 - Kurang dari 2, tetapi lebih dari 3



Di tempat kedua, saya meletakkan satu pemicu lagi pada kode dari proyek RavenDB. Izinkan saya mengingatkan Anda bahwa Anda dapat membaca tentang hasil verifikasinya (dan tidak hanya) di artikel yang sesuai .



Nah, sekarang ketemu - 2 kesalahan teratas tahun 2020:



private OrderByField ExtractOrderByFromMethod(....)
{
  ....
  if (me.Arguments.Count < 2 && me.Arguments.Count > 3)
    throw new InvalidQueryException(....);
  ....
}
      
      





Peringatan penganalisis : V3022 Expression 'me.Arguments.Count <2 && me.Arguments.Count> 3' selalu salah. Mungkin '||' operator harus digunakan di sini. QueryMetadata.cs (861) Raven.Server



Sebelumnya kita telah melihat saat-saat di mana pengecualian tak terduga dilemparkan, tetapi sekarang, sebaliknya, pengecualian yang diharapkan tidak akan pernah muncul. Ya, atau masih akan dibuang jika seseorang muncul dengan angka yang kurang dari 2, tetapi lebih dari 3.



Saya tidak akan terkejut jika Anda tidak setuju, tetapi saya sangat menyukai pemicu ini lebih dari semua yang sebelumnya. Ya, kesalahannya sangat sederhana, dan untuk memperbaikinya Anda hanya perlu mengganti operator. Ngomong-ngomong, ini juga diisyaratkan oleh pesan yang diteruskan ke konstruktor InvalidQueryException : Panggilan "ORDER BY 'spatial.distance (from, to, roundFactor)' tidak valid, diharapkan 2-3 argumen, mendapatkan" + me.Arguments.Count .



Saya setuju, ini adalah pengawasan dasar, tetapi tidak ada yang memperhatikan atau memperbaikinya, setidaknya sampai ditemukan menggunakan PVS-Studio. Itu mengingatkan saya bahwa programmer, tidak peduli seberapa berpengalaman mereka, masih (sayangnya?) Manusia. Dan orang-orang, terlepas dari kualifikasinya, dapat melewatkan bahkan kesalahan bodoh seperti itu karena berbagai alasan. Terkadang kesalahan langsung terjadi, dan terkadang Anda bisa menebak untuk waktu yang lama mengapa pengguna tidak melihat pesan tentang panggilan ORDER BY yang salah.



Juara 1 - Kutipan untuk + 100% keamanan kode



Hore, hore, hore! Kami akhirnya sampai ke pelatuk, yang menurut saya paling menarik, lucu, keren, dan seterusnya. Itu dikeluarkan untuk kode dari proyek ONLYOFFICE, yang analisisnya dikaitkan dengan salah satu artikel terbaru tahun ini - " Server Komunitas ONLYOFFICE: Bagaimana Bug Berkontribusi pada Masalah Keamanan ".



Nah, saya persembahkan kepada Anda cerita paling menyedihkan tentang ArgumentException , yang tidak akan pernah terlontar:



public void SetCredentials(string userName, string password, string domain)
{
  if (string.IsNullOrEmpty(userName))
  {
    throw new ArgumentException("Empty user name.", "userName");
  }
  if (string.IsNullOrEmpty("password"))
  {
    throw new ArgumentException("Empty password.", "password");
  }

  CredentialsUserName = userName;
  CredentialsUserPassword = password;
  CredentialsDomain = domain;
}
      
      





Peringatan Analyzer : V3022 Expression 'string.IsNullOrEmpty ("password")' selalu salah. SmtpSettings.cs 104



Sangat sulit untuk memilih kesalahan mana yang akan diletakkan di tempat mana, tetapi pemicu ini bagi saya sejak awal adalah pemimpin di antara semuanya. Kesalahan kecil yang paling sederhana - dan kodenya sudah tidak berfungsi dengan benar. Baik penyorotan di IDE, maupun ulasan, maupun akal sehat lama yang baik tidak membantu. Ini adalah fungsi kecil, sederhana, dan ditulis dengan indah. Dan bahkan di sini PVS-Studio dapat menemukan apa yang terlewatkan oleh para profesional.



Seperti biasa, iblis ada dalam detailnya. Bukankah lebih bagus jika semua detail seperti itu dicari secara otomatis? Tentu saja! Dan biarkan pengembang melakukan apa yang tidak bisa dilakukan penganalisis statis - dia membuat aplikasi baru yang indah dan aman. Dia melakukannya tanpa memikirkan apakah dia memberikan tanda kutip tambahan saat memeriksa variabel atau tidak.



Kesimpulan



Menemukan 10 kesalahan menarik di artikel tahun 2020 itu mudah. Tetapi mendistribusikannya di tempat-tempat ternyata menjadi tugas lain. Di satu sisi, beberapa pemicu lebih mencerminkan kerja mekanisme penganalisis lanjutan. Di sisi lain, beberapa kesalahan tampak lucu sampai batas tertentu. Banyak posisi yang disajikan dapat dibalik - misalnya, top-2 dan top-3.



Atau mungkin Anda berpikir harus ada beberapa hal positif lain dalam daftar ini? Bahkan, Anda bahkan dapat membuat atasan Anda sendiri dengan mengikuti tautannyake daftar artikel dan temukan pemicu paling enak menurut Anda. Dalam hal ini, pastikan untuk membuang atasan 2020 Anda di komentar, saya akan membacanya dengan senang hati. Bisakah Anda membuat daftar yang lebih menarik dari saya?



Tentu saja, pertanyaan tentang 'daya tarik' peringatan itu subjektif. Menurut saya, kriteria utama untuk mengevaluasi respon adalah apakah seorang programmer yang melihat peringatan dari PVS-Studio akan mengubah sesuatu pada kode yang sesuai? Daftar ini baru saja disusun dari pemicu untuk fragmen, yang menurut saya akan terlihat lebih baik jika pengembang menggunakan analisis statis. Selain itu, tidak ada masalah dalam mencoba PVS-Studio dengan memeriksa proyek Anda sendiri atau beberapa proyek lain. Anda hanya perlu mengikuti tautannya, unduh versi penganalisis yang diperlukan di sana dan minta kunci uji coba dengan mengisi formulir kecil.



Sekian untuk saya, terima kasih banyak atas perhatiannya dan sampai jumpa!





Jika Anda ingin berbagi artikel ini dengan audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Nikita Lipilin. 10 Bug Teratas Ditemukan di Proyek C # pada tahun 2020 .



All Articles