📑

PHPConf2023「保守性の高いアプリケーションを作り続けるための基盤整備について」というテーマで登壇します

2023/10/07に公開

Introduction

こんにちは、オープンロジ技術開発部 SREチームのたけてぃ(@takeokunn)です。 株式会社OPENLOGIを代表して PHPConference東京 2023で登壇する内容についての解説と補足を書いていきます。

弊社はPHPエンジニアを積極的に採用していますので興味のある方は是非是非気軽にカジュアル面談を受けてください。
https://corp.openlogi.com/recruit/

また、弊社はPHPカンファレンスのプラチナスポンサーであり、当日企業ブースを構えているので是非気軽に遊びにきてください。
ノベリティにカレーとハッピーターンを用意して待っています。

なお、この記事の執筆も登壇スライド作成も当日イベントの参加も 全て業務扱いでした 。 こういう技術イベントへのサポートがあるのは今まで個人で参加してた身としては嬉しい限りですね。

登壇について

レガシーLaravel開発術: 保守性の高いアプリケーションを作り続けるための基盤整備について」というタイトルで登壇します。

2023/10/08 14:15〜
トラック3 - 4F コンベンションホール 梅
スポンサーセッション(25分)

https://docs.google.com/presentation/d/e/2PACX-1vReStbxschS9awE6-aDF1q-fNvxFJmYEHt1gtwQPaST7HffRbwTwyNDCDdmnQY0kcNp-S-vOTAyos8b/pub?start=false&loop=false&delayms=3000

詳細な解説について

各ツールの概要や使い方については尺の都合上詳細に解説していませんのでご了承ください。

話すこと/話さないこと

会社概要

オープンロジは「物流版AWS」目指しサービスを展開しています。
「"今日" 頼めば "明日" 届く」のが当たり前になりつつある物流サービスですが、物流インフラの制度疲労が起きており、今のままでは雇用人数を増やしても、サービスを維持出来ない未来が見えています。

制度疲労の背景は、EC事業者や倉庫事業者など購入~配送までそれぞれの過程で課題があり、
各ステークホルダーが個別最適をしてしまっているため、全体最適につながっていないことが原因です。

そこで、オープンロジではEC事業者と倉庫事業者に共通のプラットフォームを提供し、物流を全体最適するプロダクトを提供しています。

AWSが世の中に登場してサーバーを立ち上げることが簡単になったように、EC事業者が物流に関してはオープンロジに任せておけば大丈夫な状態。
サーバーレスならぬ「物流レス」を目指し「物流版AWS」というサービスコンセプトを掲げています。

会社は今年で創業10周年で、従業員数138名、開発組織の人数は45名規模となっています。

システム概要

弊社は100万行以上あるLaravel(openlogi-api)と、一部受注連携機能を切り出した10万行Laravel(order-sync)の2つのレポシトリを主に使っています。 2023年9月時点のざっくりとしたバージョンとコード行数は以下です。 order-syncはPHP部分のみ切り出しているのでフロントエンドはありません。

PHP ver Laravel ver Node.js ver React ver app/ 行数 tests/ 行数 React行数
openlogi-api 7.4 7.3 16 18 40万行 42万行 24万行
order-sync 8.1 9.2 - - 4万行 5.5万行 -

openlogi-apiは以下の4つの機能を備えていて、通常のよくあるモノリシックな開発体制になっています。

  • WMS(倉庫マネジメント機能)
  • Portal(荷主向け管理画面機能)
  • OrderSync(受注連携機能)
  • Billing(請求管理機能)

openlogi-apiの開発者数は30人弱です。 日中倉庫が稼動している為頻繁にリリースすることができないので月2回第2第4火曜日の夜にメンテを入れて行っています。

本記事ではopenlogi-apiについて対応したことについて書いているのでご了承下さい。 詳細を省きますが、order-syncの方もついでにPHPStanをlevel max + αにしてRectorも厳しめのRuleを通す対応をしました。

SREチームとしての役割

前回PHPerKaigi 2023で「約10年もののPHPアプリケーションとの付き合い方と、今後10年改善し続けるための取り組みについて」というテーマで弊社のSREチームについて話しました。 Key Conceptとして「運用の効率化」「性能の向上」「品質の向上」「セキュリティの向上」の4つを上げて活動しています。

https://note.openlogi.com/n/n9c6c662121c1?magazine_key=m9f98ae49ed2f

今回の登壇内容は「運用の効率化」「品質の向上」についてフォーカスしています。 「運用の効率化」や「品質の向上」という単語は非常に定義が広いのでもう少し狭めて書くと、「顧客の要求を必要十分に満たすプログラムを継続的に開発できるような環境を弊社のエンジニアに提供する」ということになります。 自分にとっての「顧客」というのは「同僚のエンジニア」であり、「顧客のビジネス(開発)を止めない」というのは非常に重要な制約でした。

後述するPHPStanやRectorなど新しいツールを導入するにあたって如何に負担を減らすか、CIの高速化等をして如何に既存の負担を減らして相殺するか、というのかが今回の肝でした。 自分の改善活動について社内では自分の苗字をとって「Obarize」と呼ばれていました。

私は2023/03〜2023/09の半年間、業務の大体50%を「Obarize」に、もう50%をインフラ業務に充ていました。 今回の登壇内容は約半年間行なったObarizeについてのみなので対応しきれなかった部分も多々ありますがご了承ください。

コード品質改善 何故やるのか

技術的負債の解消やCI整備について、採用や開発組織など様々な文脈で語ることができるのですが、私はあくまで現場のエンジニアなので現場目線で話します。

何故やるかと一言で言えば「機械的に検査できる箇所を増やす為」です。 「開発者が増えた時にスケールさせる為」「依存ソフトウェアを安心してアップデートする為」という2軸を元に進めていました。

開発者が増えた時にスケールさせる為

メリットとしては以下があげられます。

  • 新しく入ったエンジニアが安心して開発に取り組めるようにする
    • PHPStan等で影響範囲を機械的に検出することができる
  • レビューコストを下げる
    • PHPレベルのエラーが既にCIで保証できているのでドメインレベルのレビューのみに集中することができる
  • コーディングルールを機械的に定める
    • 弊社はPHP畑ではないエンジニアも多数在籍している
    • php-cs-fixerやRectorでコーディングルールを機械的に定義することができる

mt_randは絶対使うな」等といったベテランPHP開発者なら当然知ってるノウハウのようなものを逐次伝えるのは不可能なのでCIで出来る限り落とせるように調整しました。

依存ソフトウェアを安心してアップデートする為

依存ソフトウェアとはPHPやLaravel、composer packageのようなものを想定しています。
100万行規模のLaravelに対して影響範囲を調査するのは一苦労です。当然CHANGELOGを読んだ上で変更して動作検証をしますが、機械的に検出できる範囲が広ければ広いほど安心してアップデートすることができると考えています。

コード品質改善 アプローチ方法

ツールの整備をする上で「そのツールは何を保証するのか」というのを明確にする必要があります。

項目 対応 保証しているもの
CI 機械的 ツールを継続的に実行し正常に終了したことを保証する
Linter 機械的 コーディング規約通りかどうかを保証する
静的解析 機械的 プログラム(の型)的に正しいかどうかを保証する
UnitTest 手動 期待した入力に対して期待した出力をすることを保証する
E2E 手動 期待したユーザの動作を期待通り実行できることを保証する
脆弱性診断 自動 外部サービスでWebアプリケーションセキュリティを保証する

上記の項目を大まかに分けると「機械的に検査できる」ものと「手動で行う必要がある」ものがあります。

今回の対応では「機械的に検査できる」ことを可能な限り増やすことが目標です。 CI/Linter/静的解析の整備を進めることによって業務ドメイン以外で問題が起こり得ない状況を作り、自分以外のエンジニアが真にドメインに集中できる環境が理想だと考えています。

ドメインの挙動を保証するにはUnitTest/E2Eを拡充していく以外の方法はありません。 UnitTestやE2Eは手動で書かなければいけない分、どうしても網羅性が欠けていたりコーナーケースを考慮しきれないものです。 工数との兼ね合いでUnitTestを書ききれていない状況もサービスが成熟していないタイミングでは発生しがちですし、実際過去に書かれたものであればあるほどそういうケースが多かったです。 静的解析ツールを整備すればUnitTestを書かなくて良いという訳ではありませんが、経験上かなりの漏れを潰すことが可能だと思っているので価値があると信じています。

コード品質改善 開発環境構築整備

「開発者が増えた時にスケールさせる」という目標と照し合わせても、何はともあれ開発環境構築を整備することが肝要です。 openlogi-apiは自分が入社した時には既にかなり整備が進んでいて、大きく対応する必要がありませんでした。docker-compose.ymlを自分で書かなくて良い現場は幸せですね。

IDEフレンドリーな環境を整える為以下のツールを導入しました。 後述しますが、Eloquent Modelに型アノテーションをつける php artisan ide-helper:models を実行しなければPHPStan導入ができないので必須事項でした。

Vagrantfileとvagrant用の分岐が一部残っていましたが、現在Dockerを使ってるエンジニアしかいなかったので認知コストを下げる為に削除しました。

開発環境で使う為のテストデータをLocalで用意するのに以前は2時間〜2時間半かかっていたのを、以下のような対応をして5〜10分程度で構築できるようにしました。 個人的にイケてる実装ではないと思ってるのでいずれ直したいです。

  • Before
    • マスキングした数GiBのsql fileをs3からdownloadしてlocalのDocker MySQLに流す
  • After
    • docker volume自体をs3からdownloadしてmountする

コード品質改善 CI整備

openlogi-apiでは以下のツールをGitHub Actionsで実行しています。

自分が入社前にPHPUnit等既にある程度CIが整備されていましたが様々な問題を抱えていました。 具体的にはphp-cs-fixerなどlintツールはgit commit-hookで差分のあったファイルに対して運用されていました。 commit-hookだと個人の環境に依存しまうのと、実際に全体に適用するとかなりの変更が出てしまったのでCIレベルで落とすように変更をしました。

また、git pushする度にCIが1時間半〜2時間かかっていました。 pull requestをdevelopにmergeするまでの時間が明らかに遲くなっており開発のボトルネックになっていました。 PHPUnitを直列で全実行していたのが原因だったのでtestsuiteを12分割しました。 この対応により20分〜25分程度で終了するようになりました。 GitHub Actionsは調子によって実行時間がまちまちなので正確な値を出すことは難しいですが、1時間以上短縮したので確実に高速化できたのではないでしょうか。

PHPUnitはECRのbase imageを元に実行していたのですが、この対応によってECRの転送量が跳ね上がり料金的に厳しかったのでGitHub Packages(GHCR)に移行しました。 細かいCacheの設定やソースコードの変更がない場合に実行しないように調整することによって格段に料金を安くすることができました。

phpunitをxdebugを有効にした状態で実行をしてカバレッジを出そうとする非常に時間がかかってしまいます。 そこで毎晩phpunitを定期実行してカバレッジ情報をGitHub Artifactを出力するGitHub Actionsを作成しました。

PHPStanの導入初期に頻繁にdevelop branchでCIが落ちてしまったので、developでCIが落ちた時に発火してslack通知をするGitHub Actionsを作成しました。 どうしても改善活動中はdevelopのCIが落ちがちなので、振り返ると非常に良い取り組みだったと感じています。(最新のdevelopを取り込まずにmergeをすると落ちる可能性がある)

コード品質改善 Linter整備

php-cs-fixer

php-cs-fixer はphp向けのLinterです。 同様のツールにPHP<sub>Codesniffer</sub>がありますが、弊社では全社的にphp-cs-fixerの方を使っています。

以下の対応をしました。

  • 全PHPファイルにphp-cs-fixerを通す
  • php-cs-fixerのversionを2系から3系に移行
  • PSR2 から PSR12 に移行

私が入社した時には既にphp-cs-fixerが導入されていましたが、前述した通りgit commit-hookで動いたので適用されていないファイルが多数ありました。 まずは全ファイルにphp-cs-fixerを適用後、バージョンアップをし、PSR2はDeprecated なので PSR12 に移行しました。

以前は PSR2 に細かいOptionが定義されていたが、最終的にはシンプルな記述に落ち着きました。

$config = new PhpCsFixer\Config();
return $config
    ->setFinder($finder)
    ->setRules(['@PSR12' => true])
    ->setRiskyAllowed(true)
    ->setUsingCache(true);

ESLint/Prettier

ESLintはJavaScriptのためのLinterで、PrettierはJavaScriptのためのFormatterです。 ESLintは静的解析ツールという側面もあるくらい非常に高機能なツールで、Prettierは自動整形をしてくれるので微妙に役割が違います。

以下の対応をしました。

  • Prettierを全ファイルに通した
  • ESLintのrule offを駆使してとりあえず全部動くようにした
  • 機械的に修正できる部分を可能な限り修正した

ESLint/Pretteirもphp-cs-fixerと同様、git commit-hookで動いていたので適用されていないファイルが多数ありました。 また、そもそもESLintはエラーが無視されていて実行すると落ちる状態でした。 とりあえずCIが通るように既存のエラーが出ているruleをoffにしつつ機械的に修正できるruleを直しました。

その他Linter

私がLinterが好きなのもあって、上記Linter以外にもactionlintsecretlintも導入しました。 まだ対応しきれていないのですが将来的にはhadolintStylelint等を導入して、コーディング規約をCIレベルで保証できる部分を増やしていきたいと考えています。

コード品質改善 静的解析整備

PHPStan

PHPStanは2023年現在最も使用されているPHPの静的解析ツールと言っても過言ではありません。 類似ツールにPhanPsalmがありますが、以前PRを出したこともあるくらい私はPHPStanが好きだったというのもあり導入しました。 正確にはPHPStanのLaravel拡張であるLarastanを入れています。

PHPStanはOSSの開発が非常に活発であり、ドキュメントが非常に豊富で、他社の導入事例は枚挙に暇がありません。

PHPStanを導入してからのlevelとbaselineにignoreされているエラー数の表です。
10/6現在、PHPStanのエラーを2500個程度潰して残りが2500個程度という折り返し地点にいます。

日付 タイミング level wms addon billing portal other 合計 差分
2023/03/31 level 0導入時 0 0 0 0 0 465 465 0
2023/04/10 level 1直前 0 0 0 0 0 122 122 -343
2023/04/11 level 1直後 1 0 0 0 0 973 973 851
2023/05/02 level 2直前 1 56 29 202 23 268 578 -395
2023/05/12 level 2直後 2 552 194 341 71 723 1881 1303
2023/06/14 level 3直前 2 350 114 305 49 460 1278 -603
2023/07/05 level 3直後 3 557 149 336 63 594 1699 421
2023/07/07 level 4直前 3 500 136 324 51 567 1578 -121
2023/07/18 level 4直後 4 954 238 357 129 841 2519 941
2023/07/21 level 5直後 5 1441 283 428 177 1228 3557 1038
2023/09/14 定点 5 1118 0 392 136 1170 2816 -741

baselineに出力されているエラー数の累計は以下のshell scriptで取得しました。

cat phpstan-baseline.neon | yq '.parameters.ignoreErrors[].count' | awk '{s+=$1} END {print s}'

PHPStanを導入して運用するまでには「初期導入」「level 0をからlevel 5まで上げる」「level 5のエラーを潰す」の3ステップで進めたので、それぞれについて詳細を書いていきます。

余談ですがlevelの目標を5に置いた理由としては以下が挙げられます。 level 6に上げるには高いハードルがあり相応の覚悟が必要なので開発者がPHPStanに十分に慣れてからでないと無理だと考えています。

  • 既存のCRITICAL LOGを潰すのにはlevel 5があれば十分
  • PHP8に上げるためにはlevel 5があれば十分
  • level 6からは配列やCollectionにも型を付けなければならないのでハードルが高い
    • Rectorの SetList::TYPE_DECLARATION_STRICT を通してから対応したい

1. 初期導入

Larastanを composer install して ./vendor/bin/phpstan analyse --generate-baseline を実行したところクラッシュしました。 そもそもPHPとして壊れている記述がそこそこの量があり、とりあえず応急処置的に直せそうな部分は修正し、無理なものは excludePaths に追加するという対応をしました。

laravel-ide-helper でEloquent Modelに型アノテーションをつける php artisan ide-helper:models を実行する必要がありました。

そもそもopenlogi-apiは最初期はCakePHPで実装していたものをLaravelに移植したという歴史的経緯があり、CakePHP時代の遺産が数多く残されています。 移植はしたものの削除しきれていないコードをLaraStanでは解析しきれなかったという問題もありました。

PHPStanがどのようなものなのかドキュメントを纏め、全体に告知をした上で導入pull requestをmergeをしました。

2. level 0をからlevel 5まで上げる

以下の3ステップをひたすら繰り返してlevel 5まで気合で上げました。

  1. phpstanのlevelを上げる
  2. errorを色分けして各チーム用のbaselineファイルに振り分ける
  3. 機械的に修正できる部分のみを自分が修正対応する

いきなりphpstanのlevelを5にしてbaselineを引く方法でも良かったのですが、errorの量が膨大で仕分けるのが現実的ではありませんでした。 そこで 3. 機械的に修正できる部分のみを自分が修正対応する を挟むことによって雑多なエラーを減らし、本当に解決しなければならないエラーを浮き彫りにしました。

2. errorを色分けして各チーム用のbaselineファイルに振り分ける に関して、errorをグルーピング化して修正方法をコメントで残すような対応をしました。 この対応によりレビューする際に何をどういう変更をしたのか分かりやすくなり、PHPStanに習熟していないエンジニアでも理解しやすくなりました。

レビューをチームごとにしてもらいたいという理由からそれぞれのチームごとに phpstan-baseline.(wms|portal|billing|addon).neon を作成し、共通部分は phpstan-baseline.other.neon に吐き出しました。

    parameters:
        ignoreErrors:
            # 未定義変数エラー
            -
                message: "#^Variable \\$targetType might not be defined\\.$#"
                count: 3
                path: app/Http/Controllers/Wms/XXXController.php
    
            # class内の未定義変数関数へのアクセスができないエラー
            -
                message: "#^Access to an undefined property App\\\\Repositories\\\\Eloquent\\\\Wms\\\\XXXItem\\:\\:\\$image\\.$#"
                count: 1
                path: app/Repositories/Eloquent/Wms/XXXItem.php
    
            # 引数に過不足があるエラー
            -
                message: "#^Method Illuminate\\\\Database\\\\Eloquent\\\\Builder\\<App\\\\Repositories\\\\Eloquent\\\\Company\\>\\:\\:ofActive\\(\\) invoked with 1 parameter, 0 required\\.$#"
                count: 1
                path: app/Console/Commands/Admin/XXXStocks.php
    
            # 未定義関数呼び出しエラー
            -
                message: "#^Call to an undefined method App\\\\Services\\\\Wms\\\\XXXXService\\:\\:dispatch\\(\\)\\.$#"
                count: 2
                path: app/Services/Wms/XXXService.php

3. level 5のエラーを潰す

「level 0をからlevel 5まで上げる」で既存の挙動を考えずに機械的に修正できる箇所は一通り潰し終わっている状態まで持っていけました。 ここから先は各チームと連携して少しずつbaselineに記述されているエラーを潰していくという作業が必要です。

普段の業務負荷やPHPStanに対しての習熟度や業務ドメインに集中してほしいという願いを踏まえて各チームと調整した結果、「毎週1時間ずつ自分がライブコーディング(モブプロ)で修正しつつ問題箇所があったら都度議論する」という運用に落ち着きました。 8月中旬から運用されているのですが、細かいPHPの仕様やPHPStanのエラー修正テクニックなど文章では伝えきれないことをモブプロなら伝えられるので非常に有意義な時間を過せている感触を持っています。

LaraStanやLaravel自体をアップデートしないと解決しないものも多々ある(特にCollection周り)ので、ある程度の経験値が必要です。 修正が厳しそうなものはbaselineに残すのではなく、コードベースにコメントと共に @phpstan-ignore-line を書いて腰を据えて対応できるタイミングで直してもらうようにしています。

修正していて一番多いエラーは嘘のphpdocでした。 過去にどういうコーディングルールで進めていたのかは分かりませんが、半分以上は嘘のphpdocの修正でひたすら前後のコードを読んで適切な型に修正(あるいはphpdoc自体を削除する)対応をしていました。 PHPStan 1.10には嘘発見器が付属する - 超PHPerになろう にも書いてある通り、PHPStanのversionが上がればより厳密に検査することができるのでPHPやLaravelのversionを積極的に上げたい、その為にもPHPStanのエラーをひたすら潰していく必要があります。

モブプロで良かったこととしては、現在は仕様変更によって使わなくなったコードや削除漏れのコードを安全に削除することができたことです。 不要なコードを削除することによって認知コストを下げることができるので非常に良い取り組みでした。

エラーを潰す活動はまだ始まったばっかりなので引き続き取り組んでいきます。

Rector

RectorはAST(PHPStan)ベースの自動リファクタリングツールです。 類似ツールは正直私は知りませんのでどなたか教えていだたけると幸いです。

他社事例はPHPStanほど多くはありませんが幾つか上げておきます。

php-cs-fixerとRectorでコーディング規約をCIレベルで保証するという為に導入しています。 弊社では一部対応しきれていない部分を除いて、 app/ tests/ の全てのコードに以下のような rector.config.php を流しています。 JsonThrowOnErrorRector をskipしている理由は破壊的変更な為十分に検証する必要があるからです。

<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\Set\ValueObject\SetList;
use Rector\Set\ValueObject\LevelSetList;
use Rector\Php73\Rector\FuncCall\JsonThrowOnErrorRector;
use Rector\Caching\ValueObject\Storage\FileCacheStorage;

return static function (RectorConfig $config) {
    $config->paths([
        __DIR__ . '/path/to/dir',
    ]);

    $config->skip([
        JsonThrowOnErrorRector::class
    ]);

    // define sets of rules
    $config->sets([
        SetList::PHP_74,
        SetList::PSR_4,
        LevelSetList::UP_TO_PHP_74,
    ]);

    $config->importNames();
    $config->parallel(360);

    $config->cacheClass(FileCacheStorage::class);
    $config->cacheDirectory('./tmp/cache/rector');
};

Rectorを使ったことがある方ならご存知だと思いますがCPU/Memoryを食う非常に重いツールとなっています。 会社から支給されているPCのスペックが非常に高いのですが、CIで動かす時に実行時間が長いのは辛いものがあります。

RectorのRuleを適用していくにあたって、一気に全ファイルに流すのは危険なのでなるべく小さいスコープに区切ってpull requestを出す必要がありました。 そこで rector.(app|tests).(addon|billing|others|portal|wms) という組合せの10ファイルを作成し各チームにレビューをしてもらいながらRuleの反映を進めていきました。

別件で各チームのCODEOWNERを明確にするという対応が追い風になったのもあり、チームごとに並列して反映対応をすることができました。

今後の展望としては SetList::TYPE_DECLARATIONSetList::DeadCode を有効化し、よりPHP内に明示的な型や不要なコードを増やしていきたいと考えています。

Rectorで起った本番のバグ

ClosureToArrowFunctionRector を流していた時に起ってしまいました。

PHPでは無名関数アロー関数可変変数の挙動が違うことが原因で発生しました。

https://3v4l.org/UAdSF

$title = 'str';

$getView1 = function () use ($title, $viewPath) {
    return compact(['title']);
};
$getView2 = fn () => compact(['title']);

var_dump($getView1()); // ['title' => 'str']
var_dump($getView2()); // []

PHP RFC: Arrow Functions 2.0 的には一応仕様みたいです。

Finally, the automatic binding mechanism only considers variables that are used literally. That is, the following code will generate an undefined variable notice, because $x has no literal uses inside the function and thus hasn't been bound:

$x = 42;
$y = 'x';
$fn = fn() => $$y;

Support for this could be added by using a more general binding mechanism (bind everything rather than binding what is used) when variable variables are encountered. It’s excluded here because it seems like an entirely unnecessary complication of the implementation, but it can be supported if people consider it necessary.

Rectorは等価な式に変換するものだと盲信していましたが、コーナーケースではあるもののこういうバグを発生させてしまったのは反省しています。

可変変数と静的解析の相性は最悪なのでなるべく可変変数の削除を行っていきたいと考えています。

TypeScript

2023年現在、モダンフロントエンドといえばTypeScriptが一強なくらいTypeScriptが普及しています。 弊社ではフロントエンドをほぼ全てReactで構築しています。親和性を考えてもTypeScriptを導入しない理由がないと考えていました。 実際私が入社する前からTypeScriptを導入したいという話は度々出ていたようですが、チーム間の足並みを揃えたりするのが難しい、普段の業務が忙しいのもあって中々進んでいない状況でした。

そこで自分の方でWebpackの設定をして、React+TypeScriptの書き方等の方針について実際にフロントエンドを書いているエンジニアと話しました。 私は過去に新規のプロジェクトでReact+TypeScriptで開発したことはあったのですが、コードベースの大きいJavaScriptに途中からTypeScriptを導入するという経験がなかったので非常に苦労しました。

Webpackに詳しい方向けに一応書いておくと、よくあるts-loaderで導入するのは既存のコードベースでは厳しく@babel/preset-typescriptを適切に設定してなんとかで入れた、という感じです。 既存のbabel-preset(特に@babel/plugin-proposal-function-bind)が邪魔をしてうまくts-loaderを動かすことができなかったです。 tsconfig.jsonの調整なども自分が不慣れなのもあって良い落とし所を見つけるのに時間がかかりました。

方針決めでは、既存のReduxにどうやって型を付けるのか、Reactにどうやって型をつけていくのか、既存のコードベースはどうするのか、関数型コンポーネントへの移行はどうするべきか等について話しました。 やりながらにはなるものの、ある程度方針が決まったので新規のコードはTypeScriptで実装していくことになり、実際に実装が進んでいます。

また、 りあクト! TypeScriptで始めるつらくないReact開発 第4版【① 言語・環境編】の輪読回も同時に行なっており、社内全体でTypeScriptへの温度感が高まっています。 TypeScriptのコード量がもう少し増えたらどのように進めていったのかについて記事に纏めますのでご期待ください。

コード品質改善 UnitTest整備

PHPUnit自体を書くことは仕様と照らし合わせて逐次手動で書く必要があるが、UnitTestの整備はある程度機械的に行うことができます。

具体的には以下の対応をしました。

  • TestSuiteを実行時間が均一になるように分割した
  • Abstract TestCase.phpの余計な処理を削除
  • カバレッジを毎晩出力するGitHub Actions作成

TestSuiteで全体のテストを12分割をし、実行時間が均一になるように調整したところCIの実行時間が90〜120分かかってたものを15〜20分程度、QA用のDockerImage作成まで含めると20〜25分程度に短縮することができました。

johnkary/phpunit-speedtrap を使ってPHPUnitのテスト毎の実行時間を計測し、手動で時間が均されるように調整するという作業をしました。 今回の対応はこれで良いのですが、職人芸的な作業をしていまったので将来的にメンテナンスをするのが難しい状態です。 今後の課題としては自動的にテストの実行時間を均すようなTestSuiteを生成するscriptを作成したいと思います。

現状ほとんと全てのテストケースは共通のabstract Testcase.phpを参照してテストが書かれています。 過去の場当たり的な変更が残ったままだったので認知コストを下げる為にも不要なコードの削除をしました。

毎晩カバレッジをGitHub Artifactに出力するGitHub Actionsを作成しました。 残念ながらカバレッジの結果を元に改善活動をできてないですが、以前はカバレッジを出力すらできてなかったので大きな成果だと自負しております。

今後はPHPUnitのテストの書き方についての全体的な方針について纏めてひたすら書いていく作業をしていきたいです。

コード品質改善 苦労した点について

兎に角コードベースが大きいというところに尽きます。 PHPStan一つ動かすのにも手元で非常に時間がかかるし、PHPUnitのCIの結果を待つのにも非常に時間がかかりました。

各ツールの習熟度は既にそれなりに高かったので既存のコードベースに関係のない技術的な面で詰まることは一切ありませんでした。 なので、「如何に依存関係のないpull requestを並列して作成し、短時間で多くのpull requestをmergeするか」ということが自分の課題でした。 過去に少人数の開発経験しかなかったので序盤は各チームとの調整などに四苦八苦しましたが、うまく落とし所を見つけてからはほぼ全て機械的な作業になり、かなりスピード感をもって修正しました。正直拙速だった部分も多々ありこれで本当によかったのか、かなり反省の余地があります。

PHPStanやRectorなどの静的解析ツールで恩恵を受けられるタイミングは、きちんとCIが整備され、大半のエラーが潰されていて、エンジニア全員が静的解析ツールの良さを理解してからだと思っています。 序盤は特にですが、今まで書けていた書き方が出来なくなってしまうだけの口うるさいツールとして認識されてしまってもおかしくはありません。 原因がよく分からないエラーで時間を浪費する場面も多々あり、ignoreするべきか否かという判断が難しかったです。

一通りの導入やオンボーディングなどは出来たものの、正直現状では「組織レベルで静的解析ツールが運用されてる」という状態まではまだまだ発展途上です。まだまだ時間がかかると思うので引き続き啓蒙活動を続けていこうと思います。

ネガティブなことばかり書きましたが、Linterや静的解析ツールやPHPUnitを標準的な設定かつ常識的な範囲内の時間でCI実行し続けられているところまで持っていけたのはそれなりに意味があったのではないでしょうか。

コード品質改善 今後の展望

  • PHPのVersionを7.4から8.xに上げる
  • mt_randやcompactのあまり推奨されていない関数を潰す
  • PHPStanのLevelを5以上にする
  • Rector RuleをよりStrictなものにする
  • DeadCodeを削除していく
  • PHPUnit周りの整備やテストを増やしていく

この活動自体が良かったのか悪かったのか、本当に効果があったのか、というのは今後行う施策が安全かつスムーズに行えるかどうかによってしか評価できないと考えています。

あまりゴールを決めずに進めてしまったというのもあり、どの時点でどこを着地点にするのか、カバレッジをどう増やしていくのか、PHPStanのlevelをどこまで上げるのかというのを今後開発チーム全体で決めていかなければなりません。

終わりに

コード品質改善やその基盤作りは地道なもので大半は定型的な作業なので奇を衒う必要はありません。 愚直にpull requestを出し続ければいつかは終わりがある作業であり、一度CIをきちんと整備すればこれ以上悪くなることはありません。 物流業界には解決しなければいけない問題が溢れているので、本質的ではない問題に足を引っぱられないように引き続き改善活動を頑張っていこうと思います。

弊社はPHPエンジニアを積極的に採用していますので、興味のある方は是非是非気軽にカジュアル面談を受けてください。 https://corp.openlogi.com/recruit/

GitHubで編集を提案
OPENLOGI Tech Blog

Discussion