Automating code quality control

About me

  • Eugene Zenko
  • Tech Lead (PHP) @ Colours (Den Bosch, Rotterdam)
  • Drupal User Group Belarus, DrupalCamp Belarus 2019
  • Drupal 5-8
  • Football, books, music, rock!
  • Socials: @zeuty
  • Linkedin: yzenko

Assumptions

  1. Quality is not a variable

2. Anything that can go wrong will go wrong (Murphy's law)

3. A man is (a little bit) lazy

3. A man is (a little bit) non-conformist

Code problems

  1. Syntax errors

2. Not following code standards, best practices, different approach to code formatting in the same team

3. Structural problems: not secure, not performing, not maintainable code

Harm

  1. Psychological

2. Reputational

3. Time

What to do?

Processes

Instruments

PHP Lint

Built-in native functionality

php -l {filename}

Example:
php -l sites/all/modules/custom/with_error.php 
PHP Parse error:  syntax error, unexpected '?>' 
in sites/all/modules/custom/with_error.php on line 4
Errors parsing sites/all/modules/custom/with_error.php

PHP Code Sniffer

Automatic code analysis on violation of the coding standards

  • Out of the box - PSR1, PEAR, PSR2, MySource, Squiz, PHPCS, Zend
  • Available - Drupal и DrupalPractice
  • Integrates with PHPStorm
  • Possible to define own standards

PHP Code Beautifier and Fixer

  • Automatically generates diff between source file and coding standard
  • Automatically applies it to the source file

PHP Mess Detector

  • Simple tool, no tons of dependencies, easy composer install or standalone phar
  • 6 rulesets: clean code, code size, naming, unused code, etc.
  • Simple configuration - xml file with a list of rules to be used
  • Fast

PHP Mess Detector

<?xml version="1.0"?>
<ruleset name="Minimal rule set"
         xmlns="http://pmd.sf.net/ruleset/1.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0
                     http://pmd.sf.net/ruleset_xml_schema.xsd"
         xsi:noNamespaceSchemaLocation="
                     http://pmd.sf.net/ruleset_xml_schema.xsd">
    <description>
        Custom rule set to be integrated into pre-commit hook.
    </description>
    <rule ref="rulesets/codesize.xml/CyclomaticComplexity"/>
    <rule ref="rulesets/codesize.xml/ExcessiveMethodLength"/>
    <rule ref="rulesets/codesize.xml/ExcessiveClassLength"/>
    <rule ref="rulesets/codesize.xml/ExcessiveClassComplexity"/>
    <rule ref="rulesets/design.xml/CouplingBetweenObjects"/>
    <rule ref="rulesets/design.xml/DevelopmentCodeFragment"/>
</ruleset>

Yes, but...

How many people will run this beautiful tools during daily routine?

Idea

  • Let's do more code reviews!
  • Let's punish harder the people sabotaging the agreements!
  • Let's simply be individually more responsible!

And maybe let's try to automate this somehow?

Integration with git

  • Using git hooks
  • In our particular case - pre-commit (or pre-receive)

Example

#!/bin/bash
continue_consent() {
    printf "${BLUE}Do you want to force commit without fixing? (y or n) ${NC}\n"
    read inp </dev/tty
    if [ "$inp" != "y" ]
    then
        printf "${GREEN}Fix the error and commit again.${NC}\n"
        exit 1
    else
        printf "${RED}I hope you know what you do... :(${NC}\n"
    fi
}

# Colours list
RED='\033[0;31m'
GREEN='\033[0;32m'
BLUE='\033[0;34m'
NC='\033[0m'

printf "${BLUE}Starting pre-commit hook...${NC}\n"

PROJECT=`php -r "echo dirname(dirname(dirname(realpath('$0'))));"`
STAGED_FILES_CMD=`git diff --cached --name-only --diff-filter=ACMR HEAD | grep -E \\\\.php\|\.module\|\.inc\|\.install\|\.test\|\.profile\|\.theme\|\.info`

# Determine if a file list is passed
if [ "$#" -eq 1 ]
then
	oIFS=$IFS
	IFS='
	'
	SFILES="$1"
	IFS=$oIFS
fi
SFILES=${SFILES:-$STAGED_FILES_CMD}

printf "${BLUE}Running PHP Lint...${NC}\n"
for FILE in $SFILES
do
	php -l -d display_errors=0 $PROJECT/$FILE
	if [ $? != 0 ]
	then
		printf "${RED}Fix the error before commit.${NC}\n"
		exit 1
	fi
	FILES="$FILES $PROJECT/$FILE"
	FILES_COMMA="$FILES_COMMA$PROJECT/$FILE,"
done

# Remove trailing comma character
FILES_COMMA="${FILES_COMMA%"${FILES_COMMA##*[![:punct:]]}"}"

if [ "$FILES" != "" ]
then
	printf "${BLUE}Running Code Sniffer - Drupal...${NC}\n"
	{path_to_cs} --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,info 
        --ignore=core,node_modules,bower_components,vendor --encoding=utf-8 -n -p $FILES
	if [ $? != 0 ]
	then
		printf "${BLUE}Do you want to run PHPCBF to automatically fix the issues? (y or n) ${NC}\n"
		read  phpcbf </dev/tty
		if [ "$phpcbf" == "y" ]
		then
			{path_to_bf} --standard=Drupal 
                        --extensions=php,module,inc,install,test,profile,theme,info 
                        --ignore=core,node_modules,bower_components,vendor --encoding=utf-8 -n -p $FILES
			
                        {path_to_cs} --standard=Drupal 
                        --extensions=php,module,inc,install,test,profile,theme,info 
                        --ignore=core,node_modules,bower_components,vendor --encoding=utf-8 -n -p $FILES
			
                        if [ $? != 0 ]
			then
				continue_consent
			else
				printf "${GREEN}All is fixed!${NC}\n"
			fi
		else
			continue_consent
		fi
	fi
	
	printf "${BLUE}Running Code Sniffer - DrupalPractice...${NC}\n"
	{path_to_cs} --standard=DrupalPractice 
        --extensions=php,module,inc,install,test,profile,theme,info 
        --ignore=core,node_modules,bower_components,vendor --encoding=utf-8 -n -p $FILES
	
        if [ $? != 0 ]
        then
            continue_consent
        fi

        printf "${BLUE}Running PHPMD...${NC}\n"
	{path_to_md} $FILES_COMMA text minimal.xml --exclude core,node_modules,bower_components,vendor --suffixes php,module,inc,install
	if [ $? != 0 ]
        then
            continue_consent
        fi
fi

exit $?

Roadmap?

  • Composer package with git hook configuration wizard and necessary dependencies

Questions, ideas, suggestions?

Become a contributor

Friday!

  • First timers contribution workshop

  • General contribution

  • Mentored contribution

Made with Slides.com