pre-commit-hookでのphp-cs-fixerの実行

[KentarouTakeda]

概要(Overview・Refs Issue)

pre-commit-hookを用いて、php-cs-fixerによるコード整形をcommit時に自動的に適用。

#4624 (comment) 以降を参照

方針(Policy)

• 実行要件（php + node.js）を満たさない環境の場合は何も起こらないようにする。
• コード整形に失敗した際にcommit不可となってしまうのを避けるため、整形不可のコードがcommit対象だった場合もフック成功とみなす。

実装に関する補足(Appendix)

実行要件

• node.js がインストールされており npm install が行われていること。
• phpがインストールされており composer install が行われていること。
• php / node にPATHが通った状態でcommitが行われること。

以上の要件を全て満たした際に、commit時に自動的にコード整形が行われます。

テスト（Test)

• 確認OS・シェル
  • OS X + zsh
  • Windows 10 + コマンドプロンプト
  • Windows 10 + Windows Power Shell
  • Windows 10 + Git Bash
• 確認事項
  • vendor/bin/php-cs-fixer を実行できない環境でもcommitを行えること（コード整形は行われない）
  • vendor/bin/php-cs-fixer を実行できる環境ではコード整形が自動的に行われた後にcommitが行わること
• 確認していない事項
  • node.js未インストール状態
    この場合pre-commit-hookの呼び出し自体が行われないので確認不要と判断）
  • fixerの実行は可能だが整形不可のイレギュラーなコードをcommitしようとしている
    再現が難しいことに加え、 確認事項 中 〜を実行できない環境 の対応とあわせ「エラーは全て無視」と実装したため確認不要と判断

参考資料

スクリーンショット添付します。

• commitの際に自動的に整形が行われている
• .php_cs.cacheが無い状態でのcommit所要時間
  MacBook Pro (15-inch, 2019) / 1ファイル / 1.4秒

相談（Discussion）

/dev/null へのリダイレクトに相当する処理をクロスプラットフォームで書く方法がよく解らなかったのですごく強引なスクリプトになってしまいました。

'php' は、内部コマンドまたは外部コマンド、
操作可能なプログラムまたはバッチ ファイルとして認識されていません。

環境によって↑のメッセージがcommitの度に表示されるのは回避するためなのですが、
今の書き方はあまり良い方法でない気がします。
他に良い方法ご存知の方がいらしたらアドバイス頂けると助かります。

マイナーバージョン互換性保持のための制限事項チェックリスト

• 既存機能の仕様変更
• フックポイントの呼び出しタイミングの変更
• フックポイントのパラメータの削除・データ型の変更
• twigファイルに渡しているパラメータの削除・データ型の変更
• Serviceクラスの公開関数の、引数の削除・データ型の変更
• 入出力ファイル(CSVなど)のフォーマット変更

レビュワー確認項目

• 動作確認
• コードレビュー
• E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
• 互換性が保持されているか
• セキュリティ上の問題がないか

[okazy]

現在 EC-CUBE 4.0.5 のリリースに向けて準備しています。
プルリクを取り込むのが遅くなる可能性もありますのでご了承ください。

[KentarouTakeda]

了解です。もちろん大丈夫です！

リリース準備もあると思うのですが #4616 が一旦closeとなったことにPR作った後になって気づきました。
こちらのPRを取り込むと、要件を満たした環境に関してはcommit対象ファイルに対し
強制的にfixerが実行 されてしまう ので #4616 ほどのインパクトは無いにせよ
逆に面倒が発生する可能性も出てくるかもしれません。

辺り考えるとどのタイミングで取り込むのかも重要かもしれないので、その辺り含めよしなにご判断ください！

[okazy]

動作確認しました。

いただいた手順でcs-fixerが走ることを確認しました。

node_modulesがインストールされていない場合は普通にcommitできました。

その他

PhpStormのGitクライアントではcs-fixerは走りませんでした。PhpStorm内のreformat機能があるので走らないので問題ないと思います。
ローカルのnode自体が古いバージョンでcs-fixerが走らず焦りましたが v12 にあげたら問題なく実行できました。

[okazy]

@KentarouTakeda
ありがとうございます！取り込みました。