The importance of coding standard.

Over the years there has been plenty of debates around coding standards. People have argued over things like tabs vs spaces, line lengths and other aspects when it comes to writing code. Groups have been setup to help define these standards in the PHP world we have the PHP-FIG who propose standards for us to use.

One thing we have noticed becoming more and more apparent is that developers are now starting to listen and think more about this, not just about standards like PSR-1, PSR-2, PSR-12 and now PER Coding Style 2.0, but also with the evolutions of the type system in PHP people are starting to develop more robust applications by using these types and writing tests (finally).

New tools are being developed that make applying standards to your code and fixing inconsistencies a lot easier, there are also tools that have been out there in the wild for a long time and are now being promoted and talked about again which is great to see.

How standards help us deliver better applications

Jump24 started out with a single developer, back then PHP standards weren’t top of the list when writing code. As the company expanded and more developers came on board the lack of standards in our PHP applications became more apparent. Each developer would write and push up code with method names either in camel case or snake case; variables declared much the same. Line lengths were sometimes very long and unreadable in Pull Requests and line endings where different as well.

With all these various issues spread across multiple projects, we found that Pull Requests were taking longer than expected to go through as each developer looking at it would leave varying comments based on what they thought the standards should be.

The solution

Once we identified the problem and how much of a bottleneck it was causing the team, we sat down and planned how to tackle the problem.

We talked about how we would go about it from both a frontend and backend perspective. The standards we should start adhering to and looked at the tools we could use that would help us going forward to minimise the amount of this type of feedback that would appear on PRs.

Each programming language has differing standards; PHP has a number of these PHP Standard Recommendations three of them outline how you should write code in particular. We decided that we should be adhering to these standards. We started with PSR1 and PSR2, then when PSR12 was approved, we added that to our code requirements.

We found it a lot easier to adhere to these standards when working on new projects as we were writing the code from the start, but when we were tackling old codebases that we’d taken on this wasn’t as straightforward. To this end, we decided that every time we had to work on one of these projects that the new code developed would be standard compliant and any code we’d change around it would be updated to be compliant. So slowly each project over time would evolve to match our requirements, we took this approach instead of merely updating the whole project in one go as the Pull Request would be too big and getting this signed off would take quite a bit of time.

The tools and packages we use to keep these standards at bay

Now that we’d decided on the standards we were going to use when working with PHP we had to figure out the best way to help us maintain these. We didn’t want to 100% rely on our developers having to remember every part of each standard. We were already using PHPStorm as our IDE our first thought was how we could set this up to let us know when we were breaking these rules hopefully stopping us even committing this kind of code in the first place.

Firstly we looked at what was available for the IDE that would help us achieve the goal. We found an excellent plugin called PHP Inspections (EA Extended). This plugin is an open-source Static Code Analyser, after installing this plugin, we were instantly made aware of several issues that we were able to solve quickly, so the plugin was already doing a great job.

We then looked at other tools we could use with composer and how we could implement them into our development lifecycle.

We finally settled on the following packages that we’ve started including in every build that we have be it a new project or working on an old project.

• Easy Coding Standards
• Slevomat Coding Standards
• PHPStan
• Larastan – only for Laravel based projects
• Laravel IDE Helper

Due to always requiring the above packages in our projects we decided to create our own packages which encapsulates all of the required items so now we simply install the following two packages to gain access to ECS, Slevomat, PHPStan and Larastan.

• Jump24 Laravel Coding Standards – only for Laravel based projects
• Jump24 PHP Coding Standards – For non Laravel based projects

To get these tools up and running in our projects firstly we would install these as dependencies in the project then there are a couple of files we need to add to our codebase to handle the configuration for the tools doing our checks. The two main files we have are phpstan.neon for PHPStan/Larastan to configure our static analysis and ecs.php for Easy Coding Standard.

Why easy coding standard

We made the decision to adopt Easy Coding Standard (ECS) by Tomas Votruba as the foundation for all of our coding standard rules. We chose ECS because we prefer writing configuration rules in PHP that are easily understood by PHPStorm, as opposed to XML.

A typical ecs.php that can be found in our projects

import(__DIR__.'/vendor/jumptwentyfour/laravel-coding-standards/ecs.php');

    $parameters = $ecsConfig->parameters();

    $parameters->set(Option::PATHS, [
        __DIR__.'/app',
        __DIR__.'/config',
        __DIR__.'/database',
        __DIR__.'/routes',
        __DIR__.'/tests',
    ]);

    $parameters->set(Option::SKIP, array_merge(ConfigHelper::make($ecsConfig)->getParameter(Option::SKIP), [
        'Unused parameter $request.' => [
            __DIR__.'/app/Http/Middleware',
            __DIR__.'/app/Http/Resources',
            __DIR__.'/app/Domain/*/Resources/*.php',
            __DIR__.'/app/Http/Controllers/*.php',
        ],
    ]));
};

As you can see from the example above we import the base jump24 standards and then tell ECS which folders to look at when reviewing the code. You can then overwrite any particular changes that you might need on a per project basis. Sometimes we need to do this, particularly if we are bringing ECS into an older project where we will need to improve the standards over time. We did have to make a change to our package to build out a new ConfigHelper to grab all the skips we defined in our package and array merge them with individual skips from the specific project we are in.

Using ECS also allows us to define our own company standards as an external open source package that can be brought into our projects and extended as needed. Furthermore, since ECS is built on top of PHPCS, we don’t lose any of the original capabilities, this includes the fixer which is now centralised under a single command passing an additional argument --fix. With ECS, we have the added benefit of being able to write rules as PHP code and access numerous predefined sets, such as PSR12 and Clean Code, which can simplify the base configuration of the standards file. They also provide a quick guide away to migrate from PHPCS How to Migrate From PHP_CodeSniffer to ECS in 7 Steps

Tomas also actively maintains RectorPHP and has recently become more involved with the Laravel community as a whole.

Running ECS

To remove the dependency of remembering the syntax of running coding standard commands, we alias them as composer commands in the composer.json file. Running vendor/bin/ecs check instead becomes composer ecs and running vendor/bin/ecs check --fix becomes composer ecs-fix.

An example of a typical scripts section of a composer.json file in our projects.

"scripts": {
        "stan": [
            "vendor/bin/phpstan analyse --memory-limit=4G"
        ],
        "test:parallel": [
            "Composer\\Config::disableProcessTimeout",
            "php artisan cache:clear",
            "php artisan test --parallel"
        ],
        "ecs": [
            "vendor/bin/ecs check --debug"
        ],
        "ecs-fix": [
            "vendor/bin/ecs check --fix"
        ],
    },

Running the standards in a CI Pipeline

The majority of the projects we have run GitHub actions on Pull Requests, we have specific steps in our actions that run our standards as well as tests.

Github Action

# GitHub Action for Laravel with MySQL and Redis
name: Testing Laravel with MySQL
on:
  pull_request:
    branches: [ main, release/**, feature/** ]
    types:
      - opened
      - reopened
      - synchronize
      - ready_for_review
jobs:
  build:
    name: Build
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v3

      - name: Setup PHP
        uses: shivammathur/setup-php@v2
        with:
          php-version: '8.2'
          extensions: bcmath, ctype, fileinfo, gd, json, intl, opcache, mbstring, openssl, pdo_mysql, tokenizer, xml, zip
          coverage: xdebug

      - name: Setup Node.js
        uses: actions/setup-node@v1
        with:
          node-version: '15.x'

      - uses: actions/cache@v3
        id: npm-cache
        with:
          path: node_modules
          key: node-modules-${{ hashFiles('package-lock.json') }}

      - name: Install JavaScript Dependencies
        if: steps.npm-cache.outputs.cache-hit != 'true'
        run: npm ci

  phpstan:
    name: PHPStan
    needs: build
    runs-on: ubuntu-latest
    steps:
      - name: Setup PHP
        uses: shivammathur/setup-php@v2
        with:
          php-version: '8.2'
          extensions: bcmath, ctype, fileinfo, gd, json, intl, opcache, mbstring, openssl, pdo_mysql, tokenizer, xml, zip
          coverage: xdebug

      - uses: actions/checkout@v3
      - uses: ramsey/composer-install@v2

      - name: Copy .env.pipeline file to .env
        run: php -r "file_exists('.env') || copy('.env.pipeline', '.env');"

      - name: PHPStan (Static Analysis Tool)
        run: composer run stan

  ecs:
    name: ECS
    needs: build
    runs-on: ubuntu-latest
    steps:
      - name: Setup PHP
        uses: shivammathur/setup-php@v2
        with:
          php-version: '8.2'
          extensions: bcmath, ctype, fileinfo, gd, json, intl, opcache, mbstring, openssl, pdo_mysql, tokenizer, xml, zip
          coverage: xdebug
      - uses: actions/checkout@v3
      - uses: ramsey/composer-install@v2

      - name: Copy .env.pipeline file to .env
        run: php -r "file_exists('.env') || copy('.env.pipeline', '.env');"

      - name: PHP ECS (Easy Coding Standard)
        run: composer run ecs

  test-php:
    name: Test PHP
    needs: build
    runs-on: ubuntu-latest
    services:
      mysql:
        image: mysql:8.0
        env:
          MYSQL_ALLOW_EMPTY_PASSWORD: false
          MYSQL_ROOT_PASSWORD: root
          MYSQL_DATABASE: testing_db
        ports:
          - 3306/tcp
        options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
      redis:
        image: redis
        ports:
          - 6379/tcp
        options: --health-cmd="redis-cli ping" --health-interval=10s --health-timeout=5s --health-retries=3
    strategy:
      fail-fast: false
      matrix:
        php-versions: ['8.2']
    env:
      DB_HOST: 127.0.0.1
      BROADCAST_DRIVER: log
      CACHE_DRIVER: file
      QUEUE_CONNECTION: sync
      SESSION_DRIVER: file
    steps:
      - name: Setup PHP
        uses: shivammathur/setup-php@v2
        with:
          php-version: '8.2'
          extensions: bcmath, ctype, fileinfo, gd, json, intl, opcache, mbstring, openssl, pdo_mysql, tokenizer, xml, zip
          coverage: xdebug

      - uses: actions/checkout@v3
      - uses: ramsey/composer-install@v2
      - uses: actions/cache@v3
        with:
          path: node_modules
          key: node-modules-${{ hashFiles('package-lock.json') }}

      - name: Start mysql service
        run: sudo /etc/init.d/mysql start

      - name: Copy .env.pipeline file to .env
        run: php -r "file_exists('.env') || copy('.env.pipeline', '.env');"

      - name: Build CSS/JavaScript Files
        run: npm run build

      - name: Run tests with PHPUnit
        run: composer run test:parallel
        env:
          REDIS_HOST: redis
          DB_PORT: ${{ job.services.mysql.ports['3306'] }}
          REDIS_PORT: ${{ job.services.redis.ports['6379'] }}

Verdict

After all the work we put into getting this far what do we think? Firstly we feel it was the right decision to make as a team. The PR feedback is now just about the code itself and recommendations on improving the design, no more comments on indentation, old use statements hanging around or even variables being declared and not being used.

We think spending time doing this was beneficial as a company but also as individual developers as we now know a lot more about the PHP Standards in place and are writing documented code that is much cleaner to read, we’re now writing more consistent and readable code as a team