gcm1001 / TFG-CeniehAriadne

CENIEH & Ariadne+ project.
GNU General Public License v3.0
3 stars 2 forks source link

Mantenimiento y calidad de código Codacy #52

Closed gcm1001 closed 4 years ago

gcm1001 commented 4 years ago

El objetivo de esta Issue será mejorar la calidad del código de forma que obtenga, al menos, una evaluación de B en Codacy.

gcm1001 commented 4 years ago

Informe de errores tratados

A continuación expondré cada uno de las tareas de mantenimiento que he llevado a cabo siguiendo como guía las issue indicadas por Codacy.

Dockerfile

Issue: La instrucción Docker MAINTAINER está obsoleta. Solución: He optado por eliminar la etiqueta MAINTANER de mi Dockerfile.

Issue: Usar la instrucción COPY en vez de ADD para ficheros y carpetas. Solución: Se recomienda usar COPY por encima de ADD para trasladar ficheros desde el origen a la imagen. Esto se debe a que ADD tiene otras funciones adicionales que, si no las vamos a utilizar, no tiene sentido utilizarla.

CSS

Issues: Se esperaba...

  • indentaciones de 2 espacios.
  • Colores en hexadecimal en minúscula.
  • Saltos de linea en determinadas ocasiones.
  • Ceros a la izquierda en números decimales.
  • Espacio después de los ':' de cada término.
  • No indicar la magnitud cuando el valor es 0.
  • Espacios entre selectores.
  • Una línea de separación entre reglas.

Solución: He seguido al pie de la letra las recomendaciones de estilo indicadas anteriormente. Todas ellas están relacionadas con el estilo del código.

PHP

Issue: Declaración de variables que no se llegan a utilizar. Solución: Muchas de estas variables eran necesarias ya que en PHP la forma más eficiente de recorrer las keys de un array es mediante un foreach, el cual declara dos variables, una para las keys y otra para los valores asociados a dichas key. Cuando solo me interesa recoger las primeras la variable de los valores se queda sin utilizar. Dejando de lado estas variables, he eliminado todas aquellas que, por despiste, declaraba y no utilizaba.

Issue: Acceso directo a variables superglobales como $_FILES, $_GET, $_POST, $_SERVER, etc. Solución: Para las variables GET y POST he podido hacer uso del framework (Zend) para acceder a ellas ya que dispone del objeto Zend_Controller_Request_Http para gestionar dichas entradas, sin embargo, para las demás variables, no existe forma de acceder a ellas que no sea de forma directa.

Issue: Tamaño de las variables (demasiado largo o demasiado corto). Solución: He modificado el nombre de aquellas variables cuyo tamaño no se correspondía con el ideal (3 < t_ideal < 20) exceptuando aquellas que no pueden cambiadas como, por ejemplo, las variables utilizadas para recoger el identificador de las tablas ($id) en los modelos.

Issue: Evitar el uso de la expresión else. Solución: He suprimido el uso de esta expresión siempre que me ha sido posible, sin embargo, existen algunos escenarios en los que es necesaria.

Issue: Seguridad: Construcción del código. Solución: para imprimir variables en las páginas web dinámicas utilizaba el comando echo dentro de la etiqueta <?php. Esta práctica no es recomendable ya que PHP permite a través de la etiqueta <?= imprimir variables de la misma forma que lo hace echo, haciendo así el código mucho más elegante.

Problemas encontrados

Estoy intentando solucionar los problema de seguridad relacionados con escapar código html. A pesar de utilizar el método de escape por defecto (htmlspecialchars()), me sigue saltando el mismo error ("Security: Escape Output"). Me he documentado y parece ser que solo reconoce los métodos de escape utilizados por Wordpress, que son:

¿Debería crear un método llamado esc_html() que a su vez llame a htmlspecialchars() o desactivo el filtro que genera el error?

Este error provoca más del 50% de las issues.

gcm1001 commented 4 years ago

Informe de errores solucionados

PHP

Issue: Complejidad elevada en determinados métodos. Solución: Fragmentar las tareas de aquellos métodos que hayan sido catalogados como "altamente complejos" siempre que me ha sido posible. En ocasiones, esta fragmentación no ha sido posible ya que afectaba negativamente en otros factores (cómputo, estructuración del código, etc.).

Issue: Métodos demasiado extensos. Solución: He llevado a cabo el mismo proceso utilizado para resolver la issue anterior.

Issue: Utilizar el método echo para imprimir código en el lado del modelo. Solución: En el informe anterior traté este "error" únicamente en el lado de las vistas (views) ya que no sabía que se podía utilizar también la etiqueta <?= desde los modelos (models). Tras descubrir que si se podía, he llevado a cabo la misma metódica descrita en el informe anterior.

Issue: Acceso directo a la variable superglobal $_SERVER. Solución: haciendo uso del método filter_input() de PHP he conseguido acceder a la variable _$SERVER de forma "indirecta", tras aplicar sobre ella un filtro de seguridad.

CSS

Issues:

  • Estructura jerárquica de alguna de las reglas inadecuada.
  • Espacios en blancos inesperados.
  • Saltos de línea inesperados.
  • Saltos de línea esperados.
  • Faltan por declarar propiedades compatibles con Safari y Google Chrome.

Solución: He seguido al pie de la letra las recomendaciones de estilo indicadas anteriormente. Todas ellas están relacionadas con el estilo del código.

Comentarios adicionales

Codacy Badge

Actualmente la rama develop ya cuenta con el grado de calidad B, sin embargo, sigue existiendo un número alto de errores relacionados únicamente con el escape de código comentado en la issue anterior, en total son 465.

Estos no pueden ser considerados como tal ya que sí que escapo correctamente el código con el método html_escape() proporcionado por Omeka. Este es igual de efectivo que los recomendados por el sniffer (que son exclusivos de WordPress y, por lo tanto, no puedo utilizarlos).

El problema está en que el revisor de código empleado por Codacy (PHP Code Sniffer) no reconoce al método html_escape() como un mecanismo de escape, por lo que sigue saltando el mismo error.

clopezno commented 4 years ago

Sobre el problema de escape de código, en Codacy se puede eliminar esa regla o se pueden incluir como falso positivos.

Consulta este los puntos 3 y 4 de este enlace.