TL;DR
- PHPコードにおける、constはpublic/privateつけようね、とか、戻り値type hintingは、型名の前にspace入れようね、みたいな話、それphpcsに指摘してもらおう
- quizlabs/php_codesniffer と slevomat/coding-standard を使用する
- 細かくコーディングルールを拡充して、機械化を加速できる
どうすればいいか
なるべく設計的な意思決定があまり介在しないような定型的なレビューを人間がやるのは、レビュワーもレビュイーも両方疲弊してしまう。その解決案として、slevomat/coding-standardを使うのが手としてあります。これは、quizlabs/php_codesniffer のsniffs(検証ルールのようなもの)を様々提供しているライブラリです。
これを使い始めるのはかんたんで、composer require --dev したあとに、
$ composer require --dev slevomat/coding-standard
rulesetにconfigとして追加する。
ruleset.xml
<?xml version="1.0" encoding="utf-8" ?> <ruleset name="php-pj"> <description>ruleset based on PSR2</description> <exclude-pattern>vendor/*</exclude-pattern> <rule ref="PSR2"> </rule> <config name="installed_paths" value="../../slevomat/coding-standard"/><!-- relative path from PHPCS source location --> </ruleset>
まず、<config>により、slevomat/coding-standardが提供するルールを認識できるようにする。
<config name="installed_paths" value="../../slevomat/coding-standard"/><!-- relative path from PHPCS source location --> ですね。
以降は、 <rule> に使いたいものを指定していきましょう。
こういうのを指摘してもらえばいいんじゃない
さまざまなルールがあるが、個人的にこれは現場でも使おう、と思ったものをいくつかピックアップします。
定数定義には可視性指定しよう
SlevomatCodingStandard.Classes.ClassConstantVisibilityを使用すると、定数定義にて public または private を指定しているかどうかを指摘してくれる。
例えばこういうコードだった場合に、
const HOGE = 'hoge';
こう指摘される
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
31 | ERROR | Constant \Hoge\Huga::HOGE
| | visibility missing.
----------------------------------------------------------------------
次のようなコードに直すように、チームみんなで統一することができる。
private const HOGE = 'hoge';
default nullはnullableだよ
SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue を使用すると、デフォルト値をnullとしている引数の型宣言が nullable になっているかをチェックしてくれます。
private function getDateFromFormat(string $date = null)
これは、こう指摘される。実質デフォルト null にしている引数のtype hintingは nullable なのでこちらのほうが健全な感じはある。
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
1291 | ERROR | [x] Parameter $date has null default value, but is
| | not marked as nullable.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
戻り値のタイプヒンティング前の space
SlevomatCodingStandard.TypeHints.ReturnTypeHintSpacing を使用すると、戻り値のタイプヒンティングの前にspaceが入っているかをチェックしてくれます。
こういうコードがあるとする、
public function hoge():void
これは、こう指摘される
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
907 | ERROR | [x] There must be exactly one space between return
| | type hint colon and return type hint.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
これまでピックアップしたものを設定すると次のようなruleset.xmlになる。
<?xml version="1.0" encoding="utf-8" ?> <ruleset name="php-pj"> <description>ruleset based on PSR2</description> <exclude-pattern>vendor/*</exclude-pattern> <rule ref="PSR2"> </rule> <config name="installed_paths" value="../../slevomat/coding-standard"/><!-- relative path from PHPCS source location --> </ruleset> <config name="installed_paths" value="../../slevomat/coding-standard"/><!-- relative path from PHPCS source location --> <rule ref="SlevomatCodingStandard.TypeHints.ReturnTypeHintSpacing"/> <rule ref="SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue"/> <rule ref="SlevomatCodingStandard.Classes.ClassConstantVisibility"/>
その他
実行時の注意点として、ローカルのグローバルなphpcsで実行すると認識できないので、ちゃんと ./vendor/bin/phpcs を使用しましょう。composer scriptなりから呼ぶと自然とそのレポジトリ内のものを使用するのでよき。