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
- 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
- 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
- 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.phpPHP 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
DrupalEurope - Code quality control (English)
By Yauhen Zenko
