Development

The importance of coding standard.

8 minutes

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.

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.

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

php 
 1<?php
 2 
 3use JumpTwentyFour\PhpCodingStandards\Support\ConfigHelper;
 4use PHP_CodeSniffer\Standards\Generic\Sniffs\NamingConventions\CamelCapsFunctionNameSniff;
 5use SlevomatCodingStandard\Sniffs\Functions\UnusedParameterSniff;
 6use Symplify\EasyCodingStandard\Config\ECSConfig;
 7use Symplify\EasyCodingStandard\ValueObject\Option;
 8 
 9return static function (ECSConfig $ecsConfig): void {
10    $ecsConfig->import(__DIR__.'/vendor/jumptwentyfour/laravel-coding-standards/ecs.php');
11 
12    $parameters = $ecsConfig->parameters();
13 
14    $parameters->set(Option::PATHS, [
15        __DIR__.'/app',
16        __DIR__.'/config',
17        __DIR__.'/database',
18        __DIR__.'/routes',
19        __DIR__.'/tests',
20    ]);
21 
22    $parameters->set(Option::SKIP, array_merge(ConfigHelper::make($ecsConfig)->getParameter(Option::SKIP), [
23        'Unused parameter $request.' => [
24            __DIR__.'/app/Http/Middleware',
25            __DIR__.'/app/Http/Resources',
26            __DIR__.'/app/Domain/*/Resources/*.php',
27            __DIR__.'/app/Http/Controllers/*.php',
28        ],
29    ]));
30};

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.

Looking to start your next Laravel project?

We’re a passionate, happy team and we care about creating top-quality web applications that stand out from the crowd. Let our skilled development team work with you to bring your ideas to life! Get in touch today and let’s get started!

Get in touch

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.

javascript 
 1"scripts": {
 2        "stan": [
 3            "vendor/bin/phpstan analyse --memory-limit=4G"
 4        ],
 5        "test:parallel": [
 6            "Composer\\Config::disableProcessTimeout",
 7            "php artisan cache:clear",
 8            "php artisan test --parallel"
 9        ],
10        "ecs": [
11            "vendor/bin/ecs check --debug"
12        ],
13        "ecs-fix": [
14            "vendor/bin/ecs check --fix"
15        ],
16    },

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

yaml-frontmatter 
  1# GitHub Action for Laravel with MySQL and Redis
  2name: Testing Laravel with MySQL
  3on:
  4  pull_request:
  5    branches: [ main, release/**, feature/** ]
  6    types:
  7      - opened
  8      - reopened
  9      - synchronize
 10      - ready_for_review
 11jobs:
 12  build:
 13    name: Build
 14    runs-on: ubuntu-latest
 15    steps:
 16      - name: Checkout
 17        uses: actions/checkout@v3
 18
 19      - name: Setup PHP
 20        uses: shivammathur/setup-php@v2
 21        with:
 22          php-version: '8.2'
 23          extensions: bcmath, ctype, fileinfo, gd, json, intl, opcache, mbstring, openssl, pdo_mysql, tokenizer, xml, zip
 24          coverage: xdebug
 25
 26      - name: Setup Node.js
 27        uses: actions/setup-node@v1
 28        with:
 29          node-version: '15.x'
 30
 31      - uses: actions/cache@v3
 32        id: npm-cache
 33        with:
 34          path: node_modules
 35          key: node-modules-${{ hashFiles('package-lock.json') }}
 36
 37      - name: Install JavaScript Dependencies
 38        if: steps.npm-cache.outputs.cache-hit != 'true'
 39        run: npm ci
 40
 41  phpstan:
 42    name: PHPStan
 43    needs: build
 44    runs-on: ubuntu-latest
 45    steps:
 46      - name: Setup PHP
 47        uses: shivammathur/setup-php@v2
 48        with:
 49          php-version: '8.2'
 50          extensions: bcmath, ctype, fileinfo, gd, json, intl, opcache, mbstring, openssl, pdo_mysql, tokenizer, xml, zip
 51          coverage: xdebug
 52
 53      - uses: actions/checkout@v3
 54      - uses: ramsey/composer-install@v2
 55
 56      - name: Copy .env.pipeline file to .env
 57        run: php -r "file_exists('.env') || copy('.env.pipeline', '.env');"
 58
 59      - name: PHPStan (Static Analysis Tool)
 60        run: composer run stan
 61
 62  ecs:
 63    name: ECS
 64    needs: build
 65    runs-on: ubuntu-latest
 66    steps:
 67      - name: Setup PHP
 68        uses: shivammathur/setup-php@v2
 69        with:
 70          php-version: '8.2'
 71          extensions: bcmath, ctype, fileinfo, gd, json, intl, opcache, mbstring, openssl, pdo_mysql, tokenizer, xml, zip
 72          coverage: xdebug
 73      - uses: actions/checkout@v3
 74      - uses: ramsey/composer-install@v2
 75
 76      - name: Copy .env.pipeline file to .env
 77        run: php -r "file_exists('.env') || copy('.env.pipeline', '.env');"
 78
 79      - name: PHP ECS (Easy Coding Standard)
 80        run: composer run ecs
 81
 82  test-php:
 83    name: Test PHP
 84    needs: build
 85    runs-on: ubuntu-latest
 86    services:
 87      mysql:
 88        image: mysql:8.0
 89        env:
 90          MYSQL_ALLOW_EMPTY_PASSWORD: false
 91          MYSQL_ROOT_PASSWORD: root
 92          MYSQL_DATABASE: testing_db
 93        ports:
 94          - 3306/tcp
 95        options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
 96      redis:
 97        image: redis
 98        ports:
 99          - 6379/tcp
100        options: --health-cmd="redis-cli ping" --health-interval=10s --health-timeout=5s --health-retries=3
101    strategy:
102      fail-fast: false
103      matrix:
104        php-versions: ['8.2']
105    env:
106      DB_HOST: 127.0.0.1
107      BROADCAST_DRIVER: log
108      CACHE_DRIVER: file
109      QUEUE_CONNECTION: sync
110      SESSION_DRIVER: file
111    steps:
112      - name: Setup PHP
113        uses: shivammathur/setup-php@v2
114        with:
115          php-version: '8.2'
116          extensions: bcmath, ctype, fileinfo, gd, json, intl, opcache, mbstring, openssl, pdo_mysql, tokenizer, xml, zip
117          coverage: xdebug
118
119      - uses: actions/checkout@v3
120      - uses: ramsey/composer-install@v2
121      - uses: actions/cache@v3
122        with:
123          path: node_modules
124          key: node-modules-${{ hashFiles('package-lock.json') }}
125
126      - name: Start mysql service
127        run: sudo /etc/init.d/mysql start
128
129      - name: Copy .env.pipeline file to .env
130        run: php -r "file_exists('.env') || copy('.env.pipeline', '.env');"
131
132      - name: Build CSS/JavaScript Files
133        run: npm run build
134
135      - name: Run tests with PHPUnit
136        run: composer run test:parallel
137        env:
138          REDIS_HOST: redis
139          DB_PORT: ${{ job.services.mysql.ports['3306'] }}
140          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