👲🏾 👨🏻‍🎨 📛 PVS-Studio para Java bajo el capó: desarrollo de diagnósticos 🗿 🎌 🙏

Para variar, hoy le contaremos un poco sobre el proceso de desarrollo y finalización de reglas de diagnóstico para PVS-Studio Java. Veamos por qué los disparadores del analizador antiguo no flotan demasiado de una versión a otra, y los nuevos no son demasiado locos. Y echaremos a perder un poco más "cuáles son los planes de los javists" y mostraremos un par de errores hermosos (y no tanto) encontrados con la ayuda de los diagnósticos de la próxima versión.

Proceso de desarrollo de diagnóstico y autocomprobación

Naturalmente, cada nueva regla de diagnóstico comienza con una idea. Y dado que el analizador de Java es la dirección más joven en el desarrollo de PVS-Studio, básicamente robamos estas ideas de los departamentos de C / C ++ y C #. Pero no todo es tan malo: también agregamos reglas que fueron inventadas por nosotros mismos (incluso por los usuarios - ¡gracias!), Para que luego los mismos departamentos nos las roben. El ciclo, como dicen.

La misma implementación de las reglas en el código en la mayoría de los casos resulta ser una tarea bastante compleja. Creas un archivo con varios ejemplos sintéticos, marcas con tus manos dónde deberían estar los errores y con un depurador listo, recorres el árbol de sintaxis hasta aburrirte y cubrir todos los casos inventados. A veces, las reglas resultan absurdamente simples (por ejemplo, V6063consiste literalmente en unas pocas líneas), y a veces hay que pensar lo suficiente en la lógica.

Sin embargo, este es solo el comienzo del proceso. Como saben, no nos gustan especialmente los ejemplos sintéticos, ya que reflejan muy mal el tipo de disparadores del analizador en proyectos reales. Por cierto, una parte importante de estos ejemplos en nuestras pruebas unitarias se extraen de proyectos reales; es casi imposible inventar todos los casos posibles por su cuenta. Y las pruebas unitarias también nos permiten no perder desencadenantes en ejemplos de la documentación. Hubo precedentes, sí, solo shh.

Por lo tanto, los aspectos positivos en los proyectos reales deben encontrarse primero de alguna manera. Y también necesita verificar de alguna manera que:

La regla no caerá en la locura del código abierto, donde las soluciones "interesantes" son comunes;
( - , );
data-flow ( ) - ;
open-source ;
over 9000%;
"" , ;
.

En general, aquí, como un caballero a caballo (un poco cojeando, pero estamos trabajando en ello), SelfTester pasa a primer plano. Su principal y única tarea es verificar automáticamente un montón de proyectos y mostrar qué activadores se han agregado, desaparecido o cambiado en relación con la "referencia" en el sistema de control de versiones. Proporciona diffs para el informe del analizador y muestra el código correspondiente en los proyectos, en resumen. Actualmente SelfTester para Java está probando 62 proyectos de código abierto de versiones barbudas, entre los que se encuentran, por ejemplo, DBeaver, Hibernate y Spring. Una ejecución completa de todos los proyectos lleva de 2 a 2,5 horas, lo que sin duda es doloroso, pero no se puede hacer nada.

En la captura de pantalla anterior, los proyectos "verdes" son aquellos en los que nada ha cambiado. Cada diferencia en proyectos "rojos" se visualiza manualmente y, si es correcta, se confirma con el mismo botón "Aprobar". Por cierto, el kit de distribución del analizador solo se compilará si SelfTester da un resultado puramente ecológico. En general, así es como mantenemos la consistencia de resultados entre diferentes versiones.

Además de mantener la consistencia de los resultados, SelfTester nos permite deshacernos de una gran cantidad de falsos positivos incluso antes del lanzamiento de los diagnósticos. Un patrón de desarrollo típico se ve así:

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

Afortunadamente, las ejecuciones completas de SelfTester son bastante raras y no tiene que esperar "2-2.5 horas" con mucha frecuencia. De vez en cuando, la suerte pasa por alto y aparecen desencadenantes en grandes proyectos como Sakai y Apache Hive: es hora de tomar café, tomar café y tomar café. También puedes estudiar la documentación, pero esto no es para todos.

"¿Por qué necesitamos pruebas unitarias, ya que existe una herramienta tan mágica?"

Y luego las pruebas son significativamente más rápidas. Unos minutos, y ya hay un resultado. También le permiten ver exactamente qué parte de la regla se cayó. Y, además, no siempre se capturan todos los disparos permitidos de cualquier regla en los proyectos SelfTester, pero también se debe verificar su operatividad.

Nuevos problemas en viejos conocidos.

Inicialmente, esta sección del artículo comenzaba con las palabras "Las versiones de proyectos en SelfTester son bastante antiguas, por lo que la mayoría de los errores presentados probablemente se hayan corregido". Sin embargo, cuando decidí asegurarme de esto, me esperaba una sorpresa. Todos y cada uno de los errores permanecieron en su lugar. Todo. Esto significa dos cosas:

Estos errores no son muy críticos para que la aplicación funcione. Muchos de ellos, por cierto, están en el código de prueba, y las pruebas incorrectas difícilmente pueden considerarse consistentes.
Estos errores se encuentran en archivos de grandes proyectos que se utilizan con poca frecuencia, a los que los desarrolladores apenas acuden. Debido a esto, el código incorrecto está condenado a permanecer allí durante mucho tiempo: muy probablemente, hasta que se produzca algún error crítico debido a él.

Para aquellos que deseen profundizar, habrá enlaces a versiones específicas que estamos revisando.

PD: Lo anterior no significa que el análisis estático solo detecta errores inofensivos en el código no utilizado. Verificamos las versiones de lanzamiento (y casi lanzamiento) de los proyectos, en los que los desarrolladores y probadores (y, a veces, desafortunadamente, los usuarios) encontraron los errores más relevantes a mano, lo cual es largo, costoso y doloroso. Puede leer más sobre esto en nuestro artículo " Errores que el análisis de código estático no puede encontrar porque no se utiliza ".

Apache Dubbo y menú en blanco

Diagnóstico de GitHub " V6080 Considere la posibilidad de buscar errores de impresión. Es posible que una variable asignada deba comprobarse en la siguiente condición " ya se publicó en la versión 7.08, pero aún no ha aparecido en nuestros artículos, por lo que es hora de corregirlo.

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);
  }
  ....
}

Un ejemplo clásico de un diccionario de "colección de claves" y un error tipográfico igualmente clásico. El desarrollador quería crear una colección correspondiente a la clave, si aún no existe, pero confundió el nombre de la variable y obtuvo no solo la operación incorrecta del método, sino también una NullPointerException en la última línea. Para Java 8 y versiones posteriores, para implementar dichos diccionarios, debe usar el método 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 y bloqueo de doble verificación

GitHub

Uno de los diagnósticos que se incluirá en la próxima versión es verificar la implementación correcta del patrón de "bloqueo de doble verificación". Glassfish resultó ser el poseedor del récord de detecciones de proyectos SelfTester: en total, PVS-Studio encontró 10 áreas problemáticas en el proyecto usando esta regla. Invito al lector a divertirse y buscar dos de ellos en el fragmento de código a continuación. Para obtener ayuda, consulte la documentación: " V6082 Bloqueo de doble verificación no seguro ". Bueno, o, si no quieres, al final del artículo.

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 y flujo de datos

GitHub

Mejorar los diagnósticos no se trata solo de cambiar directamente su código para detectar más lugares sospechosos o eliminar falsos positivos. El marcado manual de métodos para el flujo de datos también juega un papel importante en el desarrollo del analizador; por ejemplo, puede escribir que tal o cual método de biblioteca siempre devuelve un valor no nulo. Al escribir un nuevo diagnóstico, descubrimos accidentalmente que el método Map # clear () no estaba marcado . Aparte del código obviamente estúpido de que la " Colección V6009 está vacía. La llamada a la función 'borrar' no tiene sentido ", los diagnósticos comenzaron a captar , pudimos encontrar un gran error tipográfico.

MetricRepositoryRule.java:90

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

A primera vista, volver a borrar el diccionario no es un error. E incluso pensaríamos que esta es una línea duplicada al azar, si nuestra mirada no hubiera bajado un poco más, literalmente al siguiente método.

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

Exactamente. La clase tiene dos campos con nombres similares metricsById y metricsByKey . Estoy seguro de que en el método after el desarrollador quería borrar ambos diccionarios, pero el autocompletado falló o ingresó por inercia el mismo nombre. Por lo tanto, los dos diccionarios que contienen datos relacionados no estarán sincronizados después de la llamada a after .

Sakai y colecciones vacías

GitHub

Otro nuevo diagnóstico que se incluirá en la próxima versión es " V6084 Retorno sospechoso de una colección siempre vacía ". Es bastante fácil olvidarse de agregar elementos a la colección, especialmente cuando cada elemento debe inicializarse primero. Por experiencia personal, estos errores no suelen provocar un bloqueo de la aplicación, sino un comportamiento extraño o la ausencia de cualquier funcionalidad.

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;
}

Por cierto, la misma clase contiene métodos muy similares sin el mismo error. Por ejemplo:

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;
}

Planes para el futuro

Aparte de varias cosas internas no muy interesantes, estamos pensando en agregar diagnósticos para Spring Framework al analizador de Java. No es solo el pan principal para los javistas, sino que también contiene muchos momentos no obvios en los que uno puede tropezar. Todavía no estamos muy seguros de qué forma aparecerán estos diagnósticos, cuándo sucederá y si sucederá en absoluto. Pero estamos seguros de que necesitamos ideas para ellos y proyectos de código abierto que utilicen Spring para SelfTester. Entonces, si tienes algo en mente, sugiérelo (en comentarios o mensajes privados, ¡también puedes)! Y cuanto más de esta bondad recopilemos, más prioridad se le asignará.

Y finalmente, hay errores en la implementación de bloqueo de Glassfish comprobada dos veces:

El campo no se declara 'volátil'.
El objeto se publica primero y luego se inicializa.

Por qué todo esto es malo, nuevamente, puede verlo en la documentación .

Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción: Nikita Lazeba. Bajo el capó de PVS-Studio para Java: cómo desarrollamos diagnósticos .

PVS-Studio para Java bajo el capó: desarrollo de diagnósticos