はじめに
こんにちは。初投稿のneueと申します。
以前はフロントエンド関連の業務に従事していましたが、スタイル・エッジ・グループへのJoin後は、
Webインフラ運用やシステムの新規開発など、越境もあり学びの日々を過ごしています。
現在はというと、組織の開発体験向上のための、環境の開拓＆刷新がメインのプロジェクトに参加しています。
今回は、そちらで取り組んだ施策の1つ「自動コードチェックの強化」について、どのようなことを考えながら進めたかを振り返ります。
きっかけ
きっかけは直近参加していた新規開発のプロジェクトにて。
弊社では、コードレビュー文化が強く根付いていることもあり、多くのコメントが飛び交っていました。
内容を見ていくと、設計思想に関する意見交換や、視野の補完のためのアドバイス、ベターな命名・実装方法などが主ではあるものの、中にはケアレスミスの指摘なども混在しています。
塵も積もれば…と言いますので、明白な解のある修正点は、レビュアーの手へ渡る前に処理できればと考えました。
現状の確認
策が全く講じられていないということはなく、PHP_CodeSniffer（PHPコードの標準違反検出）は導入済みで、基本的なコードフォーマットは自動で適用されていました。
ただし、
- 標準として扱うコーディングスタイルが古め（PSR-2）
- 追加のルール定義が最低限
- 動作タイミングがコミットフックかつ自動整形可能なエラーの修正しか行われていない
といった導入状況だったため、今回はコード標準化の第一歩として、
- ルール定義の最新化・詳細化
- 導入・運用方法の見直し
を実施することにしました。
過去にESLintとStylelintのルール定義構築経験もあったので、割と気楽に考えていました。この時は…
使用方法の理解
使用方法を理解するにあたり、まずは公式ドキュメントであるWikiを確認します。
など、必要そうな情報は押さえられたものの…
各ルールの解説ページ（このようなものやこのようなもの）が存在しないことに不安を覚えます。
概念の把握
続いて、PHP_CodeSnifferにおける各種概念をざっくり把握します。
「ルール」が最小の単位ではない所がポイントです。
単位
- ルールセットはXMLで管理（Standardと呼ばれる）
- ファイル内で実行時オプションも指定できる
- ルールはファイル単位で定義されている（Sniffと呼ばれる）
- 1つのルールの中に、複数のチェック項目が存在している（呼称は不明）
ルールセット（Standard）
- 他のルールセットを丸ごと有効化できる（継承に近い）
- ルール単位、ルール内のチェック項目単位での有効化ができる
- 有効化した範囲のうち、一部を無効化できる
- 検出・自動整形のどちらかだけを無効化できる
- PSR-2やPSR-12など汎用的なものがパッケージに組み込まれている
ルール（Sniff）
- 名前空間として、いずれも特定のStandardに所属している
- 一部のルールに制御用のオプションが用意されている
- 修正方法が明確なもののみ、自動整形ができる
- 注意点として、異なるルールでチェック範囲がやや重複しているものが存在する
- 相反するルールの整形が有効化されている場合、ループが発生する恐れもある
ルールセットの構築（流れ・マインド）
全体としては、このような流れで進めていきます。
最初から精査しすぎず厳しめに作って、現実（生きているコード）を見せながら軟化させていく形です。
- 土台とするルールの決定
- 個別ルールをローラーして必要そうなものを追加
- 実プロジェクトのコードで検証し、不都合なものを除外
ポイント
ここで作成するベースに完璧を求めすぎないことも重要です。
実際に使用するプロジェクト側で、ルールの調整や部分無効化を可能にすることを前提として、気楽に。
- あくまでも土台となる1つの標準であること
- プロジェクトごとのイレギュラーなコードを考慮しすぎないこと
- 要望に応じて見直しは可能である（システムへの影響は及ぼさないので変更ハードルも高くない）こと
ルールセットの構築（実作業）
土台とするルールの決定
非推奨となっているPSR-2からの変更先として、PSR-12を選択します。
XMLファイル内に、各項目をどのルールを用いて検出しているかが細かく書かれているため、参考にします。
個別ルールのローラー
PSRや特定のフレームワークに紐付かない項目として、Generic、Squizのルールを1つずつ確認します。
冒頭での不安は的中し、ソースコードを読み解く必要が出てきたことで序盤は探り探りでしたが、
手順を確立してからは併読支援のブックマークレットを作り、時間短縮できました。
- 組み込み済のPSR-12ルールセットで有効化されていないルールであることを確認
- publicのメンバ変数がある場合は、プロパティ一覧に記載があるはずなので制御内容を把握
- ソースコード中の
add(Fixable)?(Error|Warning)(OnLine)?（＝チェック項目）を確認 - チェック項目名と前後の処理やコメントを読んで（ざっくり）チェック内容を把握
- イメージが付かなければ、テストファイル（Invavlidパターン例とValidパターン例）を確認
- それでもイメージが付かなければ、サンプルコードを書いてエラー扱いになる内容を確認
- 導入できそうなルールであれば有効化（怪しいものも目印コメント付きで一旦有効化）
ルールセットの検証
実プロジェクトで使用しているコードに対して試験導入し、ルールを見直します。
※便宜上ルールと記載していますが、チェック項目単位で制御可能です。
導入直後は大量のエラーが出力されることが予想されます。
出力量が多い状態を維持して確認を続けると、待ち時間や見落としが発生するため、やや工夫が必要です。
- 最初にInformation Report形式で出力して、全体的な傾向を確認
- 発生件数が多いエラーから確認
- 確認を終えたルールは一時的に無効化
- 実行時間短縮のため、連続する検証時は適宜対象ディレクトリを絞る
- ※広く発見→狭く解決→広く解決できているか確認→狭く解決→…
自動整形の結果が望ましくないルールの無効化
使用時のイメージと合わせるため、先に自動整形可能なルールから検証します。
自動整形の結果が問題ない状態となるまで、適用→ルール調整→復元を繰り返します。
- 当該ルール・整形後状態の確認
- 自動整形を実行
- git diffを出力（整形前後の差分を記録）
- git resetで整形実行前に戻る
- チェックを実行
- 記録した箇所に関するチェック項目を確認
- あるべき姿に手動調整
- 再度チェックを実行
- エラー扱いになる：項目自体を無効化
- エラー扱いにならない：自動整形のみを無効化
競合するルールの無効化 ※整形ループが生じた場合
整形内容の組み合わせによっては、ループが生じてしまう（状態A→状態B→A→B→…）ことがあります。
-vvオプション付きで自動整形を実行すると、処理の流れをトレースできるため、どちらか一方のルールを無効化します。
エラー内容の精査・対応
残りのエラーについて、なぜエラーになっているかを確認し、対応方針を検討します。
- コードに問題がある
- ルールは維持（コードを修正する想定）
- 可読性目的でコードの書き方を工夫した箇所が引っかかる
- ルールは維持（コメントでチェック対象から除外する想定）
- 全体的に適合が難しい
- ルールを無効化
- 検証中のプロジェクトにおいてではなく、今後新規開発する場合を基準に検討
- 解析の精度が低いのか、意図しない状態になる
- ルールを無効化
- 比較的最近導入されたルールや、複雑なコードの組み合わせなどで発生
- サポートされたら復活できるよう、経緯付きでコメントアウトする（issueがあればリンクも添えておく）
- ルールは問題ないが特定領域のみエラーが多発している
- ルールは維持するが、除外パスのパターンを追加
- 自動テストやDBシーディングなど、命名規則や記述方法がやや特殊になる範囲などで発生
導入・運用方法の見直し
ルールセットは、ある程度実用的なものになったので、PHP_CodeSnifferの導入方法を見直します。
プロジェクトへの導入
導入や運用開始後のアップデートを使い慣れた方法で実現させるため、Composerパッケージとして構築します。
PHP_CodeSniffer及び関連パッケージや設定ファイルをそれぞれ手動でプロジェクトへ導入するのではなく、1つのパッケージとして纏めることで、以下が実現できます。
以下の手順だけで、導入が可能になりました。
インストール
- composer install
- composer scriptsにphpcs, phpcbfへのaliasを追加
- XML雛形をプロジェクトルートにコピー
- 独自調整があれば、プロジェクトの優先ルールをXMLに指定
アップデート
- composer update
- 不都合な差分があれば、プロジェクトの優先ルールとして調整
エディタとの連動
エディタ上でファイルを保存した際に、当該コードのハイライト＆自動整形が行われるようにします。
PhpStorm
標準サポートしているため、公式ドキュメントを参考に設定します。
VS Code
標準ではサポートしていないので、PHP Sniffer & Beautifierを導入します。
全プロジェクトで自動実行されないようグローバル設定上は無効にし、ワークスペース単位で有効化します。
Gitコミットフック
元は自動整形の適用のみでしたが、差分ファイル内で違反が残存している場合はコミットが中止される強固な形に調整します。
動作イメージ
実際にエディタ上で保存すると、自動整形→残りのエラーが問題パネルに表示されます。
おわりに
いかがでしたでしょうか。
この記事を書いている時点では試験運用のタイミングのため、まだ具体的な効果は見えていませんが、コードレビューの時間が少しでも有意義に使えるようになることを願って、継続的にメンテナンスしていこうと思います。今回は既存ルールのみでしたが、頻出する指摘があれば独自ルールの作成も効果的かもしれません。
☆スタイル・エッジ・グループでは、一緒に働く仲間を募集しています☆
環境改善が好きな方など、もし興味を持っていただけましたら、採用サイト↓も覗いてみてください！
