Clever Geek Handbook

PVS-Studio untuk Java: pengembangan diagnostik



Untuk perubahan, hari ini kami akan memberi tahu Anda sedikit tentang proses pengembangan dan penyelesaian aturan diagnostik untuk PVS-Studio Java. Mari kita lihat mengapa pemicu lama tidak terlalu banyak mengambang dari rilis ke rilis, dan yang baru tidak terlalu gila. Dan kami akan membocorkan sedikit lagi "apa saja rencana para javist" dan menunjukkan beberapa kesalahan indah (dan tidak demikian) yang ditemukan dengan bantuan diagnostik dari rilis berikutnya.



Proses Pengembangan Diagnostik dan SelfTester



Secara alami, setiap aturan diagnostik baru dimulai dengan sebuah ide. Dan karena penganalisis Java adalah arah termuda dalam pengembangan PVS-Studio, pada dasarnya kami mencuri ide-ide ini dari departemen C / C ++ dan C #. Namun tidak semuanya buruk: kami juga menambahkan aturan yang kami buat sendiri (termasuk oleh pengguna - terima kasih!), Sehingga nanti departemen yang sama akan mencurinya dari kami. Siklusnya, seperti yang mereka katakan.



Penerapan aturan dalam kode dalam banyak kasus ternyata merupakan tugas pipeline yang cukup. Anda membuat file dengan beberapa contoh sintetis, menandai dengan tangan Anda di mana kesalahan seharusnya, dan dengan debugger di siap Anda menjalankan melalui pohon sintaks sampai Anda bosan dan mencakup semua kasus yang ditemukan. Terkadang aturan berubah menjadi sangat sederhana (misalnya, V6063terdiri dari beberapa baris), dan terkadang Anda harus berpikir cukup lama tentang logikanya.



Namun, ini baru permulaan. Seperti yang Anda ketahui, kami tidak terlalu menyukai contoh sintetik, karena contoh tersebut sangat tidak mencerminkan jenis pemicu penganalisis dalam project nyata. Ngomong-ngomong, bagian penting dari contoh ini dalam pengujian unit kami diambil dari proyek nyata - hampir tidak mungkin untuk menemukan semua kemungkinan kasus sendiri. Dan pengujian unit juga memungkinkan kami untuk tidak kehilangan pemicu pada contoh-contoh dari dokumentasi. Ada preseden, ya, hanya ssst.



Jadi, hal positif dalam proyek nyata harus ditemukan terlebih dahulu. Dan Anda juga perlu memeriksanya:



  • Aturannya tidak akan jatuh pada kegilaan open-source, di mana solusi "menarik" biasa terjadi;
  • ( - , );
  • data-flow ( ) - ;
  • open-source ;
  • over 9000%;
  • "" , ;
  • .


Secara umum, di sini, seperti kesatria di atas kuda (sedikit pincang, tetapi kami sedang mengerjakannya), SelfTester tampil ke depan. Tugas utamanya dan satu-satunya adalah memeriksa sekumpulan proyek secara otomatis dan menunjukkan pemicu mana yang telah ditambahkan, dihilangkan, atau diubah relatif terhadap "referensi" dalam sistem kontrol versi. Berikan perbedaan untuk laporan penganalisis dan singkatnya, tunjukkan kode yang sesuai dalam proyek. Saat ini SelfTester untuk Java sedang menguji 62 proyek sumber terbuka versi berjanggut, di antaranya adalah, misalnya, DBeaver, Hibernate, dan Spring. Satu proses penuh dari semua proyek membutuhkan waktu 2-2,5 jam, yang tidak diragukan lagi menyakitkan, tetapi tidak ada yang bisa dilakukan.





Pada gambar di atas, proyek "hijau" adalah proyek yang tidak ada yang berubah. Setiap perbedaan dalam proyek "merah" ditinjau secara manual dan, jika benar, dikonfirmasi dengan tombol "Setuju" yang sama. Omong-omong, kit distribusi penganalisis akan dibuat hanya jika SelfTester memberikan hasil hijau murni. Secara umum, inilah cara kami menjaga konsistensi hasil di antara versi yang berbeda.



Selain untuk menjaga konsistensi hasil, SelfTester memungkinkan kami menyingkirkan sejumlah besar positif palsu bahkan sebelum diagnostik dirilis. Pola perkembangan yang khas terlihat seperti ini:



  • , . , " double-checked locking" ;
  • SelfTester-, ;
  • , -;
  • SelfTester- , ;
  • 3-4, ;
  • , , ( , );
  • , master.


Untungnya, SelfTester berjalan penuh cukup jarang, dan Anda tidak perlu menunggu "2-2,5 jam" terlalu sering. Dari waktu ke waktu, keberuntungan melewati, dan pemicu ternyata ada di proyek besar seperti Sakai dan Apache Hive - saatnya minum kopi, minum kopi, dan minum kopi. Anda juga dapat mempelajari dokumentasi, tetapi ini tidak untuk semua orang.



"Mengapa kita perlu tes unit, karena ada alat ajaib seperti itu?"



Dan kemudian tesnya jauh lebih cepat. Beberapa menit - dan sudah ada hasilnya. Mereka juga memungkinkan Anda untuk melihat dengan tepat bagian mana dari aturan yang jatuh. Selain itu, tidak selalu semua operasi yang dapat diterima dari aturan apa pun terperangkap dalam project SelfTester, tetapi pengoperasiannya juga harus diperiksa.



Masalah baru pada kenalan lama



Awalnya, bagian artikel ini dimulai dengan kata-kata "Versi proyek di SelfTester sudah cukup lama, jadi sebagian besar kesalahan yang disajikan kemungkinan besar telah diperbaiki". Namun, ketika saya memutuskan untuk memastikan ini, saya terkejut. Setiap kesalahan tetap ada. Segala sesuatu. Ini berarti dua hal:



  • Kesalahan ini tidak terlalu penting agar aplikasi berfungsi. Ngomong-ngomong, banyak dari mereka ada dalam kode pengujian, dan pengujian yang salah hampir tidak bisa disebut konsisten.
  • Kesalahan ini ditemukan di file yang jarang digunakan pada proyek besar, yang sulit dijangkau oleh pengembang. Karena itu, kode yang salah ditakdirkan untuk berada di sana untuk waktu yang sangat lama: kemungkinan besar, sampai beberapa bug kritis terjadi karenanya.


Bagi mereka yang ingin menggali lebih dalam, akan ada tautan ke versi tertentu yang kami periksa.



PS Di atas tidak berarti bahwa analisis statis hanya menangkap kesalahan yang tidak berbahaya dalam kode yang tidak digunakan. Kami memeriksa versi rilis (dan hampir rilis) proyek - di mana pengembang dan penguji (dan terkadang, sayangnya, pengguna) menemukan bug yang paling relevan dengan tangan, yang panjang, mahal dan menyakitkan. Anda dapat membaca lebih lanjut tentang ini di artikel kami " Kesalahan yang tidak dapat ditemukan oleh analisis kode statis, karena tidak digunakan ".



Apache Dubbo dan menu kosong



GitHub



Diagnostics " V6080 Pertimbangkan untuk memeriksa kesalahan cetak. Ada kemungkinan variabel yang ditetapkan harus diperiksa dalam kondisi berikutnya " sudah dirilis di versi 7.08, tetapi belum muncul di artikel kami, jadi sekarang saatnya untuk memperbaikinya.



Menu.java:40 



public class Menu
{
  private Map<String, List<String>> menus = new HashMap<String, List<String>>();

  public void putMenuItem(String menu, String item)
  {
    List<String> items = menus.get(menu);
    if (item == null)                      // <=
    {
      items = new ArrayList<String>();
      menus.put(menu, items);
    }
    items.add(item);
  }
  ....
}


Contoh klasik kamus "koleksi kunci" dan kesalahan ketik yang sama klasiknya. Pengembang ingin membuat koleksi yang sesuai dengan kunci tersebut, jika belum ada, tetapi dia mencampur nama variabel dan tidak hanya mendapatkan operasi metode yang salah, tetapi juga NullPointerException di baris terakhir. Untuk Java 8 dan yang lebih baru, untuk mengimplementasikan kamus seperti itu, Anda harus menggunakan metode computeIfAbsent : 



public class Menu
{
  private Map<String, List<String>> menus = new HashMap<String, List<String>>();

  public void putMenuItem(String menu, String item)
  {
    List<String> items = menus.computeIfAbsent(menu, key -> new ArrayList<>());
    items.add(item);
  }
  ....
}


Glassfish dan penguncian ganda



GitHub



Salah satu diagnostik yang akan disertakan dalam rilis berikutnya adalah memeriksa penerapan yang benar dari pola "penguncian yang diperiksa ulang". Glassfish ternyata menjadi pemegang rekor untuk deteksi dari proyek SelfTester: secara total, PVS-Studio menemukan 10 area masalah dalam proyek menggunakan aturan ini. Saya mengundang pembaca untuk bersenang-senang dan mencari dua di antaranya dalam potongan kode di bawah ini. Untuk bantuan, lihat dokumentasi: " V6082 Penguncian dua kali tidak aman ". Nah, atau, jika Anda tidak mau sama sekali, di akhir artikel.



EjbComponentAnnotationScanner.java 



public class EjbComponentAnnotationScanner
{
  private Set<String> annotations = null;

  public boolean isAnnotation(String value)
  {
    if (annotations == null)
    {
      synchronized (EjbComponentAnnotationScanner.class)
      {
        if (annotations == null)
        {
          init();
        }
      }
    }
    return annotations.contains(value);
  }

  private void init()
  {
    annotations = new HashSet();
    annotations.add("Ljavax/ejb/Stateless;");
    annotations.add("Ljavax/ejb/Stateful;");
    annotations.add("Ljavax/ejb/MessageDriven;");
    annotations.add("Ljavax/ejb/Singleton;");
  }

  ....
}


SonarQube dan aliran data



GitHub



Meningkatkan diagnostik tidak hanya tentang langsung mengubah kode mereka untuk menangkap lebih banyak tempat yang mencurigakan atau menghapus kesalahan positif. Penandaan metode secara manual untuk aliran data juga memainkan peran penting dalam pengembangan penganalisis - misalnya, Anda dapat menulis bahwa metode pustaka ini dan itu selalu mengembalikan bukan nol. Saat menulis diagnostik baru, kami secara tidak sengaja menemukan bahwa metode Map # clear () tidak diberi markup . Terlepas dari kode yang jelas-jelas bodoh bahwa " Koleksi V6009 kosong. Panggilan fungsi 'hapus' tidak masuk akal ", diagnostik mulai menangkap , kami dapat menemukan kesalahan ketik yang hebat.



MetricRepositoryRule.java:90 



protected void after()
{
  this.metricsById.clear();
  this.metricsById.clear();
}


Pada pandangan pertama, membersihkan kamus lagi bukanlah kesalahan. Dan kami bahkan akan berpikir bahwa ini adalah garis yang digandakan secara acak, jika pandangan kami tidak turun sedikit - secara harfiah ke metode selanjutnya. 



protected void after()
{
  this.metricsById.clear();
  this.metricsById.clear();
}
public Metric getByKey(String key)
{
  Metric res = metricsByKey.get(key);
  ....
}


Persis. Kelas tersebut memiliki dua kolom dengan nama yang mirip metricsById dan metricsByKey . Saya yakin bahwa dalam metode setelah pengembang ingin menghapus kedua kamus, tetapi penyelesaian otomatis gagal, atau dia secara tidak sengaja memasukkan nama yang sama. Dengan demikian, dua kamus yang menyimpan data terkait akan tidak sinkron setelah panggilan ke setelah .



Sakai dan koleksi kosong



GitHub



Diagnostik baru lainnya yang akan disertakan dalam rilis berikutnya adalah " V6084 Suspicious return of an always empty collection ". Cukup mudah untuk lupa menambahkan item ke dalam koleksi, apalagi setiap item perlu diinisialisasi terlebih dahulu. Dari pengalaman pribadi, kesalahan seperti itu paling sering menyebabkan aplikasi tidak mogok, tetapi perilaku aneh atau tidak adanya fungsi apa pun.



DateModel.java:361 



public List getDaySelectItems()
{
  List selectDays = new ArrayList();
  Integer[] d = this.getDays();
  for (int i = 0; i < d.length; i++)
  {
    SelectItem selectDay = new SelectItem(d[i], d[i].toString());
  }
  return selectDays;
}


Omong-omong, kelas yang sama berisi metode yang sangat mirip tanpa kesalahan yang sama. Misalnya: 



public List getMonthSelectItems()
{
  List selectMonths = new ArrayList();
  Integer[] m = this.getMonths();
  for (int i = 0; i < m.length; i++)
  {
    SelectItem selectMonth = new SelectItem(m[i], m[i].toString());
    selectMonths.add(selectMonth);
  }
  return selectMonths;
}


Rencana untuk masa depan



Terlepas dari berbagai hal internal yang tidak terlalu menarik, kami sedang berpikir untuk menambahkan diagnostik untuk Kerangka Kerja Musim Semi ke penganalisis Java. Ini bukan hanya roti utama bagi Javist, tetapi juga mengandung banyak momen tidak jelas yang bisa membuat orang tersandung. Kami belum begitu yakin dalam bentuk apa diagnosis ini pada akhirnya akan muncul, kapan itu akan terjadi dan apakah itu akan terjadi sama sekali. Tetapi kami yakin bahwa kami membutuhkan ide untuk mereka dan proyek sumber terbuka menggunakan Spring for SelfTester. Jadi, jika Anda memiliki sesuatu dalam pikiran, sarankan (di komentar atau pesan pribadi, Anda juga bisa)! Dan semakin banyak kebaikan yang kita kumpulkan, semakin banyak prioritas akan dialihkan ke sana.



Dan terakhir, ada kesalahan dalam implementasi penguncian yang diperiksa ulang dari Glassfish:



  • Bidang ini tidak dinyatakan 'mudah menguap'.
  • Objek tersebut pertama kali diterbitkan dan kemudian diinisialisasi.


Mengapa semua ini buruk - sekali lagi, Anda dapat melihat di dokumentasi .





Jika Anda ingin berbagi artikel ini dengan audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Nikita Lazeba. Di Balik Terpal PVS-Studio untuk Java: Bagaimana Kami Mengembangkan Diagnostik .


More articles:


All Articles