Calidad de código y análisis estático: Error Prone, SpotBugs, PMD y SonarQube

Los tests te dicen si el código hizo lo correcto con las entradas que elegiste. El análisis estático lee el código sin ejecutarlo y razona sobre todos los caminos, incluidos los que ningún test ha recorrido jamás. Esa diferencia es todo el argumento para usar ambos: una suite de tests con una cobertura excelente puede pasar por alto un stream que nunca se cierra en el camino de la excepción, un equals invocado sobre dos arrays, una cadena de formato con un número de marcadores que no cuadra, o una comparación de enteros encapsulados con == que funciona hasta 127 y falla en 128.

La otra razón para ejecutar análisis es económica. Detectar un hallazgo con una herramienta no cuesta nada: llega segundos después de escribir el código y no necesita que nadie esté de acuerdo en que merece un comentario en la revisión. El tiempo de revisión humana es escaso y debería dedicarse a si el diseño es el adecuado, no a si un recurso tiene una fuga, que es el mismo argumento que plantea la guía de estándares de código para automatizar el formato.

Lo que el análisis ve y lo que no puede ver

Los analizadores trabajan sobre el árbol de sintaxis abstracta, el grafo de flujo de control y, en las herramientas más potentes, una aproximación del flujo de datos. Eso les da capacidades que ningún test tiene: pueden considerar una rama que nunca se ha ejecutado, demostrar que un null puede llegar a una desreferencia sea cual sea la entrada, o rastrear datos de usuario contaminados desde un parámetro de la petición hasta una cadena SQL.

También les impone límites permanentes. No saben nada de la intención, así que ningún analizador te dirá nunca que un tipo de IVA es incorrecto. Tienen que aproximar, porque las preguntas de fondo son indecidibles, y aproximar implica falsos positivos (señalar algo que es seguro) o falsos negativos (pasar por alto algo real). Cada herramienta elige un punto de ese espectro, y saber dónde se sitúa te dice cómo tratar sus resultados: Error Prone está ajustado para tener casi cero falsos positivos y por eso puede romper un build, mientras que un escáner de seguridad se inclina deliberadamente por avisar y requiere triaje.

Las familias de herramientas

Meterlo todo bajo la etiqueta de "análisis estático" lleva a los equipos a adoptar herramientas que se solapan y a ahogarse en ellas. Hay cinco trabajos distintos:

Adopta una de cada fila, no tres de la misma. Dos verificadores de estilo con opiniones distintas producen hallazgos contradictorios y una discusión.

Empieza por el compilador

Antes de cualquier herramienta, el JDK ya informa de muchas cosas que la mayoría de los proyectos ignora. Activar los avisos y tratarlos como errores es la mejora de calidad más barata que existe:

<plugin>
  <groupId>org.apache.maven.plugins</groupId>
  <artifactId>maven-compiler-plugin</artifactId>
  <configuration>
    <release>21</release>
    <compilerArgs>
      <arg>-Xlint:all,-serial,-processing</arg>
      <arg>-Werror</arg>
    </compilerArgs>
  </configuration>
</plugin>

-Xlint:all cubre operaciones genéricas no comprobadas, elementos obsoletos, fall-through en los switch, @Override ausentes y más. -Werror hace que rompan el build, que es el único ajuste que funciona: un build que imprime cuatrocientos avisos es un build en el que nadie lee los avisos. En un proyecto existente, activa las categorías de una en una.

Detectores de bugs que merece la pena ejecutar

Error Prone se engancha a javac y analiza con toda la información de tipos durante la compilación. Sus reglas están seleccionadas por su fiabilidad, y por eso muchas son errores por defecto: llamar a Object.equals con tipos incompatibles, un argumento que no encaja con un @FormatMethod, un valor de retorno ignorado cuando no debe ignorarse, una Duration comparada con ==. Como se ejecuta dentro del compilador, es rápido y puede ofrecer correcciones automáticas.

<compilerArgs>
  <arg>-XDcompilePolicy=simple</arg>
  <arg>--should-stop=ifError=FLOW</arg>
  <arg>-Xplugin:ErrorProne -Xep:MissingOverride:ERROR -Xep:UnusedVariable:WARN</arg>
</compilerArgs>

SpotBugs analiza el bytecode compilado y encuentra un conjunto distinto: desreferencias de null en un camino, fugas de recursos, estado interno mutable expuesto a través de un getter, sincronización incoherente, clases Serializable con campos no serializables. El plugin find-sec-bugs añade patrones de inyección y de criptografía. El análisis de bytecode ve lo que produjo el compilador y no lo que tú escribiste, lo que a veces hace que un hallazgo sorprenda hasta que miras el código generado.

PMD trabaja sobre el código fuente y se especializa en reglas de diseño y complejidad (métodos largos, demasiados parámetros, condicionales muy anidados, bloques catch vacíos), además de su detector de copiar y pegar, CPD, que encuentra bloques duplicados entre archivos que ninguna otra herramienta de esta lista detecta.

Checkstyle hace cumplir convenciones, no corrección: nombres, orden de los imports, Javadoc en la API pública, estructura de los archivos. Es la que menos se solapa con las demás y la que más necesita que se pode su conjunto de reglas para ajustarlo al equipo, porque una configuración por defecto se quejará de un montón de cosas que nadie ha decidido que importen.

Hay dos reglas de configuración que valen para todas. Haz que el build falle con las reglas que el equipo ha decidido que importan, porque un informe que nadie abre es un informe que nadie lee. Y elimina de la configuración compartida las reglas con las que el equipo no esté de acuerdo en lugar de suprimirlas una a una en el código: un comentario de supresión es una decisión de un solo archivo que, sin que nadie lo note, acaba convirtiéndose en política.

SonarQube y la gate sobre el código nuevo

SonarQube agrega sus propios analizadores con los informes importados de las herramientas anteriores, guarda el histórico y aplica una quality gate: un conjunto de condiciones que un build supera o no.

La decisión de configuración más importante es que la gate se aplique al código nuevo (las líneas añadidas o modificadas desde una referencia elegida) y no a todo el proyecto. Eso es lo que hace que el análisis sea adoptable en una base de código real. Una gate que exige un 80% de cobertura en un sistema de quince años es inalcanzable y se desactivará en un mes; una gate que exige un 80% de cobertura en las líneas que has tocado esta semana es una petición razonable, y la base de código mejora allí donde se está trabajando.

Una gate viable sobre el código nuevo:

• Cobertura de las líneas nuevas por encima del 80%
• Líneas duplicadas en el código nuevo por debajo del 3%
• Ninguna incidencia nueva calificada como blocker o critical
• Ningún security hotspot nuevo sin revisar
• Calificación global del código nuevo en A

El modelo más reciente de SonarQube clasifica las incidencias según pongan en riesgo un código consistent, intentional, adaptable o responsible, y las asocia a cualidades del software (seguridad, fiabilidad, mantenibilidad) en lugar de la antigua tríada bug/vulnerabilidad/code smell. La correspondencia importa menos que la disciplina de hacer triaje de cada blocker nuevo antes del merge.

Intégrala en el pipeline para que la gate sea una condición de merge y no un panel que alguien visita de vez en cuando:

mvn -B verify sonar:sonar \
    -Dsonar.projectKey=shop-service \
    -Dsonar.qualitygate.wait=true

sonar.qualitygate.wait=true hace que el comando se bloquee hasta conocer el resultado de la gate y que termine con un código distinto de cero si falla, que es lo que la convierte en una gate de verdad. La guía de integración continua explica en qué punto del pipeline va, en relación con las etapas de test.

Métricas, evaluadas con honestidad

El análisis estático produce números, y los números acaban en paneles tanto si significan algo como si no.

La complejidad ciclomática cuenta los caminos independientes de un método. Es una herramienta de cribado aceptable (ordena los métodos por ella y los peores de la base de código suelen estar arriba) y un mal objetivo. Un switch plano sobre veinte valores de un enum puntúa alto y se lee perfectamente; un método con tres bucles anidados y un continue puntúa más bajo y es mucho más difícil de seguir. La complejidad cognitiva se diseñó precisamente para resolver eso, penalizando el anidamiento e ignorando las estructuras abreviadas, y refleja bastante mejor la dificultad que percibe una persona.

El porcentaje de duplicación es útil cuando señala lógica de negocio copiada y pegada, que es donde un bug acaba corregido en dos de tres sitios. Es ruido cuando marca código generado, tests deliberadamente explícitos o boilerplate de DTO.

La deuda técnica expresada como tiempo de remediación ("4 días 2 horas") es una suma aproximada de estimaciones por regla. Trátala como un criterio para ordenar, no como una estimación; nadie ha saneado nunca una base de código en el tiempo que predecía la herramienta. Su tendencia dice más que su valor, y su valor por módulo dice más que el total.

La cobertura se importa en lugar de calcularla el analizador, y arrastra la advertencia de la guía de estrategia de automatización de tests: mide ejecución, no aserciones, y se comporta mal cuando se convierte en objetivo.

Las calificaciones de mantenibilidad, fiabilidad y seguridad comprimen el número de incidencias en una letra. Comunican bien hacia fuera del equipo y ocultan todo lo que necesita un especialista, así que úsalas para informar y la lista de incidencias para trabajar.

Análisis orientado a la seguridad

Hay dos análisis distintos que deberían estar en todo pipeline.

El SAST busca patrones explotables en tu propio código: SQL construido concatenando cadenas, un comando ejecutado a partir de la entrada del usuario, path traversal, hashing débil, una credencial escrita en el código, una verificación de certificados desactivada, parsing de XML con las entidades externas habilitadas. Los motores más sólidos hacen taint tracking (siguen los datos desde una fuente no confiable hasta un punto de uso peligroso), y por eso pueden distinguir un SQL concatenado que usa una constante de uno que usa un parámetro de la petición.

El SCA (análisis de dependencias) compara tus librerías con bases de datos de vulnerabilidades. En la práctica encuentra más riesgo real que el SAST, porque la mayoría de las aplicaciones incluyen mucho más código de terceros que propio. Las reglas operativas que lo hacen llevadero: bloquear solo ante hallazgos nuevos de severidad alta, para que la cola no bloquee cada build; vigilar las dependencias transitivas además de las directas; y registrar los riesgos aceptados con fecha de caducidad en lugar de con una supresión permanente.

La detección de secretos es un tercer análisis, barato y que merece la pena: revisar el diff en busca de claves privadas, tokens y cadenas de conexión. Tiene que ejecutarse en un hook pre-receive o en una etapa temprana del pipeline, porque en cuanto un secreto llega al histórico compartido, el único remedio es rotar la credencial.

Ninguno de ellos sustituye a una revisión de seguridad ni a un test de penetración, y un escaneo limpio es un mínimo, no una garantía. Lo que sí hacen de forma fiable es impedir que los patrones conocidos lleguen a main.

Adoptar el análisis en una base de código legacy

La primera ejecución completa sobre una base de código consolidada informa de miles de hallazgos. Esa cifra es la razón por la que fracasan la mayoría de los intentos de adopción: un backlog tan grande no es accionable, así que se ignora, y una vez ignorado, la herramienta es solo ruido.

Una secuencia que funciona:

1. Congela la línea base

   Ejecuta el análisis, registra el estado actual y configura la gate para que se aplique solo al código nuevo. El objetivo inmediato es que la cifra deje de crecer, no que baje.

2. Activa en todas partes un puñado de reglas seguras

   Elige entre diez y quince reglas de corrección de alta confianza (fugas de recursos, valores de retorno ignorados, errores de comparación, bloques catch vacíos) y haz que rompan el build en toda la base de código. Corrige las infracciones existentes solo de esas. Suele ser uno o dos días de trabajo y elimina defectos reales.

3. Automatiza el formato en un solo commit

   Aplica el formateador a toda la base de código en un único commit dedicado solo al formato, añade su hash al archivo ignore-revs para que el histórico siga siendo legible y, a partir de ahí, hazlo obligatorio. Así desaparece para siempre una categoría entera de hallazgos.

4. Corrige sobre la marcha

   Quien modifica un archivo corrige los hallazgos de la parte que ha tocado. En un año, los archivos que importan, los que cambian activamente, quedan limpios, y los que están dormidos siguen como estaban, que es el reparto correcto del esfuerzo.

5. Añade reglas poco a poco

   Una categoría de reglas nueva por iteración, hablada de antemano y acordada por el equipo. Una regla impuesta sin discusión acaba suprimida en lugar de cumplida.

6. Revisa las supresiones

   Una vez por trimestre, lista todas las supresiones y exclusiones. Algunas recogen una decisión que merece la pena mantener; otras fueron un apaño para un falso positivo de una versión concreta que ya se ha corregido.

El principio de fondo es que el análisis tiene que formar parte del ciclo en el que el desarrollador ya está. Un hallazgo que aparece en el IDE mientras escribes cuesta segundos; el mismo hallazgo en un informe nocturno cuesta un cambio de contexto, y en una auditoría trimestral cuesta una reunión. Configura los plugins del IDE a partir de los mismos archivos de configuración compartidos que usa el build, para que los resultados en local y en el pipeline coincidan: que no coincidan es la forma más rápida de que la gente desconfíe de todo.

Puntos clave

• El análisis estático razona sobre caminos que ningún test ejercita, y los tests verifican una intención que ningún analizador puede conocer; cubren clases de defectos distintas y ninguno sustituye al otro.
• Elige una herramienta por familia (formateador, verificador de estilo, detector de bugs, SAST, escáner de dependencias) y configura el conjunto de reglas compartido de forma deliberada en lugar de aceptar los valores por defecto.
• Empieza por el compilador: -Xlint:all -Werror no cuesta nada y detecta una cantidad sorprendente de problemas.
• Aplica la gate al código nuevo, no a toda la base de código; esa sola decisión es lo que hace adoptable el análisis en un sistema legacy.
• Trata las cifras de complejidad, duplicación y deuda técnica como herramientas de cribado y tendencias, nunca como objetivos, y prefiere la complejidad cognitiva para las cuestiones de legibilidad.
• El análisis de dependencias suele encontrar más riesgo real que el análisis de tu propio código, así que bloquea ante hallazgos nuevos de severidad alta y pon fecha de caducidad a los riesgos aceptados.