Sider Blog

コードレビュー自動化サービス「Sider」を運営するSider株式会社のコーポレートブログです。

PHP向け解析ツールPhinderのご紹介

Siderでは新しいPHP向け解析ツール「Phinder」の提供を開始しています。PhinderはPHP向けのプロジェクト固有のルールを気軽に追加できるオープンソース解析ツールで、私たちが開発してきたQuerlyGoodcheckなどのツールと同様の哲学に基づいています。

Phinderを使うと、簡単にチーム向けにコードレビューで検査したいルールを追加することができます。チームが必ず従わなくてはいけないルールを自動で検査するLintツールと言うよりは、チームの事情を考慮した上での知識やベストプラクティスの共有を補助することが目的です。Phinderでは、次の様にルールを書くことができます。

gist.github.com

Phinderのルールは patternmessage の組です。PhinderはPHPコードを走査し、 pattern にマッチするコードを発見したときに message を表示します。

f:id:soutaro:20190111155546g:plain

ルールを定義して、リポジトリに追加し、Siderでチェックして、コードレビューを改善しましょう。

QuerlyやGoodcheckについてご存じの方であれば、Phinderがそれらのツールと全く同じメカニズムに基づいていることがわかるはずです。

プロジェクト固有の知識

「コードレビューをツールで効率化する」という話題になると、私たちは一般的なベストプラクティスに注目しがちです。つまり、その言語とかフレームワークで書かれたプログラムであれば、全てが従うべき、そういう習慣について考えてしまいます。こういった目的には、PHPMDやPHP_CodeSnifferなどのツールが活用できます。これらのツールは、ベストプラクティスに反するコードを自動で検出し、開発者が修正することを助けます。

しかし、プロジェクトが進んでコードベースが大きくなると、このような「一般的なベストプラクティス」よりも「プロジェクト固有の知識」が、コードレビューにおいてより重要になっていきます。プロジェクトの中で提供されているAPIの一部を安全に使うには特別な注意が必要になったりしますし、その状況を他の開発者にもれなく伝えるのはなかなかに苦労します。

f:id:soutaro:20190111160552j:plain
プロジェクトの知識に関する正確で科学的なグラフ

問題は、一般的なLintツールでは、こういったプロジェクトに固有の知識を上手く取り扱うことができないことです(なぜなら、Lintツールはあなたのプロジェクトのために開発されたものではないから)。多くのツールでは、こういった状況のためにプラグインを開発できるようになっていますが、5名程度で開発しているプロジェクトのためにわざわざプラグインを開発するのはなかなか正当化するのが困難です。

Siderは、この問題、つまり「開発チームがチーム内で知識を上手く共有すること」に集中しています。「コードレビュールールブック」とか「レビューチェックリスト」を作るのではなく、Phinderにルールを追加しましょう。Phinderは、自動で検査してくれます。

使い方の例

Phinderをどのように活用すれば良いのか、いくつかの例を示します。ここに書かれているルールはあくまで例であり、パターンやメッセージは個々のプロジェクト向けにカスタマイズが必要です。

秘密情報の取り扱いに関する警告

User クラスに oauthToken() メソッドが定義されているとします。このメソッドはユーザーのOAuthトークンを返します。トークンは秘密情報なので、ログに書き出されたりしないように特別な注意が必要です。oauthToken() メソッドの呼び出しを見つけたときに、警告を出してみましょう。

gist.github.com

Phinderルールの _ は任意のPHPの式にマッチします。このパターンは $user->oauthToken()$this->currentUser()->oauthToken() にマッチしますが、 oauthToken() 関数の呼び出しにはマッチしません。

過去に発生した障害の参照

過去に、 PostCategory::import_records(associations) メソッドの呼び出しによって、サービス障害が発生したとします。このメソッドは、複数のレコードをバルクインサートするため個々のレコードを一つずつINSERTするよりも高速ですが、デッドロックを引き起こすことがあり、過去にサービスが停止する障害を引き起こしたことがありました。このメソッドが必要になることはありますが、呼び出すときには開発者・レビュアーは障害に関するレポートを確認して、障害が再度発生することがないようにしたいです。

gist.github.com

おそらくここでは「関連するレコードについてロックが取られているか」「ソートされているか」「タイムアウトなどは適切か」などの事前条件が必要となりますが、このような事前条件をツールによって自動で検査するのは困難です。Phinderは、それを諦めてしまい、単に開発者・レビュアーに注意喚起をします。現実的には「単にチェックするのを忘れていた」ことによって問題は発生するので、このくらいでも役に立ちます。

新しいAPIのアナウンス

Postテーブルのレコードを取得するための新しいAPIを追加したとしましょう。この新しいAPIでは、関連する tagsauthors を自動で同時に取得します。既存の古いAPIの呼び出しを一気に置き換えてしまえるなら話は簡単なのですが、一部では tags を取得しないAPIが欲しかったりするので、簡単ではありません。こういう状況では、Phinderを使って新しいAPIの存在をアナウンスしていきましょう。このルールでは、ORMネイティブのAPIを使ってPostを取得しているコードを発見したときに、新しいUsers::posts()を使うようにメッセージを出します。

gist.github.com

このルールには justifications というフィールドを含めています。このメッセージを無視してもかまわない状況、つまりUser::posts()を使うことができない状況について、説明を追加することができます。

Phinderを使うには

一番簡単な方法はSiderを使ってみることですが(笑)、Phinderをローカルでインストールして試してみることもできます。PhinderのWebサイトに説明がありますので、どういう動作になるのか、実際に試すと理解できると思います。

PhinderとSiderを組み合わせて、コードレビューをもっと効率的に進められるようになることを、そしてもっと自信をもってデプロイできるようになることを期待しています。なにか質問やフィードバックがあったら、気軽にお聞かせください。

PHP以外の言語では?

Ruby向けにはQuerly、特定の言語に基づかずに正規表現でルールを書くにはGoodcheckが使えます。Goodcheckは、正規表現ベースなのでPhinderやQuerlyに比べると表現力が弱いのですが、任意のテキストファイルに対して使えて、意外と便利です。

リンク


この記事は英語版Siderブログに公開された記事からの翻訳です。