Sergey Protko
Some stuff
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
Add type checking to existing project
- Configure levels of errors
- Fix errors if you touching file with errors
- Fail only if new error occurred
Use metrics to find interesting places
Opportunities for improvements
PhpMetrics
Documentation from source code
And let's static analysis save some of your time!
That's All!
Static Analysis (4:3)
By Sergey Protko
