Decidí probar el analizador de código Java estático IntelliJ IDEA y, con él, probé el proyecto The Chemistry Development Kit . Aquí daré algunos errores que encontré. Creo que algunos de ellos son típicos de los programas Java en su conjunto, por lo que pueden ser interesantes.
El Kit de Desarrollo de Química es una biblioteca Java de código abierto para resolver los problemas de quimioinformática y bioinformática. Cuando me dedicaba a la bioinformática, la utilizamos activamente. El proyecto ha estado en desarrollo durante más de 20 años, tiene docenas de autores y la calidad del código allí es muy desigual. Sin embargo, hay pruebas unitarias en el proyecto y la integración con el analizador de cobertura JaCoCo se prescribe en pom.xml . Además, los complementos para tres analizadores estáticos están configurados allí: FindBugs , PMD , Checkstyle . Es aún más interesante comprobar qué advertencias quedan.
El analizador de código Java estático integrado en IntelliJ IDEA no es inferior a las herramientas especializadas de análisis estático, pero de alguna manera las supera. Además, prácticamente todas las capacidades de análisis estático están disponibles en Community Edition , un IDE de código abierto gratuito. En particular, la versión gratuita produce todas las advertencias descritas en este artículo.
Por defecto, el análisis estático se realiza constantemente en modo de edición de código, por lo que si escribe código en IntelliJ IDEA, corregirá muchos errores literalmente en segundos después de que se hayan realizado, incluso antes de ejecutar las pruebas. Puede verificar todo el proyecto o su parte en modo por lotes utilizando Analizar | Inspeccione el código o realice una inspección por separado utilizando Analizar | Ejecute la inspección por nombre . En este caso, algunas inspecciones están disponibles que, debido a la complejidad, no funcionan en modo de edición. Sin embargo, hay pocas inspecciones de este tipo.
Muchas inspecciones de IntelliJ IDEA no reportan errores, sino códigos inexactos u ofrecen una alternativa más idiomática, hermosa o rápida. Esto es útil cuando trabajas constantemente en el IDE. Sin embargo, en mi caso, es mejor comenzar con esos mensajes que advierten sobre errores reales. Básicamente, la categoría Java es interesante | Errores probables , aunque hay otras categorías que vale la pena explorar, como problemas numéricos .
Solo le contaré algunas advertencias interesantes.
1. Unario más
Ya había 66 ventajas unarias en el proyecto. Para escribir +1 lugar de solo 1 veces quiero ser hermosa. Sin embargo, en algunos casos, el plus unario aparece si en lugar de += escribieron =+ :
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(); }
Un error tipográfico obvio que ignora todas las iteraciones del bucle, excepto la última. Puede parecer extraño que no esté escrito "espacio igual a más espacio", sino "espacio igual a espacio más". Sin embargo, la extrañeza desaparece si profundizas en la historia . Inicialmente, "igual" y "más" estaban realmente allí, pero en 2008 pasaron por un formateador automático, y el código cambió. Por cierto, esta es la moraleja para los analizadores estáticos: es razonable emitir advertencias basadas en un formato extraño, pero si el código se formatea automáticamente, las advertencias desaparecerán y los errores permanecerán.
2. División entera que conduce a fraccional
Un error bastante molesto, pero los analizadores estáticos lo encuentran bien. Aquí hay un ejemplo :
angle = 1 / 180 * Math.PI;
Desafortunadamente, el ángulo resultó no ser un grado, sino cero. Error similar
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);
Parece que ambos números c1 y c2 no c2 negativos, lo que significa que el módulo de la diferencia nunca excederá la suma. Por lo tanto, el resultado será 0 si ambos números son distintos de cero o 1 si uno de ellos es 0.
3. Llame a Class.getClass ()
A veces las personas llaman al método getClass() en un objeto de tipo Class . El resultado es nuevamente un objeto de tipo Class con el valor constante Class.class . Esto suele ser un error: no es necesario llamar a getClass() . Por ejemplo, aquí :
public <T extends ICDKObject> T ofClass(Class<T> intf, Object... objects) { try { if (!intf.isInterface()) throw new IllegalArgumentException("expected interface, got " + intf.getClass()); ...
Si ocurre una excepción, informarlo será completamente inútil. Por cierto, los errores en el procedimiento de manejo de errores a menudo se encuentran mediante análisis estático en proyectos antiguos: como regla, los procedimientos de manejo de errores se prueban peor.
4. Llame a toString () en una matriz
Este es un clásico del género: toString () no está redefinido para matrices, y su resultado es bastante inútil. Por lo general, esto se puede encontrar en los mensajes de diagnóstico .
int[] dim = {0, 0, 0}; ... return "Dim:" + dim + " SizeX:" + grid.length + " SizeY:" + grid[0].length + " SizeZ:"...
Es difícil notar el problema con los ojos, porque aquí dim.toString() implícito, pero la concatenación de cadenas le delega. Se sugiere una solución inmediata: envuelva en Arrays.toString(dim) .
5. La colección se lee pero no se llena
Esto también se encuentra a menudo en una base de código que no se somete a pruebas constantes por parte de un analizador estático. Aquí hay un ejemplo simple :
final Set<IBond> bondsToHydrogens = new HashSet<IBond>();
Obviamente llenando simplemente se perdió. Los analizadores estáticos tienen comprobaciones más simples que indican una variable no utilizada, pero la variable se usa aquí, por lo que son silenciosos. Necesitamos una inspección más inteligente que sepa sobre colecciones.
6. Por el contrario: llenamos, pero no leemos
Los casos inversos también son posibles. Aquí hay un ejemplo con una matriz :
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);
La inspección sabe que el tercer argumento del método arraycopy se usa solo para escribir la matriz, y después de eso la matriz no se usa en absoluto. A juzgar por la lógica del código, trueBits = tmp; omite la línea trueBits = tmp; .
7. Comparación de Integer por ==
Este es un error insidioso, porque los valores pequeños de los objetos Integer se almacenan en caché, y todo puede funcionar bien hasta que un día el número exceda 127. Tal problema puede no ser evidente en absoluto :
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)); } } }
Bueno, parece que se comparan algunos objetos en algunas listas, tal vez todo está bien. Hay que tener cuidado de ver que estas listas contienen objetos de tipo Integer.
8. Duplicar en el mapa
En esta inspección, una imagen vale más que mil palabras. ¿Ves el error ?
9. El resultado del método no se utiliza.
El resultado de algunos métodos es absurdo de no usar, lo que IDEA informa fácilmente :
currentChars.trim();
Probablemente, significaba currentChars = currentChars.trim(); . Dado que las cadenas en Java son inmutables, si el resultado no se reasigna, no sucederá nada. También se encuentra, por ejemplo , str.substring(2) .
Por cierto, esta es una inspección bastante complicada. Además de una lista preparada de métodos, a veces tratamos de determinar automáticamente los métodos cuyo resultado vale la pena usar. Aquí, se requiere un análisis interprocedural, tanto en el texto fuente como en el código de bytes de las bibliotecas. ¡Y todo esto se hace sobre la marcha en el proceso de edición del código!
10. Ramas de interruptor inalcanzables
Como excluimos caracteres con un código mayor que 128, las ramas \u2012-\u2212 inalcanzables. Parece que no valía la pena excluirlo.
11. Condición inalcanzable
Un problema absolutamente maravilloso en la cadena de condiciones :
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")))
En la lógica condicional compleja, esto no es infrecuente: verificamos una condición que no puede ser verdadera, porque su fragmento ya se ha verificado anteriormente. Aquí tenemos una rama separada oxNum == 0 , de lo contrario verificamos oxNum == 0 && hybrid.equals("sp") , que, por supuesto, no puede ser.
12. Escribimos a una matriz de longitud cero
A veces, IntelliJ IDEA se dará cuenta si escribe en una matriz fuera de su tamaño :
Point3d points[] = new Point3d[0];
13. Comprobación de la longitud después de acceder al índice
Otro problema común con el procedimiento y nuevamente durante el manejo de errores :
public void setParameters(Object[] params) throws CDKException { if (params.length > 1) { throw new CDKException("..."); } if (!(params[0] instanceof Integer)) {
En el caso de una matriz vacía, el autor del código quería salir silenciosamente, pero debido a la verificación, saldría, golpeando ruidosamente ArrayIndexOutOfBoundsException. Obviamente, la orden de verificación está fuera de servicio.
14. Comprobar si hay nulo después del acceso
Y nuevamente, se viola el orden de las acciones, esta vez con nulo :
while (!line.startsWith("frame:") && input.ready() && line != null) { line = input.readLine(); logger.debug(lineNumber++ + ": ", line); }
IDEA escribe esa line != null siempre line != null cierto. Sucede que la verificación es realmente redundante, pero aquí el código parece nulo realmente podría serlo.
15. Disyunción en lugar de conjunción.
La gente a menudo confunde los operadores lógicos AND y OR. El proyecto CDK no es una excepción :
if (rStereo != 4 || pStereo != 4 || rStereo != 3 || pStereo != 3) { ... }
Independientemente de lo que rStereo y pStereo sean iguales, está claro que no pueden ser iguales a cuatro y tres al mismo tiempo, por lo tanto, esta condición siempre es cierta.
16. Nuevamente disyunción en lugar de conjunción
Un error similar , pero atrapado por otro mensaje:
if (getFirstMapping() != null || !getFirstMapping().isEmpty()) { ... }
Podemos llegar al lado derecho solo si getFirstMapping() devuelve null , pero en este caso tenemos garantizada una NullPointerException, de la que IDEA advierte. Por cierto, aquí confiamos en la estabilidad de los resultados del método getFirstMapping() . A veces usamos heurística, pero aquí se analiza directamente la estabilidad. Como la clase es final, el método no se puede anular. return firstSolution.isEmpty() ? null : firstSolution IDEA verifica que su cuerpo return firstSolution.isEmpty() ? null : firstSolution return firstSolution.isEmpty() ? null : firstSolution y determina que la estabilidad se reduce a la estabilidad del método Map#isEmpty , que anteriormente se Map#isEmpty como estable.
17. Jerarquía de interfaces e instancia de
Al verificar que un objeto pertenece a cualquier interfaz, no olvide que las interfaces pueden heredarse entre sí :
if (object instanceof IAtomContainer) { root = convertor.cdkAtomContainerToCMLMolecule((IAtomContainer) object); } else if (object instanceof ICrystal) { root = convertor.cdkCrystalToCMLMolecule((ICrystal) object); } ...
La interfaz ICrystal extiende la interfaz IAtomContainer , por lo que la segunda rama es obviamente inalcanzable: si un cristal viene aquí, caerá en la primera rama.
18. Recorriendo una lista vacía
El autor de este código probablemente no esté muy familiarizado con el lenguaje Java:
List<Integer> posNumList = new ArrayList<Integer>(size); for (int i = 0; i < posNumList.size(); i++) { posNumList.add(i, 0); }
El parámetro de tamaño en ArrayList indica el tamaño inicial de la matriz interna. Esto se utiliza para la optimización con el fin de reducir el número de asignaciones, si sabe de antemano cuántos elementos colocar allí. Sin embargo, de hecho, los elementos de la lista no aparecen y el método size() devuelve 0. Por lo tanto, el siguiente ciclo con un intento de inicializar los elementos de la lista con ceros es completamente inútil.
19. No olvides inicializar los campos.
El analizador verifica de manera especial los constructores, teniendo en cuenta los inicializadores de campo. Gracias a esto, se encontró un error de este tipo :
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];
A pesar del hecho de que los campos son públicos, nadie aquí definitivamente podría acuñarlos e inicializarlos frente al constructor. Por lo tanto, IDEA audazmente emite una advertencia de que acceder a un elemento de matriz generará una NullPointerException.
20. No repetir dos veces
Las condiciones repetidas también ocurren a menudo. Aquí hay un ejemplo :
if (commonAtomCount > vfMCSSize && commonAtomCount > vfMCSSize) { return true; }
Tales errores son insidiosos, porque nunca se sabe, la segunda condición es simplemente superflua, o el autor quería verificar otra cosa. Si esto no se soluciona de inmediato, puede ser difícil resolverlo. Esta es otra razón por la cual el análisis estático debe usarse constantemente.
Informé algunos de estos errores en el rastreador de errores del proyecto . Es curioso que cuando los autores del proyecto corrigieron una parte, ellos mismos usaron el analizador IntelliJ IDEA, encontraron otros problemas sobre los que no escribí y también comenzaron a solucionarlos . Creo que esta es una buena señal: los autores se dieron cuenta de la importancia del análisis estático.