En Human Made nos llegan muchísimos Pull Requests en todos los proyectos que tenemos de forma diaria y por regla todos los PRs tienen que ser revisados. Si quieres aprender bien en qué consiste una revisión de código y cómo llevarla a buen término, recomiendo estos dos posts: https://mtlynch.io/human-code-reviews-1/ y https://mtlynch.io/human-code-reviews-2/. Por cierto, si en tu lugar de trabajo no haceis revisiones de código, nunca es tarde para empezar. Se aprende muchísimo y dejas de tomarte muchas cosas como algo personal. Además aprendes a tratar con las personas de tu equipo y lo mejor es que el código de producción mejorará notablemente.

Hacer una revisión de código sobre grandes cambios es algo muy difícil. Aparte de lo que conlleva en cuanto a la delicadeza de decirle a alguien que sus cambios son una birria o que no valen (tratar con gente es lo más difícil aquí por si no te habías dado cuenta, estimado lector); GitHub no ofrece una buena herramienta para revisar código: No tenemos colorines o no hay nada que nos señale dónde puede estar el error. De forma adicional, si estamos siguiendo un coding standard (¿A qué estás esperando?), buscar esos errores es muy pesado y aporta bien poco a ambas partes.

Lo ideal es que en nuestro proyecto tengamos ya instaladas herramientas para ahorrarnos las revisiones de sintaxis como PHP Code Sniffer, ESLint o JSHint (Aquí tengo un repositorio para empezar un proyecto en WordPress con todo eso: https://github.com/igmoweb/wp-phpcs-boilerplate) pero no siempre es así y puede que necesitemos hacer algún tipo de revisión en un repositorio en el que no hay integración contínua. Para ahorrarme tarea suelo utilizar phpcs instalado globalmente y así escanear un directorio completo. Para eso hay que seguir una serie de pasos:

Instalar phpcs

Hay que instalarlo de forma global en nuestro equipo usando Composer (también instalado de forma global):

composer global require squizlabs/php_codesniffer

Si quisiéramos ejecutar phpcs, tendríamos que lanzar el siguiente comando:

~/.composer/vendor/bin/phpcs test.php

Pero sería mejor si accediéramos a phpcs sin escribir toda la ruta. Para ello podemos crearnos un enlace simbólico dentro de nuestra carpeta personal ~/bin (Si no existe, puedes crearla).

ln -s ~/.composer/vendor/bin/phpcs ~/bin/phpcs

Seguramente tengas que reiniciar la consola para que el cambio funcione. Para probar puedes ejecutar phpcs sobre cualquier fichero PHP:

phpcs test.php

Instalar reglas de validación

Con PHP Code Sniffer puedes indicar las reglas que te gustaría aplicar sobre tu código mediante un fichero de configuración xml. Aquí vamos a usar un conjunto de reglas específicas para cumplir el estándar de WordPress además de la librería PSR2R que contiene un monton de reglas más.

composer global require wp-coding-standards/wpcs
composer global require fig-r/psr2r-sniffer

Ya tenemos todo lo necesario para empezar a trabajar

Configuración de phpcs

Ahora simplemente tenemos que indicar las reglas que vamos a utilizar. Para eso vamos a crear un fichero .xml. ¿Dónde? Donde quieras en tu equipo, en alguna carpeta que localices fácilmente, yo suelo usar ~/source para meter todo mi código.

Vamos a crear dos ficheros:

phpcs-security.xml: Sólo para las reglas más básicas de seguridad.

<?xml version="1.0"?>
<ruleset name="Ignacio">
    <description>Security reviews with PHPCS.</description>

    <file>.</file>

    <!-- Exclude the Composer Vendor directory. -->
    <exclude-pattern>/vendor/*</exclude-pattern>

    <!-- Exclude the Node Modules directory. -->
    <exclude-pattern>/node_modules/*</exclude-pattern>

    <!-- wpcs installed path -->
    <config name="installed_paths" value="/Users/ignacio/.composer/vendor/wp-coding-standards/wpcs" />

    <!-- PHPCS WP Aliases. Needed to execute WP Rules -->
    <autoload>/Users/ignacio/.composer/vendor/wp-coding-standards/wpcs/WordPress/PHPCSAliases.php</autoload>

    <!-- Just check php files -->
    <arg name="extensions" value="php"/>

    <!-- Colors! Nice! -->
    <arg name="colors"/>

    <!-- Set of rules we're going to use -->
    <rule ref="WordPress.XSS"/>
    <rule ref="WordPress.CSRF"/>
    <rule ref="WordPress.VIP.ValidatedSanitizedInput"/>
    <rule ref="WordPress.WP.PreparedSQL"/>
    <rule ref="WordPress.Variables.GlobalVariables"/>
    <rule ref="WordPress.PHP.StrictInArray"/>
    <rule ref="Squiz.PHP.Eval"/>
    <rule ref="Squiz.PHP.Eval.Discouraged">
        <type>error</type>
        <message>eval() is a security risk so not allowed.</message>
    </rule>
</ruleset>

Aquí usamos varias reglas de seguridad como revisión de posibles problemas de XSS, CSRF y reglas de WordPress VIP sobre validación entre otras cuantas muy básicas que nos valen para hacer un primer chequeo de seguridad en cualquier código. Ten en cuenta que algunas rutas tienen que ser absolutas o al menos no he encontrado forma de usar ~/ para usar rutas relativas a nuestra carpeta home.

phpcs-all.xml: Para todas las reglas (cuidado, esto es lo más parecido a un compañero psicópata pero al menos puedes quitarle reglas).

<?xml version="1.0"?>
<ruleset name="My-Project">
    <description>Sniffs for the coding standards of your project.</description>

    <file>.</file>

    <!-- Exclude the Composer Vendor directory. -->
    <exclude-pattern>/vendor/*</exclude-pattern>

    <!-- Exclude the Node Modules directory. -->
    <exclude-pattern>/node_modules/*</exclude-pattern>

    <!-- wpcs installed path -->
    <config name="installed_paths" value="/Users/ignacio/.composer/vendor/wp-coding-standards/wpcs" />

    <!-- PHPCS WP Aliases. Needed to execute WP Rules -->
    <autoload>/Users/ignacio/.composer/vendor/wp-coding-standards/wpcs/WordPress/PHPCSAliases.php</autoload>

    <!-- Just check php files -->
    <arg name="extensions" value="php"/>

    <!-- Colors! Nice! -->
    <arg name="colors"/>

    <!-- Full report -->
    <arg name="report" value="full"/>

    <rule ref="WordPress">
        <!-- Disable Yoda conditions check as an example. -->
        <exclude name="WordPress.PHP.YodaConditions" />
    </rule>

    <!-- Add this rule too -->
    <rule ref="/Users/ignacio/.composer/vendor/fig-r/psr2r-sniffer/PSR2R/Sniffs/ControlStructures/NoInlineAssignmentSniff.php"/>
</ruleset>

En este ejemplo he eliminado una regla de las que vienen incluídas en el paquete de WordPress: Las Yoda Conditions y además he incluído otra de PSR2R: NoInlineAssignmentSniff.

Con esto ya tenemos todo listo para empezar a revisar código.

Revisar el código

Si algún compañero ha hecho un Pull Request en el repositorio de nuestro proyecto, podemos descargar la rama de Git en dicho proyecto:

cd mi-proyecto && git fetch origin pull-request-branch && git checkout pull-request-branch

Y ahora podemos ejecutar phpcs a todo el directorio.

phpcs --standard=/ruta/a/fichero/phpcs-security.xml

o también

phpcs --standard=/ruta/a/fichero/phpcs-all.xml

Con esto sería bastante sencillo crearnos un script que nos ejecute automáticamente el escaneo, incluso podríamos elegir qué fichero queremos utilizar:

#!/usr/bin/env bash

# Change this!
PATH_TO_RULESETS="~/source"

echo "Select what type of review you want to make: Security(s)/All(a). Default Security"
read SUFFIX
if [[ -z "$SUFFIX" ]]
then
	SUFFIX='s'
fi

if [ $SUFFIX == 'a' ]
then
	SUFFIX='all'
else
	SUFFIX='security'
fi

RULESET=$PATH_TO_RULESETS/"phpcs-$SUFFIX.xml"
echo "Selected phpcs file: "
echo $RULESET

phpcs --standard=$RULESET

Si copias ese fichero en tu carpeta bin (~/source/bin) y le das permisos de ejecución (sudo chmod +x ~/bin/review.sh) podrás escanear cualquier carpeta simplemente invocando review.sh desde dicha carpeta.

Con esto tenemos ya cubierto una buena parte en una revisión de código. Evidentemente, no hemos revisado todavía la lógica del código pero al menos podemos tener bastante claro si hay algún problema de seguridad con este método y todo en unos pocos segundos.

Nota: El script se podría mejorar un poco sacando el listado de archivos de un commit en concreto a partir del hash de Git. De esta forma no tendríamos que hacer un escaneo a la carpeta completa (que puede ser enorme). Sería interesante verlo en funcionamiento.

A mí me funciona

category => Git

¿Conoces GitHub CLI?

Cómo utilizar PHP Code Sniffer para revisiones de código

Descargar un Pull Request de Bitbucket y aplicarlo en local