Saya memutuskan untuk menguji penganalisis kode Java statis IntelliJ IDEA dan, dengan itu, menguji proyek The Chemistry Development Kit . Di sini saya akan memberikan beberapa kesalahan yang saya temukan. Saya pikir beberapa dari mereka adalah tipikal untuk program Java secara keseluruhan, sehingga mereka bisa menarik.
The Chemistry Development Kit adalah perpustakaan Java sumber terbuka untuk menyelesaikan masalah chemoinformatika dan bioinformatika. Ketika saya terlibat dalam bioinformatika, kami secara aktif menggunakannya. Proyek ini telah dikembangkan selama lebih dari 20 tahun, memiliki puluhan penulis, dan kualitas kode di sana sangat tidak merata. Namun demikian, ada tes unit dalam proyek, dan integrasi dengan penganalisa cakupan JaCoCo ditentukan dalam pom.xml . Selain itu, plugin untuk tiga analisis statis dikonfigurasikan di sana: FindBugs , PMD , Checkstyle . Semakin menarik untuk memeriksa peringatan yang tersisa.
Alat analisis kode Java statis yang dibangun ke IntelliJ IDEA tidak kalah dengan alat analisis statis khusus, tetapi dalam beberapa hal melampaui mereka. Selain itu, hampir semua kemampuan analisis statis tersedia dalam Community Edition , sebuah IDE open source gratis. Khususnya, versi gratis menghasilkan semua peringatan yang dijelaskan dalam artikel ini.
Secara default, analisis statis dilakukan terus-menerus dalam mode pengeditan kode, jadi jika Anda menulis kode di IntelliJ IDEA, maka Anda akan memperbaiki banyak kesalahan secara harfiah dalam beberapa detik setelah dibuat, bahkan sebelum menjalankan tes. Anda dapat memeriksa seluruh proyek atau bagiannya dalam mode batch menggunakan Analisis | Periksa Kode atau jalankan inspeksi terpisah menggunakan Analisis | Jalankan Inspeksi berdasarkan Nama . Dalam hal ini, beberapa inspeksi menjadi tersedia yang, karena kerumitannya, tidak berfungsi dalam mode edit. Namun, ada beberapa inspeksi semacam itu.
Banyak inspeksi IntelliJ IDEA tidak melaporkan bug, melainkan kode yang tidak akurat atau menawarkan alternatif yang lebih idiomatis, indah, atau cepat. Ini berguna ketika Anda terus-menerus bekerja di IDE. Namun, dalam kasus saya, lebih baik memulai dengan pesan-pesan yang memperingatkan tentang bug nyata. Pada dasarnya, kategori Java menarik | Kemungkinan Bug , meskipun ada kategori lain yang perlu ditelusuri, seperti Masalah Numerik .
Saya hanya akan memberi tahu Anda tentang beberapa peringatan menarik.
1. Unary plus
Sudah ada 66 plus unary dalam proyek ini. Untuk menulis +1 bukannya hanya 1 kadang-kadang saya ingin menjadi cantik. Namun, dalam beberapa kasus, plus unary muncul jika alih-alih += menulis =+ :
int totalCharge1 = 0; while (atoms1.hasNext()) { totalCharge1 = +((IAtom) atoms1.next()).getFormalCharge(); } Iterator<IAtom> atoms2 = products.atoms().iterator(); int totalCharge2 = 0; while (atoms2.hasNext()) { totalCharge2 = +((IAtom) atoms2.next()).getFormalCharge(); }
Kesalahan ketik yang jelas yang mengabaikan semua iterasi loop kecuali yang terakhir. Mungkin tampak aneh bahwa itu tidak ditulis "ruang sama dengan ruang plus", tetapi "ruang sama dengan ruang plus". Namun, keanehan menghilang jika Anda mempelajari sejarah . Awalnya, "sama" dan "plus" benar-benar ada, tetapi pada 2008 mereka pergi melalui formatter otomatis, dan kode berubah. Ngomong-ngomong, ini adalah moral untuk analisa statis: masuk akal untuk mengeluarkan peringatan berdasarkan pemformatan aneh, tetapi jika kode diformat secara otomatis, peringatan akan hilang dan bug akan tetap ada.
2. Pembagian integer yang mengarah ke pecahan
Sebuah kesalahan yang agak menjengkelkan, tetapi analisa statis menemukannya dengan baik. Berikut ini sebuah contoh :
angle = 1 / 180 * Math.PI;
Sayangnya, sudutnya ternyata bukan satu derajat, tetapi nol. Kesalahan serupa :
Integer c1 = features1.get(key); Integer c2 = features2.get(key); c1 = c1 == null ? 0 : c1; c2 = c2 == null ? 0 : c2; sum += 1.0 - Math.abs(c1 - c2) / (c1 + c2);
Tampaknya kedua angka c1 dan c2 non-negatif, yang berarti bahwa perbedaan modulus tidak akan pernah melebihi jumlah. Oleh karena itu, hasilnya akan menjadi 0 jika kedua angka bukan nol, atau 1 jika salah satunya adalah 0.
3. Panggil Class.getClass ()
Kadang-kadang orang memanggil metode getClass() pada objek bertipe Class . Hasilnya lagi objek bertipe Class dengan nilai konstan Class.class . Ini biasanya kesalahan: getClass() tidak perlu dipanggil. Sebagai contoh, di sini :
public <T extends ICDKObject> T ofClass(Class<T> intf, Object... objects) { try { if (!intf.isInterface()) throw new IllegalArgumentException("expected interface, got " + intf.getClass()); ...
Jika pengecualian terjadi, melaporkannya akan sama sekali tidak berguna. By the way, kesalahan dalam prosedur penanganan kesalahan sering ditemukan oleh analisis statis di proyek-proyek lama: sebagai aturan, prosedur penanganan kesalahan diuji yang terburuk.
4. Panggil toString () pada sebuah array
Ini adalah genre klasik: toString () tidak didefinisikan ulang untuk array, dan hasilnya cukup berguna. Biasanya ini dapat ditemukan dalam pesan diagnostik .
int[] dim = {0, 0, 0}; ... return "Dim:" + dim + " SizeX:" + grid.length + " SizeY:" + grid[0].length + " SizeZ:"...
Sulit untuk memperhatikan masalah dengan mata, karena di sini dim.toString() implisit, tetapi string concatenation mendelegasikannya. Perbaikan segera disarankan - bungkus dalam Arrays.toString(dim) .
5. Koleksi dibaca tetapi tidak diisi
Ini, juga, sering ditemukan dalam basis kode yang tidak menjalani pengujian konstan oleh penganalisa statis. Ini adalah contoh sederhana :
final Set<IBond> bondsToHydrogens = new HashSet<IBond>();
Tentunya pengisian baru saja terlewat. Analisis statis memiliki pemeriksaan yang lebih sederhana yang menunjukkan variabel yang tidak digunakan, tetapi variabel tersebut digunakan di sini, sehingga mereka diam. Kami membutuhkan inspeksi yang lebih cerdas yang tahu tentang koleksi.
6. Sebaliknya: kita mengisi, tetapi tidak membaca
Kasus sebaliknya juga dimungkinkan. Berikut ini adalah contoh dengan array :
int[] tmp = new int[trueBits.length - 1]; System.arraycopy(trueBits, 0, tmp, 0, i); System.arraycopy(trueBits, i + 1, tmp, i, trueBits.length - i - 1);
Inspeksi tahu bahwa argumen ketiga ke metode arraycopy hanya digunakan untuk menulis array, dan setelah itu array tidak digunakan sama sekali. Dilihat oleh logika kode, baris trueBits = tmp; dilewati trueBits = tmp; .
7. Perbandingan Integer dengan ==
Ini adalah bug yang berbahaya, karena nilai kecil dari objek Integer di-cache, dan semuanya dapat bekerja dengan baik sampai suatu hari jumlahnya melebihi 127. Masalah seperti itu mungkin tidak jelas sama sekali :
for (int a = 0; a < cliqueSize; a++) { for (int b = 0; b < vecSize; b += 3) { if (cliqueList.get(a) == compGraphNodes.get(b + 2)) { cliqueMapping.add(compGraphNodes.get(b)); cliqueMapping.add(compGraphNodes.get(b + 1)); } } }
Nah, tampaknya beberapa objek dalam beberapa daftar dibandingkan, mungkin semuanya baik-baik saja. Orang harus berhati-hati untuk melihat bahwa daftar ini berisi objek bertipe Integer.
8. Gandakan di Peta
Dalam inspeksi ini, sebuah gambar bernilai ribuan kata. Lihat kesalahannya ?
9. Hasil dari metode ini tidak digunakan.
Hasil dari beberapa metode adalah bodoh untuk tidak digunakan, yang dengan mudah dilaporkan IDEA:
currentChars.trim();
Mungkin, itu berarti currentChars = currentChars.trim(); . Karena string di Jawa tidak berubah, jika hasilnya tidak dipindahkan, tidak ada yang akan terjadi. Juga ditemukan, misalnya , str.substring(2) .
Omong-omong, ini adalah pemeriksaan yang agak rumit. Selain daftar metode yang telah disiapkan, kami terkadang mencoba untuk secara otomatis menentukan metode yang hasilnya layak digunakan. Di sini, sebuah analisis antar prosedur diperlukan, baik dalam teks sumber maupun dalam bytecode perpustakaan. Dan semua ini dilakukan dengan cepat dalam proses mengedit kode!
10. Cabang switch yang tidak terjangkau
Karena kami mengecualikan karakter dengan kode lebih besar dari 128, cabang \u2012-\u2212 tidak \u2012-\u2212 dijangkau. Tampaknya itu tidak layak dikecualikan.
11. Kondisi yang tidak terjangkau
Masalah yang sangat luar biasa dalam rantai kondisi :
if (oxNum == 0) { if (hybrid.equals("sp3")) { ... } else if (hybrid.equals("sp2")) return 47; } else if (oxNum == 1 && hybrid.equals("sp3")) return 47; else if ((oxNum == 2 && hybrid.equals("sp3")) || (oxNum == 1 && hybrid.equals("sp2")) || (oxNum == 0 && hybrid.equals("sp")))
Dalam logika kondisional yang kompleks, ini tidak jarang: kami memeriksa kondisi yang tidak mungkin benar, karena fragmennya telah diperiksa di atas. Di sini kita memiliki cabang terpisah oxNum == 0 , kalau tidak kita periksa oxNum == 0 && hybrid.equals("sp") , yang tentu saja tidak bisa.
12. Kami menulis ke array dengan panjang nol
Terkadang IntelliJ IDEA akan melihat jika Anda menulis ke array di luar ukurannya :
Point3d points[] = new Point3d[0];
13. Memeriksa panjang setelah mengakses indeks
Masalah umum lainnya dengan prosedur dan lagi selama penanganan kesalahan :
public void setParameters(Object[] params) throws CDKException { if (params.length > 1) { throw new CDKException("..."); } if (!(params[0] instanceof Integer)) {
Dalam kasus array kosong, pembuat kode ingin keluar dengan tenang, tetapi karena verifikasi, ia akan keluar, dengan keras membenturkan ArrayIndexOutOfBoundsException. Jelas, pesanan cek sudah rusak.
14. Periksa null setelah akses
Dan lagi, urutan tindakan dilanggar, kali ini dengan nol :
while (!line.startsWith("frame:") && input.ready() && line != null) { line = input.readLine(); logger.debug(lineNumber++ + ": ", line); }
IDEA menulis line != null selalu benar. Kebetulan ceknya benar-benar berlebihan, tetapi di sini kodenya tampak seolah-olah nol.
15. Disjungsi alih-alih konjungsi
Orang sering membingungkan operator logika AND dan OR. Proyek CDK tidak terkecuali :
if (rStereo != 4 || pStereo != 4 || rStereo != 3 || pStereo != 3) { ... }
Apapun rStereo dan pStereo sama dengan, jelas bahwa mereka tidak dapat sama dengan empat dan tiga pada saat yang sama, oleh karena itu kondisi ini selalu benar.
16. Sekali lagi disjungsi alih-alih konjungsi
Kesalahan serupa , tetapi ditangkap oleh pesan lain:
if (getFirstMapping() != null || !getFirstMapping().isEmpty()) { ... }
Kita bisa sampai ke sisi kanan hanya jika getFirstMapping() mengembalikan null , tetapi dalam hal ini kami dijamin NullPointerException, yang diperingatkan IDEA. Omong-omong, di sini kita mengandalkan stabilitas hasil dari metode getFirstMapping() . Terkadang kita menggunakan heuristik, tetapi stabilitas dianalisis langsung di sini. Karena kelas adalah final, metode tidak dapat diganti. return firstSolution.isEmpty() ? null : firstSolution IDEA memeriksa kembali body return firstSolution.isEmpty() ? null : firstSolution return firstSolution.isEmpty() ? null : firstSolution dan menentukan stabilitas yang turun ke stabilitas metode Map#isEmpty , yang sebelumnya dijelaskan sebagai stabil.
17. Hirarki antarmuka dan instanceof
Saat memeriksa objek untuk memiliki antarmuka apa pun, jangan lupa bahwa antarmuka dapat saling mewarisi:
if (object instanceof IAtomContainer) { root = convertor.cdkAtomContainerToCMLMolecule((IAtomContainer) object); } else if (object instanceof ICrystal) { root = convertor.cdkCrystalToCMLMolecule((ICrystal) object); } ...
Antarmuka ICrystal memperluas antarmuka IAtomContainer , sehingga cabang kedua jelas tidak dapat IAtomContainer : jika kristal datang ke sini, ia akan jatuh ke cabang pertama.
18. Menelusuri daftar kosong
Penulis kode ini mungkin tidak terlalu mengenal bahasa Jawa:
List<Integer> posNumList = new ArrayList<Integer>(size); for (int i = 0; i < posNumList.size(); i++) { posNumList.add(i, 0); }
Parameter ukuran di ArrayList menunjukkan ukuran awal array internal. Ini digunakan untuk optimasi untuk mengurangi jumlah alokasi, jika Anda tahu sebelumnya berapa banyak item yang akan ditempatkan di sana. Namun, pada kenyataannya, item dalam daftar tidak muncul, dan metode size() mengembalikan 0. Oleh karena itu, siklus berikutnya dengan upaya untuk menginisialisasi item daftar dengan nol sama sekali tidak berguna.
19. Jangan lupa untuk menginisialisasi bidang
Alat analisa dengan cara khusus memeriksa konstruktor, dengan mempertimbangkan inisialisasi lapangan. Berkat ini, kesalahan seperti itu ditemukan:
public class IMatrix { public double[][] realmatrix; public double[][] imagmatrix; public int rows; public int columns; public IMatrix(Matrix m) { rows = m.rows; columns = m.columns; int i, j; for (i = 0; i < rows; i++) for (j = 0; j < columns; j++) { realmatrix[i][j] = m.matrix[i][j];
Terlepas dari kenyataan bahwa bidangnya bersifat publik, tidak ada seorang pun di sini yang dapat mengganjal dan menginisialisasi mereka di depan konstruktor. Oleh karena itu, IDEA dengan berani mengeluarkan peringatan bahwa mengakses elemen array akan meningkatkan NullPointerException.
20. Jangan ulangi dua kali
Kondisi berulang juga sering terjadi. Berikut ini sebuah contoh :
if (commonAtomCount > vfMCSSize && commonAtomCount > vfMCSSize) { return true; }
Bug seperti itu berbahaya, karena Anda tidak pernah tahu, kondisi kedua hanya berlebihan, atau penulis ingin memeriksa sesuatu yang lain. Jika ini tidak segera diperbaiki, maka bisa sulit untuk mengetahuinya. Ini adalah alasan lain mengapa analisis statis harus digunakan secara konstan.
Saya melaporkan beberapa bug ini di pelacak bug proyek . Sangat mengherankan bahwa ketika penulis proyek memperbaiki bagian, mereka sendiri menggunakan penganalisa IntelliJ IDEA, menemukan masalah lain yang tidak saya tulis, dan juga mulai memperbaikinya . Saya pikir ini adalah pertanda baik: penulis menyadari pentingnya analisis statis.