やさしいPHPコーディング規約の導入・完全版

はじめに

こんにちは、社内でコーディング規約おじさんと呼ばれ始めて久しい高野(@fortkle)です！ ここ2ヶ月間ほどに渡って通常の開発業務とは別に社内のアプリケーションにコーディング規約を導入する試みをしており、PHP7 Casual Talks や PHP BLT などのPHP関連の勉強会で都度共有してきました。

今回はそれらをまとめ、共有したいと思います。興味のある方の参考になれば幸いです。

開発効率を阻害するもの

弊社が運営している ママリjp、ママリQといったサービスが順調に成長していっている中、その成長を支える開発チームの人数も少しずつ拡大しています。今後もこの流れは続くと思いますが、エンジニアたるものそういった場合でも「コードの質」は落とさずに成長させたいものです。

弊社では非常に丁寧にコードレビューを実施していますが、質の高いコードを維持し続けるためにはコードレビューをより効率的に実施できる工夫をする必要があるように感じています。開発メンバーが増えれば増えるほど、コードは複雑になり仕様の全体像を把握したエンジニアの数は相対的に少なくなるからです。

そのような状況の中で、弊社では以前から「コードスタイルの些細な違い」がレビューで指摘されることが多々あり、大切ではあるけれど開発効率を阻害しているのではないか、という課題がありました。

コーディング規約の形骸化

前述した課題を解決するため、弊社でも当初からコーディング規約を用意し一部ドキュメント等も残されていましたが、あまり効果的なものではありませんでした。その原因として下記のような点が挙げられます。

原因	説明
チェック機構や自動整形がなかった	コーディング規約を守らせる仕組みを作らないと開発者の善意だけでは継続されずうまくいかない。
独自コーディング規約を導入した	PSRなどの業界標準の規約ではなく独自のコーディング規約を導入しようとすると、チェック・自動整形ツールなど周辺のエコシステムの恩恵を享受できない。
一気に規約を導入しようとして規約エラーが膨大	それまで規約のなかった既存のソースコードに何の工夫もなく単に規約チェックをすると簡単に数千〜数万行のエラーが出てしまうため、規約を守れている状態まで持っていく過程で疲弊していた。

phpcs / phpcbf による自動化とドキュメント整備

まず、開発者の善意に委ねるのではなく仕組みでコーディング規約を導入するために phpcs / phpcbf を使って自動でコーディング規約を守れる仕組みを作りました。

phpcsでコードが規約に違反していないかのチェック、phpcbfで違反部分の自動整形をすることができます。また、phpcs / phpcbfでは共通で使える「カスタム規約」を作ることができます。

弊社では、まず大量に検出されてしまったエラー項目を一時的に全て無効化し、PSR2など理想とする規約をベースに「カスタム規約」を作成しました。次に、期間を区切って徐々に無効化する項目を減らしていくことで、最終的にベースである規約(PSR2)に近づいていくアプローチを取りました。この戦略であれば無理がなく、リリース間近の忙しい時期でも柔軟に対応することができます。

また、コーディング規約を使う理由をチームで振り返り、実現したい価値を最速で実現するためにPSR2などの業界標準の規約に乗り換える選択をしました。

3つの導入ステップ

前提

私がどういった手順でチームに導入していったのかをご紹介します。前提として、採用するコーディング規約はPSR2で、導入したいプロジェクトに composer 経由で phpcs/phpcbf*1をインストールしているものとします。

1. 既存コードからカスタム規約を作る

まずはphpcs/phpcbfで共通して使えるカスタム規約を作りましょう。規約は .xml 形式で作成します。詳細は公式のドキュメントを参考にしてください。ここではシンプルにPSR2を継承しただけのカスタム規約を作ります。名前を仮に c-psr2.xml としアプリケーションのルートに保存します（この時点ではPSR2を指定したのと同じ状態です）。

<?xml version="1.0"?>
<ruleset name="Custom_PSR2">
  <description>Custom ruleset Based on PSR2</description>
  <rule ref="PSR2" />
</ruleset>

次に ライブラリやテスト対象としたくないディレクトリなどを除外対象に指定します。

<?xml version="1.0"?> 
<ruleset name="Custom_PSR2"> 
  <description>Custom ruleset Based on PSR2</description> 

  <!-- 除外したいファイル、ディレクトリを列挙 --> 
  <exclude-pattern>*/vendor/*</exclude-pattern>
  <exclude-pattern>*/app/Console/cake.php</exclude-pattern>
 
  <rule ref="PSR2" /> 
</ruleset>

次に phpcs の下記コマンドを実行しましょう。現時点のカスタム規約（＝PSR2）に引っかかっているエラー項目が列挙されます。

$ vendor/bin/phpcs -s --report=source --standard=./c-psr2.xml ./

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
----------------------------------------------------------------------
    SOURCE                                                       COUNT
----------------------------------------------------------------------
[x] Generic.WhiteSpace.DisallowTabIndent.TabsUsed                71
[ ] Generic.Files.LineLength.TooLong                             23
[x] Squiz.WhiteSpace.SuperfluousWhitespace.EndLine               16
[x] PSR2.Classes.ClassDeclaration.OpenBraceNewLine               15
[x] PSR2.Classes.ClassDeclaration.CloseBraceAfterBody            13
[x] Squiz.ControlStructures.ControlSignature.SpaceAfterKeyword   8
[x] PSR2.Methods.FunctionCallSignature.MultipleArguments         8
[x] Generic.Functions.FunctionCallArgumentSpacing.TooMuchSpaceA  5
[x] Squiz.ControlStructures.ControlSignature.SpaceAfterClosePar  4
[x] Squiz.WhiteSpace.ControlStructureSpacing.SpacingBeforeClose  1
[x] Squiz.WhiteSpace.ControlStructureSpacing.SpacingAfterOpen    1
----------------------------------------------------------------------
A TOTAL OF 165 SNIFF VIOLATIONS WERE FOUND IN 11 SOURCES
----------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SOURCES AUTOMATICALLY (142 VIOLATIONS IN TOTAL)
----------------------------------------------------------------------

Time: 1.71 secs; Memory: 23.75Mb

ここでおもむろに検出された項目を全て除外項目としてc-psr2.xml で指定します。

<?xml version="1.0"?> 
<ruleset name="Custom_PSR2"> 
  <description>Custom ruleset Based on PSR2</description> 
 
  <!-- 除外したいファイル、ディレクトリを列挙 --> 
  <exclude-pattern>*/vendor/*</exclude-pattern> 
  <exclude-pattern>*/app/Console/cake.php</exclude-pattern> 
 
  <!-- PSR2をベースとするため継承 --> 
  <rule ref="PSR2"> 
    <!-- 除外したい項目を列挙 --> 
    <exclude name="Generic.WhiteSpace.DisallowTabIndent.TabsUsed"/> 
    <exclude name="Generic.Files.LineLength.TooLong"/> 
    <exclude name="Squiz.WhiteSpace.SuperfluousWhitespace.EndLine"/> 
    <exclude name="PSR2.Classes.ClassDeclaration.OpenBraceNewLine"/> 
    <exclude name="PSR2.Classes.ClassDeclaration.CloseBraceAfterBody"/> 
    <exclude name="Squiz.ControlStructures.ControlSignature.SpaceAfterKeyword"/> 
    <exclude name="PSR2.Methods.FunctionCallSignature.MultipleArguments"/> 
    <exclude name="Generic.Functions.FunctionCallArgumentSpacing.TooMuchSpaceA"/> 
    <exclude name="Squiz.ControlStructures.ControlSignature.SpaceAfterClosePar"/> 
    <exclude name="Squiz.WhiteSpace.ControlStructureSpacing.SpacingBeforeClose"/> 
    <exclude name="Squiz.WhiteSpace.ControlStructureSpacing.SpacingAfterOpen"/> 
  </rule> 
 
</ruleset>

すると再度実行してもエラーが検出されなくなりました *2 。PSR2をベースに違反した項目が除外されている状態です。これで「最も緩いカスタム規約」が完成しました。

2. 規約違反が増えない仕組みを作る

定期的に自動チェックする

弊社ではTravisCIでCIを回しているので今回はpushやPull Request(以下、PR)のタイミングに合わせてTravisCI上で実行するようにします。JenkinsやCircleCIなど他のCIツールでも同様の事ができるので適宜使っているツールに合わせて読み進めて下さい。

TravisCIの場合、単純に実行コマンドを .travis.yml に記述するだけです。弊社ではPR開発を採用しているので万が一、新たなエラー項目が発生した場合でもTravisがエラーを吐いてマージすることができなくなります。これで新しくエラー項目が増えることはなくなりました*3。

language: php 
 
php: 
  - 7.0 
 
sudo: 
  false 
 
cache: 
  directories: 
    - $HOME/.composer/cache 
 
before_script:
  - vendor/bin/phpcs --standard=./c-psr2.xml ./

ドキュメントを整備する

ここまでの作業でエラー項目が新たに増えることはなくなりましたが、具体的に何が正しくて何が間違った書き方なのかが現時点ではいまいち分かりません。このままではphpcsのエラーが頻発し修正時間が増えることが予想されます。

そうならないために、コーディング規約に関するドキュメントを社内情報共有ツールにまとめ、共有しました。「この1ページに書かれていないことは守らなくてOK！」という前提で書かれたドキュメントなので「そこまで書かなくても」というぐらい詳細なレベルまで規定しています。これによりそもそもphpcsでエラーになる頻度が少なくなりました。

↓目次

↓ if 関連の規約

3. 規約違反を減らす仕組みを作る

弊社ではスプリント毎に無効化している規約の中からいくつか有効化する項目を選んで対処するようにしました。しかし、項目によっては1つ有効化しただけでスペースやインデントの違いなどによって数十行〜数百行の規約違反が検出される場合が少なくなく、これを手動で1つ1つ修正していくのはミスも発生する上にあまりに非創造的です。

こういう単調な作業は出来る限り機械に任せましょう。そんな時に役立ったのが phpcbf です。phpcbf はphpcsと同じカスタム規約(c-psr2.xml)を使って自動整形することが出来ます。またコマンドがphpcsとほぼ同じなのも嬉しいところです。

ではさっそくphpcbfを使ってコードの自動整形をしてみましょう。試しに c-psr2.xml で無効化している項目の中からいくつか選んで記述を消し、項目を有効化*4してから下記コマンドを実行します。大量の処理メッセージが出力されますが git status で変更したファイルを確認することができます。

$ vendor/bin/phpcbf --standard=./c-psr2.xml ./app

念のため git diff で自動整形した箇所を確認し問題なければコミットしてしまいましょう。あわせてコードレビューをしてもらえば大きな障害が発生する確立をかなり抑えられるはずです。

この手順を「毎スプリント」など決められたタイミングで実行すれば少しずつコーディング規約に準拠していくことができ、最終的にはPSR2に準拠したコードに修正することができます。

実際にやってみて

弊社では上記手順に沿って既存のコードベースにPSR2を実際に適用しました。結果は、2ヶ月程度の短い期間でほぼ全ての項目について有効化できています*5。 どうしても優先しなくてはいけないリリースなどが立て込んだ時期に、有効化する項目の数を減らして柔軟に調整できたのも今回ご紹介した方法のメリットだと思います。

さいごに

いかがだったでしょうか。コーディング規約は導入した瞬間に価値が出るものではありませんが中長期的に見て、コーディング規約によってコードの品質が向上し、メンテナンス性や複雑性などが改善することが期待できます。今まで導入を避けていた方もぜひ今回の"やさしい"コーディング規約の導入を試していただければ幸いです！

*1:https://github.com/squizlabs/PHP_CodeSniffer

*2:たまにエラー項目を除外してもエラーが出続ける場合がありますがその場合は親の項目名で指定しましょう。「Squiz.ControlStructures.ControlSignature.SpaceAfterClosePar」なら「Squiz.ControlStructures.ControlSignature」にすると親の項目名による指定になります（.まで削る）

*3:もちろんローカルでも各開発者がコマンド実行したり、適宜Linterの設定もしました

*4:有効化した項目は開発メンバーに随時共有してあげると丁寧だと思います

*5:CakePHPを使っているので名前空間周りのPSR2規約に対応できないため