PHP_CodeSnifferを導入して、git commit時にチェックする Part 2

2021年01月08日

以下の続きになります。

前回、PHP_CodeSnifferを導入し、Gitのフックを使って、git commit時にチェックが走るようにしました。ただし、前回のシェルスクリプトだとチェックが正しく機能しないパターンがありますので、今回はそこを改善します。

前回のシェルスクリプト

詳細は前回の投稿を見てもらいたいのですが、以下が前回導入したシェルスクリプト（.git/hooks/pre-commit）です。

.git/hooks/pre-commit

#!/bin/sh

./vendor/bin/phpcs --standard=PSR12 --filter=GitStaged .

上記のスクリプトは状況によってチェックが正しく機能しない問題があります。その状況とは「ステージング状態のものと実際のファイルの内容に差がある場合」です。

問題点

問題について具体例を見ながら考えます。

まず、以下のHogeクラスがコミット済みの状態とします。

src/Models/Hoge.php

<?php

namespace Src\Models;

class Hoge
{
}

このHogeクラスにメソッドを追加することを考えます。

src/Models/Hoge.php(to_stringメソッド追加)

<?php

namespace Src\Models;

class Hoge
{
    public function to_string()
    {
        return 'hoge';
    }
}

この変更をコミットしようとすると、追加しようとしているメソッド名が規約違反なので、チェックでエラーとなります。

$ git commit -m "add a method"

FILE: /var/www/html/src/Models/Hoge.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 7 | ERROR | Method name "Hoge::to_string" is not in camel caps
   |       | format
----------------------------------------------------------------------

Time: 69ms; Memory: 6MB

規約違反状態を解消すべく、修正します。

src/Models/Hoge.php(メソッド名修正)

<?php

namespace Src\Models;

class Hoge
{
    public function toString()
    {
        return 'hoge';
    }
}

メソッド名を修正したので改めてコミットします。が、この時、変更分をステージング状態にするのを忘れてしまったとします。すなわち「ステージング状態のものと実際のファイルの内容に差がある」状態です。

$ git diff
diff --git a/src/Models/Hoge.php b/src/Models/Hoge.php
index 5581d9d..a337912 100644
--- a/src/Models/Hoge.php
+++ b/src/Models/Hoge.php
@@ -4,7 +4,7 @@ namespace Src\Models;
 
 class Hoge
 {
-    public function to_string()
+    public function toString()
     {
         return 'hoge';
     }
$ git diff --cached
diff --git a/src/Models/Hoge.php b/src/Models/Hoge.php
index 37e7ad7..5581d9d 100644
--- a/src/Models/Hoge.php
+++ b/src/Models/Hoge.php
@@ -4,4 +4,8 @@ namespace Src\Models;
 
 class Hoge
 {
+    public function to_string()
+    {
+        return 'hoge';
+    }
 }

この場合、ステージング状態のコードは規約違反のままなので、コミット時のチェックでエラーとなって欲しい所ですが、コミットできてしまいます。

$ git commit -m "add a method"
[test e162b8b] add a method
 1 file changed, 4 insertions(+)

これはチェック時に参照するコードが作業ディレクトリ内のsrc/Models/Hoge.phpの内容であり、まさにコミットしようとしているステージング状態のコードを対象としてはいないために発生します。

実際には意図した変更が反映できていないことにすぐに気付くと思うので、実用上は問題ない気もしますが、せっかくなので、チェック時に参照するものをステージング状態のコードにするような改善を行います。

改善したシェルスクリプト

いきなり結論ですが、以下が改善したシェルスクリプト（.git/hooks/pre-commit）になります。

.git/hooks/pre-commit

#!/bin/bash

ret_phpcs=0

while read mode object stage_number file; do
  if [ $stage_number -ne 0 ]; then
    continue
  fi

  git cat-file -p $object | ./vendor/bin/phpcs --standard=PSR12 --stdin-path=$file

  if [ $? -ne 0 ]; then
    ret_phpcs=1
  fi
done < <(git ls-files --stage `git diff --cached --name-only`)

if [ $ret_phpcs -ne 0 ]; then
  exit 1
fi

実際、チェック時に参照するものがステージング状態のコードとなっていることが、確認できます。

$ git diff
diff --git a/src/Models/Hoge.php b/src/Models/Hoge.php
index 5581d9d..a337912 100644
--- a/src/Models/Hoge.php
+++ b/src/Models/Hoge.php
@@ -4,7 +4,7 @@ namespace Src\Models;
 
 class Hoge
 {
-    public function to_string()
+    public function toString()
     {
         return 'hoge';
     }
$ git diff --cached
diff --git a/src/Models/Hoge.php b/src/Models/Hoge.php
index 37e7ad7..5581d9d 100644
--- a/src/Models/Hoge.php
+++ b/src/Models/Hoge.php
@@ -4,4 +4,8 @@ namespace Src\Models;
 
 class Hoge
 {
+    public function to_string()
+    {
+        return 'hoge';
+    }
 }
$ git commit -m "add a method"

FILE: /var/www/html/src/Models/Hoge.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 7 | ERROR | Method name "Hoge::to_string" is not in camel caps
   |       | format
----------------------------------------------------------------------

Time: 47ms; Memory: 6MB

解説

そもそも今回の問題をクリアするには、以下の2つのステップが必要になります。

ステージング状態のコードを取り出す。
取り出したコードをphpcsでチェック。結果の表示は元のファイル名にする。

1. ステージング状態のコードを取り出す

GItの仕組みとしてステージング時にそのファイルのスナップショットが作成されます。なので、このスナップショットを取り出して使うことにしました。

まず git ls-files --stage で対象のファイルのオブジェクト名を調べます（git diff --cached --name-onlyの結果を引数として渡すことで、表示対象を絞っています）。

$ git ls-files --stage `git diff --cached --name-only`
100644 5581d9d01d5f68eedfacc723a182a34e4eeaae46 0       src/Models/Hoge.php

1列目から、ファイルモード、オブジェクト名、ステージ番号、ファイル名となります。src/Models/Hoge.phpのオブジェクト名は 5581d9d01d5f68eedfacc723a182a34e4eeaae46 であることがわかりました。オブジェクト名がわかったのでgit cat-file -pで中身を標準出力に出力できます。

$ git cat-file -p 5581d9d01d5f68eedfacc723a182a34e4eeaae46
<?php

namespace Src\Models;

class Hoge
{
    public function to_string()
    {
        return 'hoge';
    }
}

ということで、git ls-filesとgit cat-fileを使って対象のコードを取り出すことができました。

2. 取り出したコードをphpcsでチェック

次に取り出したコードをphpcsでチェックするのですが、実はphpcsは標準入力を受け取ると、受け取ったものに対して検証を行います。なので、パイプラインを使って

$ git cat-file -p 5581d9d01d5f68eedfacc723a182a34e4eeaae46 | ./vendor/bin/phpcs --standard=PSR12

FILE: STDIN
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 7 | ERROR | Method name "Hoge::to_string" is not in camel caps
   |       | format
----------------------------------------------------------------------

Time: 36ms; Memory: 6MB

という感じで検証できます。ただし、これだとFILE部分がSTDINになっていて、対象が複数ある場合、どのファイルが規約違反なのかがわかりにくいので --stdin-path を使用して識別しやすくします。

$ git cat-file -p 5581d9d01d5f68eedfacc723a182a34e4eeaae46 | ./vendor/bin/phpcs --standard=PSR12 --stdin-path=src/Models/Hoge.php

FILE: /var/www/html/src/Models/Hoge.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 7 | ERROR | Method name "Hoge::to_string" is not in camel caps
   |       | format
----------------------------------------------------------------------

Time: 36ms; Memory: 6MB

ということで、2つのステップはクリアできたので、あとは良い感じにループで回してあげるようにすると、上にあるようなシェルスクリプトになります。

まとめ

前回導入したgit commit時のチェック処理を改善して、コーディング規約違反のコードはリポジトリに存在できないようにしました。これでうっかり規約違反のコードが紛れ込むことがなくなるので、すっきりします。