(Draft) Code Style Guide

draft-codestyle.md

(Draft) Code Style guide

Use PSR-12 code style as base, read about it here https://www.php-fig.org/psr/psr-12/

Code Formatting rules

You MUST format only code you add or change to reduce cognitive load for people doing PR reviews.

You SHOULD prefer selection formatting over full file formatting.

PhpStorm: setup

Import preconfigured setting from here: [<..todo settings link...>](http://<..todo settings link...>) or setup manually as described below.

In PhpStorm: Editor > Code Style > PHP > Set from... choose PSR-12.

Also you SHOULD enable PSR-12 inspections (disabled by default).

PhpStorm: Formatting code

Most formating now can be done by IDE. You press Ctrl + Alt + L to reformat current file or current selection.

PhpStorm: Additional formatting settings

Go to: Editor > Code Style > PHP > Wrapping and Braces

• Function/constructor call arguments - Chop down if long

  • Align when multiline - disabled
  • New line after '(' - disabled
  • Place ')' on new line - disabled
  • Place '()' for constructor - always

• Array initializer - Chop down if long

  • Align key-value pairs - enabled
  • New line after ')' - enabled
  • Place ')' on new line - enabled

Good:

$x = [
    "x",
    "y",
    [
        1 => "abc",
        2 => "def",
        3 => "ghi",
    ],
];

$devices->each(
    function (Device $device) use ($sample, $org) {
        factory(Sample::class)->create([
            'device_id'         => $device->id,
            'percent_remaining' => $sample,
            'organization_id'   => $org->id,
        ]);
    }
);

Bad:

$x = ["x",
    "y",
    [1 => "abc",
     2 => "def",
     3 => "ghi"]];

$devices->each(
    function (Device $device) use ($sample, $org) {
        factory(Sample::class)->create([
                                          'device_id' => $device->id,
                                          'percent_remaining' => $sample,
                                          'organization_id' => $org->id,
                                      ]);
    }
);

Main Code Base

All the rules are not retroactive and MUST be used only for new and refactored code.

#M1 - File header SHOULD contain strict_types declaration on the same line with <?php tag

Example:

<?php declare(strict_types=1);

#M2 - Function MUST have return typehint

Good:

function hello(): void
{
}

Bad:

function hello()
{
}

#M3 - Function parameters MUST have typehints

Good:

function hello(string $name): void
{
}

Bad:

function hello($name): void
{
}

#M4 - Function SHOULD NOT have nullable return types.

Where possible try to have predicate methods for checking existense.

Good:

function hasName(): bool
{
}

function getName(): string
{
}

Acceptable:

function getName(): ?string
{
}

#M5 - SHOULD NOT add phpdoc block to every code piece

Add only if function/interface throws or you really need to describe it. Most of the parameters are infered by modern IDEs from signature.

Good:

/**
 * Very helpful description
 *
 * @param Device $device
 *
 * @return RefillTaskHandlerInterface
 * @throws ShoulNotBeHandledException
 */
public function getHandler(Device $device): RefillTaskHandlerInterface
{}

Bad (everything from this block can be infered from signature):

/**
 * @param Device $device
 *
 * @return bool
 */
public function hasHandler(Device $device): bool
{}

#M6 - SHOULD write PHPStan compatible comments. https://phpstan.org/writing-php-code/phpdoc-types

Good:

/**
 * @param Collection<RefillTask> $refillTasks
 */
protected function notifyBatch(Collection $refillTasks): void

Acceptable:

/**
 * @param Collection|RefillTask[] $refillTasks
 */
protected function notifyBatch(Collection $refillTasks): void

#M7 ...add your suggestions

Testing Code Base

Main codebase rules plus...

#T0 - MUST use @test tag for test methods

Good:

/** @test */
public function is_low_when_no_samples_returns_false(): void

Bad:

public function test_is_low_when_no_samples_returns_false(): void

#T1 - MUST use “snake” case in tests

Good:

public function method_returns_zero(): void

Bad:

public function methodReturnsZero(): void

#T2 - SHOULD assert one method/case at a time

Good:

public function bar_method_returns_zero(): void
{
    $class = new \Foo():
    $this->assertSame(0, $class->bar());
}

Bad:

public function everything_is_valid(): void
{
    $class = new \Foo();
    $class->calculate(3, 5);
    $this->assertSame(0, $class->bar());
    $this->assertEquals('status', $class->getStatus());
}

#T3 - Test name MUST be descriptive about use case under test (follow a common pattern)

When testing method call result: <method_name>_<condition>_?<tested object>_<expected result>

When testing some action/behavior: <condition>_?<tested object>_<expected result>

Good:

public function show_page_when_have_multi_device_dispenser_has_all_devices_present_on_page(): void

public function when_dispenser_is_low_page_has_alert_tag(): void

Acceptable:

public function show_page_can_be_seen_by_admin(): void

public function has_organization_relation(): void // i.e. when testing a Model

Bad:

public function files(): void

public function was_created(): void

public function can_see_it(): void

#T4 - SetUp() MUST NOT be used to create models, mocks and faking Events, Notifications etc.

Consider other developers adding tests to files you created, they would have to deal with overhead of created models they do not want to use (.i.e. to make a count() assert against same model).

Also this makes all dependencies explicitly visible in each test and easier to understand.

Main guidline when writing arrange/act/assert stages - to ask yourself: "Can I understand whats going on if I come back here in 6 months?"

Good:

public function my_test(): void
{
  $user = factory(User::class)->create();
  $restroom = factory(Restroom::class)->create();
  $secret = str_random(40);
  config()->set('app.api_secret', $secret);
  
  // your test case...
}

Bad:

public function setUp(): void
{
  parent::setUp();
  $this->user = factory(User::class)->create();
  $this->restroom = factory(Restroom::class)->create();
  $this->secret = str_random(40);
  config()->set('app.api_secret', $this->secret);
}

public function my_test(): void
{
  // your test case...
}

#T5 ...add your suggestions