Static World

Dynamic World

Static Analysis

get properties of program without executing it

What properties?

Is this code use proper code style?
Does this code works?
What complexity this code have?
How often this method is changed?
Who is using this method?
...

Code Review Goals

Knowledge sharing
Opportunities for improvements
Double check for stupid mistakes
Preserve coding standards of project

Consistent code style

improves readability

class UserResource{

/**
  * Transformer for user resource
 * @return array<id: int, name: string, age: int, score: float>
*/
public function default(User $user, Scores $scores) {
   return [
     'id' => $user->id(),     'name' => $user->name(),
     'age' => $user->age(), 'score' => $scores->forUser($user),
   ];
}
}

It's too boring to maintain consistent code style

class UserResource
{
    /**
     * Transformer for user resource.
     *
     * @return array<id: int, name: string, age: int, score: float>
     */
    public function default(User $user, Scores $scores)
    {
        return [
            'id'    => $user->id(),
            'name'  => $user->name(),
            'age'   => $user->age(),
            'score' => $scores->forUser($user),
        ];
    }
}

So let's automate it.

php-cs-fixer, php code sniffer, prettier-php

Coding standards

is not only about code style

Acyclic dependencies principle

Type Checking

PhpStorm
Phan
PhpStan
Psalm

from Rasmus Lerdorf

Statically typed languages just too verbose

Don't turn PHP into Java!

I don't need type checking, I have tests!

ERROR: UndefinedMethod - 11:1 
Method Bar::foo does not exist

Text

Type inference

function tokenize(string $expression)
{
    $groups = ['number', 'operation'];
    $regex = '/(?<number>\d+)|(?<operation>[\+\-\*\/]?)|\s+$/';
    preg_match_all($regex, $expression, $matches, PREG_SET_ORDER);
    $tokens = [];
    foreach ($matches as $match) {
        /** @var string[] $lexem */
        $lexem = array_intersect_key($match, array_flip($groups));
        foreach ($groups as $group) {
            if (
                !isset($lexem[$group])
                || '' === $lexem[$group]
            ) {
                continue;
            }

            $tokens[] = [
                'type'  => $group,
                'value' => $lexem[$group],
            ];
        }
    }

    return $tokens;
}

More complicated example...

/**
 * @return string[][]
 *
 * @psalm-return array<int, array{type:string, value:string}>
 */
function tokenize(string $expression): array
{
// ...

Psalter

May fix some of your typings

/**
 * @return string[][]
 *
 * @psalm-return array<int, array{type:string, value:string}>
 */
function tokenize(string $expression): array
{
// ...

No standard way to express types

<?php
declare(strict_types=1);

function sum(int $a, int $b): int {
    return $a + $b;
}

$tokens = tokenize('2 + 3 * 2');

sum(
    $tokens[0]['value'], 
    $tokens[0]['value']
);

A bit simpler example

It's not perfect...

But PhpStan could catch it!

If all types defined...

function processFile(string $content): array 
{
    // do something with it
}


processFile(
    file_get_contents($filePath)
);

No errors found!

Fatal error: Uncaught TypeError: Argument 1 passed to processFile() must be of the type string, boolean given

Type checking is imprecise

dynamic testing is even worse

CS 4723: Lecture 5, Test Coverage

Add type checking to existing project

Configure levels of errors
Fix errors if you touching file with errors
Fail only if new error occurred