Integrating Some PHP QA Tools

Tag

php
Tag

docker
Tag

bash
Tag

git
Tag

make
Author: Ally
Published:
Suggest changes

Summary:

How you can integrate some PHP QA tools in your project, and guard your project with git hooks.

Table of Contents

  1. phplint
  2. phpcs
  3. phpcbf
  4. Violation Prevention (git hooks)

TL;DR:

We use the excellent bunch of PHP QA tools from the jakzal/phpqa Docker image. We’re only using a small subset of these tools for now, but decided to use the container option to save having to install all these tools per project in composer.

To run some of these tools, we’ll create a shortcut for them, in a Makefile per project.

Will create a qa_docker variable at the top of the Makefile to save typing this out for each QA check. Used verbose docker run options for clarity.

Makefile:

qa_docker = docker run \
    --init \
    --tty \
    --rm \
    --user=$$(id -u) \
    --volume="$$(pwd):/project" \
    --workdir "/project" \
    jakzal/phpqa:php7.3

phplint

A fairly simple tool - it detects syntax (lint) errors in PHP code. Can be parallelised and gives context to your error(s).

Example: .phplint.yml:

path: ./
jobs: 10
cache: build/phplint.cache
extensions:
  - php
exclude:
  - vendor

The recipe for this one is easy! (the command is prefixed with @ which stops the command being printed)

phplint:
    @${qa_docker} phplint

Example (when everything is good):

$ make phplint
phplint 2.0.2 by overtrue and contributors.

Loaded config from "/project/.phplint.yml"

.................................................

Time: < 1 sec	Memory: 12.0 MiB	Cache: Yes

OK! (Files: 1433, Success: 1433)

Example (when something isn’t quite right):

$ make phplint          
phplint 2.0.2 by overtrue and contributors.

Loaded config from "/project/.phplint.yml"

E

Time: < 1 sec	Memory: 12.0 MiB	Cache: Yes

FAILURES!
Files: 1433, Failures: 1

There was 1 errors:
1. /project/app/Model/Job.php:8611
    8608|         $updated = $this->updateAll(
    8609|             ['Job.void' => 0],
    8610|             ['Job.id' => $id]
  > 8611|         ));
    8612| 
    8613|         if (!$updated) {
    8614|             // redacted
 unexpected ')' in line 8611

It certainly beats my own implementation from years ago:

find app/ -type f \
    -regextype posix-egrep \
    -regex "app\/(Config|Console|Controller|Lib|Model|Test).*.php" \
    -exec php -l {} \; | (! grep -v "No syntax errors detected")

phpcs

Some of our code base is fairly old, so over time trying to polish it up (with phpcbf).

We’re going to follow PSR-12 (Mostly! base phpcs.xml for that).

Makefile:

phpcs:
    @${qa_docker} phpcbs -s

The -s flag:

 -s    Show sniff codes in all reports

Handy if your code is impossible to comply with all PSR-12 rules, you now know the code to add to an exemption list, or tweak some rules parameters to suit.

Quick example on linting from stdin, but obviously can pass a path to a file, or a list (more on that later).

docker run -i jakzal/phpqa:php7.3 phpcs --standard=PSR12 -s - <<"PHP"

<?php

echo "violation";

class Classy
{
    //
} 

PHP

You can see the output looks like this. Lines removed in diff are when run without -s.

 ----------------------------------------------------------------------
 FOUND 2 ERRORS AND 1 WARNING AFFECTING 3 LINES
 ----------------------------------------------------------------------
  1 | WARNING | [ ] A file should declare new symbols (classes,
    |         |     functions, constants, etc.) and cause no other
    |         |     side effects, or it should execute logic with side
    |         |     effects, but should not do both. The first symbol
    |         |     is defined on line 5 and the first side effect is
    |         |     on line 3.
-   |         |     (PSR1.Files.SideEffects.FoundWithSymbols)
  5 | ERROR   | [ ] Each class must be in a namespace of at least one
    |         |     level (a top-level vendor name)
-   |         |     (PSR1.Classes.ClassDeclaration.MissingNamespace)
  8 | ERROR   | [x] Whitespace found at end of line
-   |         |     (Squiz.WhiteSpace.SuperfluousWhitespace.EndLine)
 ----------------------------------------------------------------------
 PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
 ----------------------------------------------------------------------

phpcbf

Uses the same config as phpcs but will try its best to fix these violations for you.

I haven’t figured out how to integrate this (automatically) into a workflow yet. However, when I need to work in a file (e.g. src/Reader.php), I tend to follow this process:

There is an easy access recipe.

Makefile:

phpcbf:
    ${qa_docker} phpcbf $(args)

Some exceptions in our setup:

phpcs.xml:

    <!-- Some Exemptions -->
    <rule ref="Generic.Arrays.DisallowLongArraySyntax"/>
    <rule ref="PSR1">
        <!-- Warning -->
        <!-- A file should declare new symbols (classes, functions, constants, etc.) -->
        <!-- and cause no other side effects, or it should execute logic with side -->
        <!-- effects, but should not do both. -->
        <!-- Rationale: We exclude this because CakePHP 2.x isn't namespaced, -->
        <!-- so we need to use App::uses before class definitions -->
        <exclude name="PSR1.Files.SideEffects.FoundWithSymbols" />

        <!-- Error -->
        <!-- Each class must be in a namespace of at least one level -->
        <!-- (a top-level vendor name) -->
        <!-- Rationale: CakePHP 2.x models, etc. aren't namespaced -->
        <exclude name="PSR1.Classes.ClassDeclaration.MissingNamespace" />
    </rule>
    
    <!-- converts array() to [], etc. -->
    <rule ref="Generic.Arrays.DisallowLongArraySyntax.Found">
        <type>error</type>
    </rule>

Violation Prevention (git hooks)

I’m pretty new to git hooks to prevent stuff from happening, though I’ve known about it for a long time. I won’t go into great detail about them, instead I suggest you watch this tutorial and refer to relevant git book chapter. Each sample has a link to the content, the will direct you to relevant section in git hooks man page for more info on them.

They usually live in .git/hooks so normally aren’t part of a project in source control.

Having the custom logic for your project not in source control is a bad idea, thankfully, however, there is git config core.hookspath.

With this you can, for example, have a .githooks folder in your project with your custom hooks/logic and then tell git about it.

git config core.hookspath "$(pwd)/.githooks"

Now to get the hooks to work, give them the relevant name (i.e without .sample suffix) in .githooks/ and make them executable, i.e. chmod +x.

If we want to prevent a git action from happening, e.g. git commit when there are parse errors, then we would likely want to add a pre-commit hook.

It might look something like this!

.githooks/pre-commit:

#!/usr/bin/env bash
# Need to pass file to make since we're not in the same folder as it
ROOT_MAKE="make --file=$(git rev-parse --show-toplevel)/Makefile"
LINT_RESULT=$($ROOT_MAKE phplint)

if [ $? -eq "0" ]; then
  exit 0
else
  echo "There were some errors."
  echo "You must add these fixes before being allowed to commit."
  echo ""
  echo -e "$LINT_RESULT"
  exit 1
fi

Now this probably isn’t really the logic you want to apply. We’ll only want to lint files actually in the commit. Think of the case if there’s an error elsewhere in the local file system within the repository, but the change isn’t for something that’s being committed, then this will be a false positive.

To get smarter results, add the following:

Makefile:

files_in_git_diff = git \
    --no-pager \
    diff \
    --name-only \
    --staged

php_files_in_git_diff = ${files_in_git_diff} \
    --diff-filter=AMRC \
    -- '*.php'

Read more on:

Staging a file will then give something like below:

$ make phpdiff
Reader.php

Now with a small tweak to the phplint recipe, we can make it more flexible.

We will allow args to be passed to phplint.

Makefile:

 phplint:
-       @${qa_docker} phplint
+       @${qa_docker} phplint $(args)

Example:

$ make phplint args=--help
Description:
  Lint something

Usage:
  phplint [options] [--] [<path>...]

Arguments:
  path                               Path to file or directory to lint.

[truncated]

We can now combine make phpdiff with make phplint to lint only the relevant files to a commit.

Just another issue, make phpdiff will print one file per line, so we need to put them all on one line for phpcs args.

 phpcs:
-    @${qa_docker} phpcbs -s
+    @${qa_docker} phpcbs -s $(args)

+phpdiff-one-line:
+    @${php_files_in_git_diff} \
+        | paste --serial --delimiters=' '

Now to update the pre-commit hook to lint only relevant files. Might look something like this:

.githooks/pre-commit:

 #!/usr/bin/env bash
 ROOT_MAKE="make --file=$(git rev-parse --show-toplevel)/Makefile"
-LINT_RESULT=$($ROOT_MAKE phplint)
+LINT_RESULT=$($ROOT_MAKE phplint \
+    args=$($ROOT_MAKE phpdiff-one-line)
+)

A complete check might look something like this:

.githooks/pre-commit:

#!/usr/bin/env bash
MAKE="make --file=$(git rev-parse --show-toplevel)/Makefile"
PHP_FILES_IN_DIFF=$($MAKE phpdiff)

if [ -z "$PHP_FILES_IN_DIFF" ]; then
  echo "# No PHP file changes detected. No phplint required."
  exit 0
fi

LINT_RESULT=$($MAKE phplint args="$(make phpdiff-one-line)")

if [ $? -eq "0" ]; then
  exit 0
else
  echo "There were some errors."
  echo "You must add these fixes before being allowed to commit."
  echo ""
  echo -e "$LINT_RESULT"
  exit 1
fi
Apache httpd Server Configuration Hardening
Developing in PHP with Docker: one nginx ingress to isolated httpd & php containers
To bottom
To top