Pylint: una prueba detallada del analizador de código

Cuando Luke trabajó con Flake8 y mantuvo un ojo en Pylint, tuvo la impresión de que el 95% de los errores generados por Pylint eran falsos. Otros desarrolladores tenían una experiencia diferente al interactuar con estos analizadores, por lo que Luke decidió investigar la situación en detalle y estudiar su trabajo en 11 mil líneas de su código. Además, elogió los beneficios de Pylint, viéndolo como una adición a Flake8.



Luke ( Luke Plant ): uno de los desarrolladores británicos, en cuyo artículo con el análisis de analizadores de código populares nos encontramos recientemente. Linters estudia el código, lo ayuda a encontrar errores y lo hace estilísticamente coherente con los estándares y el código que escriben los desarrolladores de su equipo. Los más comunes son Pylint y Flake8. Estamos en ID de líderTambién los usamos, así que felizmente tradujimos su artículo. Esperamos que enriquezca su experiencia con estas herramientas.

Configuraciones iniciales y base de prueba


Para este experimento, tomé parte del código de uno de mis proyectos y lancé Pylint con configuraciones básicas. Luego trató de analizar el resultado: qué advertencias fueron útiles y cuáles fueron falsas.

Un poco de ayuda sobre el proyecto del que se tomó el código:

  • Una aplicación normal escrita en Django (es decir, dentro del mismo Python). Django tiene sus propias peculiaridades y, como marco, tiene sus limitaciones, pero le permite escribir código Python normal. Otras bibliotecas que usan plantillas (funciones de devolución de llamada o plantillas de diseño de Método de plantilla) también tienen algunos de sus inconvenientes como marco.
  • Consiste en 22,000 líneas de código. Aproximadamente 11,000 líneas pasaron a través de Pylint (9,000 si se omiten huecos). Esta parte del proyecto consistió principalmente en vistas y código de prueba.
  • Para analizar el código de este proyecto, ya utilicé Flake8, ya que procesé todos los errores recibidos. El objetivo de este experimento fue evaluar los beneficios de Pylint como una adición a Flake8.
  • El proyecto tiene una buena cobertura de prueba del código, pero como soy su único autor, no tuve la oportunidad de utilizar la revisión por pares.

Espero que este análisis sea útil para que otros desarrolladores decidan usar Pylint en la pila para verificar la calidad del código. Supongo que si ya está usando algo como Pylint, lo usará sistemáticamente para el control de calidad necesario del código con el fin de minimizar los problemas.
Entonces, Pylint ha emitido 1650 reclamos a mi código.

A continuación, agrupé todos los comentarios del analizador por tipo y les di mis comentarios. Si necesita una descripción detallada de estos mensajes, eche un vistazo a la sección correspondiente de la lista de funciones de Pylint.

Loco


Pylint encontró exactamente un error en mi código. Considero un error como un error que ocurre o puede surgir durante la operación del programa. En este caso, utilicé excepciones, broad-except.es decir except Exception, no solo excepto, que Flake8 atrapa. Esto daría lugar a un comportamiento en tiempo de ejecución con algunas excepciones. Si alguna vez apareció este error en tiempo de ejecución (no por el hecho de que aparece), el comportamiento incorrecto del código no tendría consecuencias graves, aunque ...

Total: 1

Útil


Además de ese error, Pylint encontró algunos más que categoricé como útiles. El código no cayó de ellos, pero podría haber problemas durante la refactorización y, en principio, errores en el futuro al expandir la lista de características y admitir el código.

Siete de ellos fueron too-many-locals/ too-many-branches/ too-many-local-variables. Pertenecían a tres partes de mi código que estaban mal estructuradas. Sería bueno pensar en la estructura, y estoy seguro de que podría hacerlo mejor.

Otros errores:

  • unused-argument× 3: uno de ellos era realmente una jamba, y el código se ejecutaba correctamente al azar. Los otros dos argumentos innecesarios y no utilizados conducirían a problemas en el futuro si los usara.
  • redefined-builtin × 2
  • dangerous-default-value × 2: no errores, porque nunca usé los valores predeterminados, pero sería bueno arreglar esto por si acaso.
  • stop-iteration-return × 1: aquí aprendí algo nuevo para mí, nunca lo habría encontrado yo mismo.
  • no-self-argument × 1

Total: 16

Ediciones cosméticas


Prestaría menos atención a estas cosas. Son insignificantes o poco probables. Por otro lado, su corrección no será superflua. Algunos de ellos son polémicos estilísticos. Hablé sobre algunas jambas similares en otras secciones, pero las que se enumeran aquí también son adecuadas para este contexto. Usando Pylint regularmente, corregiría estos "defectos", pero en la mayoría de los casos no me preocuparía por ellos.

invalid-name× 192

Éstos eran básicamente nombres de variables de una letra. Usado en aquellos contextos donde no daba miedo, por ejemplo:


o


Muchos estaban en el código de prueba:

  • len-as-condition × 20
  • useless-object-inheritance × 16 (legado de Python 2)
  • no-else-return × 11
  • no-else-raise × 1
  • bad-continuation × 6
  • redefined-builtin × 4
  • inconsistent-return-statements × 1
  • consider-using-set-comprehension × 1
  • chained-comparison × 1

TOTAL: 252

Inútil


Esto es cuando tuve razones para escribir el código de la manera en que lo escribí, incluso si en algunos lugares parece inusual. Y, en mi opinión, es mejor dejarlo en esta forma, aunque algunos no estarán de acuerdo o preferirán un estilo diferente de código de escritura que podría evitar problemas.

  • too-many-ancestors × 76

En el código de prueba, utilicé muchas clases de impurezas para colocar utilidades o talones.

  • unused-variable × 43

Se encontró casi todo el tiempo en el código de prueba, donde rompí el récord:


... y no usó ninguno de los elementos. Hay varias formas de evitar que Pylint reporte errores (por ejemplo, dar nombres unused). Pero si lo deja en la forma en que escribí, será legible y las personas (incluido yo mismo) podrán entenderlo y apoyarlo.

  • invalid-name × 26

Estos fueron casos en los que asigné nombres apropiados en el contexto, pero aquellos que no cumplían con los estándares de nombres de Pylint. Por ejemplo, db (esta es una abreviatura común para la base de datos) y algunos otros nombres no estándar, que, en mi opinión, eran más comprensibles. Pero puede que no estés de acuerdo conmigo.

  • redefined-outer-name × 16

A veces, un nombre de variable se escribe correctamente para contextos internos y externos. Y nunca tendrá que usar un nombre externo de un contexto interno.

  • too-few-public-methods × 14

Los ejemplos incluyen clases con datos creados usando attrs , donde no hay métodos públicos, y una clase que implementa la interfaz del diccionario, pero que es necesaria para garantizar que el método funcione correctamente__getitem__

  • no-self-use × 12

Aparecieron en el código de prueba, donde agregué métodos intencionalmente a la clase base sin un parámetro self, porque de esta manera era más conveniente importarlos y ponerlos a disposición para el caso de prueba. Algunos de ellos incluso envueltos en funciones separadas.

  • attribute-defined-outside-init × 10

En estos casos, había buenas razones para escribir el código tal como está. Básicamente, estos errores ocurrieron en el código de prueba.

  • too-many-locals× 6, too-many-return-statements× 6, too-many-branches× 2, too-many-statements× 2

Sí, estas características fueron demasiado largas. Pero al mirarlos, no vi ninguna buena manera de limpiarlos y mejorarlos. Una de las características era simple, aunque larga. Tenía una estructura muy clara y no estaba escrita de forma torcida, y cualquier forma de reducirla que pudiera pensar incluiría capas inútiles de funciones innecesarias o inconvenientes.

  • arguments-differ × 6

Esto se debe principalmente al uso de * args y ** kwargs en el método anulado, que le permite protegerse de los cambios en la firma de métodos de bibliotecas de terceros (pero en algunos casos este mensaje puede indicar errores reales).

  • ungrouped-imports × 4

Ya uso isort para importar

  • fixme × 4

Sí, hay varias cosas que deben corregirse, pero en este momento no quiero arreglarlas.

  • duplicate-code × 3

A veces, utiliza una pequeña cantidad de código repetitivo, que es simplemente necesario, y cuando no hay mucha lógica real en el cuerpo de la función, esta advertencia vuela.

  • broad-except × 2
  • abstract-method × 2
  • redefined-builtin × 2
  • too-many-lines × 1

Traté de averiguar de qué manera natural romper este módulo, pero no pude. Este es un ejemplo en el que puede ver que la interfaz es la herramienta incorrecta. Si tengo un módulo con 980 líneas de código, y agrego otras 30, cruzo el límite de 1000 líneas, y las notificaciones del linter no me ayudarán aquí. Si 980 líneas están bien, entonces ¿por qué está mal 1010? No quiero refactorizar este módulo, pero quiero que la interfaz no produzca errores. La única solución en este momento que veo es hacer de alguna manera que el linter esté en silencio, y esto contradice el objetivo final.

  • pointless-statement × 1
  • expression-not-assigned × 1
  • cyclic-import × 1

Descubrimos el ciclo moviendo sus partes a una de las funciones. No pude encontrar una mejor manera de estructurar el código basado en restricciones.

  • unused-import × 1

Ya agregué # NOQAal usar Flake8 para que este error no aparezca.

  • too-many-public-methods × 1

Si en mi clase de examen hay 35 exámenes en lugar de 20 regulados, ¿es realmente un problema?

  • too-many-arguments × 1

Total: 243

Incapaz de arreglar


Esta categoría refleja errores que no puedo corregir, incluso si quisiera, debido a restricciones externas, como la necesidad de devolver o pasar clases a una biblioteca o marco de terceros que debe cumplir ciertos requisitos.

  • unused-argument × 21
  • invalid-name × 13
  • protected-access × 3

Acceso incluido a "objetos internos documentados", como sys._getframeen stdlib y Django Model._meta.

  • too-few-public-methods × 3
  • too-many-arguments × 2
  • wrong-import-position × 2
  • attribute-defined-outside-init × 1
  • too-many-ancestors × 1

Total: 46

Mensajes falsos


Cosas en las que Pylint está claramente equivocado. En este caso, estos no son errores de Pylint: el hecho es que Python es dinámico, y Pylint está tratando de descubrir cosas que no se pueden hacer de manera perfecta o confiable.

  • no-member × 395

Asociado con varias clases base: de Django y las que yo mismo creé. Pylint no pudo detectar la variable debido al dinamismo / metaprogramación.

Muchos errores ocurrieron debido a la estructura del código de prueba (usé una plantilla de django-functest, que en algunos casos podría corregirse agregando clases base adicionales usando métodos "abstractos" que llaman NotImplementedError) o, posiblemente, renombrando muchas clases de prueba (I No lo hice, porque en algunos casos sería confuso).

  • invalid-name × 52

El problema surgió principalmente porque Pylint aplicó la regla constante PEP8 , considerando que cada nombre de nivel superior definido con =es una "constante". Definir exactamente lo que queremos decir con una constante es más difícil de lo que parece, pero esto no se aplica a algunas cosas que son inherentemente constantes, por ejemplo, las funciones. Además, la regla no debe aplicarse a formas menos familiares de crear funciones, por ejemplo:


Algunos ejemplos son discutibles debido a la falta de definición de lo que es una constante. Por ejemplo, ¿una instancia de una clase definida en el nivel del módulo debe considerarse constante, que puede o no tener un estado mutable? Por ejemplo, en esta expresión:


  • no-self-use × 23

Pylint declaró incorrectamente que el Método podría ser una función para muchos casos en los que uso la herencia para realizar diversas implementaciones, respectivamente, no puedo convertirlos en funciones.

  • protected-access × 5

Pylint evaluó incorrectamente quién era el "propietario" (el fragmento de código actual crea el atributo protegido del objeto y lo usa localmente, pero Pylint no ve esto).

  • no-name-in-module × 1
  • import-error × 1
  • pointless-statement × 1

Esta declaración realmente produjo el resultado:


Utilicé esto para causar intencionalmente un error inusual que es poco probable que se encuentre en las pruebas. No culpo a Pylint por no reconocerlo ...

Total: 477

Total parcial


Todavía no estamos en la línea de meta, pero es hora de agrupar nuestros resultados:

  1. "Bueno" - bloques "Errores" y "Utilidades" - aquí Pylint definitivamente ayudó: 17.
  2. "Neutral" - "Cambios cosméticos" - un beneficio menor de Pylint, los errores no causarán daños: 252.
  3. "Malo" - "Inútil", "No se puede arreglar", "Inexactitudes" - donde Pylint quiere cambios en el código, donde no es necesario. Incluyendo dónde no se pueden hacer ediciones debido a dependencias externas o donde Pylint simplemente analizó incorrectamente el código: 766.

La relación de bueno a malo es muy pequeña. Si Pylint fuera mi colega ayudando a revisar el código, le rogaría que se fuera.

Para eliminar las notificaciones falsas, puede ahogar la clase de error saliente (que luego aumentará la inutilidad de usar Pylint) o agregar comentarios especiales al código. No quiero hacer esto último porque:

  1. ¡Toma tiempo!
  2. No me gusta la acumulación de comentarios que existen para silenciar el linter.

Con mucho gusto agregaré estos pseudo-comentarios cuando haya una ventaja definitiva del linter. Además, estoy ansioso por los comentarios, por lo que mi resaltado de sintaxis los muestra vívidamente: como se recomienda en este artículo . Sin embargo, en algunos lugares ya he agregado # comentarios NOQApara amortiguar Flake8, pero con ellos para una sección, puede agregar solo cinco códigos de error.

Docstrings


Los problemas restantes que descubrió Pylint eran líneas de documentación faltantes. Los puse en una categoría separada porque:

  1. Son muy controvertidos y es posible que tenga una política completamente diferente con respecto a tales cosas.
  2. No tuve tiempo de analizarlos todos.

En total, Pylint descubrió 620 dockstrings faltantes (en módulos, funciones, métodos de clase). Pero en muchos casos, tenía razón. Por ejemplo:

  • Cuando todo está claro por el nombre. Por ejemplo:
  • Cuando una cadena de documentación ya está definida, por ejemplo, si implemento la interfaz del enrutador de la base de datos de Django . Agregar su propia línea en este caso puede ser peligroso.

En otros casos, estas líneas de descripciones no dañarían mi código. En aproximadamente el 15-30% de los casos encontrados por Pylint, habría pensado: "Sí, necesito agregar una cadena de documentación aquí, gracias Pylint por el recordatorio".

En general, no me gustan las herramientas que hacen que agregar cadenas de acoplamiento en todas partes y en todas partes, porque creo que en este caso resultarán malas. Es mejor no escribirlos en absoluto. Una situación similar con malos comentarios:

  • leerlos es una pérdida de tiempo: no tienen información adicional o contienen información incorrecta,
  • ayudan a resaltar inconscientemente las cadenas de documentos en el texto, porque contienen información útil (si las escribe en el caso).

Las advertencias sobre líneas de documentación faltantes son molestas: para eliminarlas, debe agregar comentarios por separado, lo que lleva aproximadamente la misma cantidad de tiempo que agregar el acoplamiento en sí. Además, todo crea ruido visual. Como resultado, obtendrá líneas de documentación que no son necesarias o útiles.

Conclusión


Creo que mis suposiciones iniciales sobre la inutilidad de Pylint resultaron ser correctas (en el contexto de la base de código para la que uso Flake8). Para que pueda usarlo, el indicador que refleja el número de falsos positivos debe ser menor.

Además de crear ruido visual, se requiere tiempo adicional para clasificar o filtrar notificaciones falsas, y no quisiera agregar nada de esto al proyecto. Los desarrolladores junior serán más humildes para realizar ediciones para eliminar los comentarios de Pylint. Pero esto los llevará a romper el código de trabajo, sin darse cuenta de que Pylint está mal, o al hecho de que, como resultado, tendrá que refactorizar mucho para que Pylint pueda entender su código correctamente.

Si usa Pylint en un proyecto desde el principio o en un proyecto donde no hay dependencias externas, creo que tendrá una opinión diferente y la cantidad de notificaciones falsas será menor. Sin embargo, esto puede generar costos de tiempo adicionales.

Otro enfoque es usar Pylint para un número limitado de tipos de errores. Sin embargo, solo hubo unas pocas, cuyas respuestas resultaron ser correctas o extremadamente raramente falsas (en términos relativos y absolutos). Entre ellos se encuentran: dangerous-default-value, stop-iteration-return, broad-exception, useless-object-inheritance.

En cualquier caso, espero que este artículo te haya ayudado cuando consideres si usar Pylint o en una disputa con colegas.

All Articles