レガシーシステムへのPHPStan導入から半年での課題と効果

PHPerKaigi 2024 で発表した内容になります。

Transcript

1. #phperkaigi #c レガシーシステムへの PHPStan 導入から 半年での課題と効果 2024 年03 月08 日 PHPerKaigi 2024 don （頓花） 　　©RAKUS Co., Ltd.
2. don （頓花） 出身：大阪府 年齢：29 歳 趣味：ゲーム, アニメ, マンガ, デバイス収集 etc... 略歴 2019 年 株式会社ラクス入社 技術スタック PHP 歴： 5 年 #phperkaigi #c 自己紹介 　　©RAKUS Co., Ltd. 2
3. 楽楽販売 クラウド型の販売管理システム Web アプリケーション バックエンドはPHP ／PostgreSQL 独自フレームワーク PHP5 の時代から 15 年以上 続くソースコード 約35 万行（PHP コードのみで） #phperkaigi #c プロダクトの説明 　　©RAKUS Co., Ltd. 3
4. なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ 　　©RAKUS Co., Ltd. 4
5. ところで、みなさん #phperkaigi #c 　　©RAKUS Co., Ltd. 5
6. 静的解析ツールは何を使っていますか？ #phperkaigi #c 　　©RAKUS Co., Ltd. 6
7. 導入していた静的解析ツール PhpStorm のInspection IDE 上で解析を実行 SonarQube 重複コードのチェックなど CI で解析を実行 SonarQube とは 多くのプログラミング言語をサポートしている静的解析ツール ※ https://docs.sonarsource.com/sonarqube/latest/ #phperkaigi #c 静的解析ツール 　　©RAKUS Co., Ltd. 7
8. しかし、 それらの静的解析ツールでは こんな課題が発生していました... 。 #phperkaigi #c 　　©RAKUS Co., Ltd. 8
9. PhpStorm のInspection の課題 既存コードと新規コードの指摘が判別つかない CI を設定するのが難しい ⇒ 解析は強力であるが、修正可否をチェックしづらい SonarQube の課題 型のチェックなどの解析がない カスタムルールの追加が難しい ⇒ 言語に特化した静的解析が実施しづらい #phperkaigi #c 既存の静的解析ツールでの課題 　　©RAKUS Co., Ltd. 9
10. また、 静的解析ツール以外の課題も... #phperkaigi #c 　　©RAKUS Co., Ltd. 10
11. null に関連する不具合が頻発 null オブジェクトへの参照エラー 関数の nullable でない型の引数に null を渡す コードレビューの時間増加による開発効率の低下 新任メンバー増加などが要因 #phperkaigi #c 開発チームの課題 　　©RAKUS Co., Ltd. 11
12. 少し本筋から外れますが #phperkaigi #c 　　©RAKUS Co., Ltd. 12
13. PHP の最近の動向について #phperkaigi #c 　　©RAKUS Co., Ltd. 13
14. 新機能 型付きプロパティの導入 列挙型／交差型／never 型／true ・false ・null 型の追加 readonly アクセス修飾子／readonly クラスの導入 非推奨 PHP 8.0 ：非厳密な比較演算子の挙動変更 PHP 8.1 ：ビルトイン関数の nullable でない引数に null を渡す PHP 8.2 ：動的プロパティが非推奨 #phperkaigi #c 型定義の厳密化するPHP 　　©RAKUS Co., Ltd. 14
15. 静的型付け言語に近づいていくPHP #phperkaigi #c 　　©RAKUS Co., Ltd. 15
16. 閑　話 ／ 休　題 Kan 　　　　 Wa 　　　　　　　Kyu 　　　　Dai
17. 課題 null に関連する不具合が頻発 コードレビューの時間増加による開発効率の低下 PhpStorm のInspection はCI 化されていない SonarQube はPHP のコード解析力が足りない ⇒ 既存の静的解析ツールでは力不足... 開発チームの課題
18. 他に良い解析ツールはないか... ？
20. PHPStan PHP コードを静的に解析する 実行時にエラーとなるような問題のあるコードを検知する https://phpstan.org/ PHPStan のメリット 型定義のチェック 独自ルールの追加の容易さ #phperkaigi #c PHPStan とは 　　©RAKUS Co., Ltd. 20
21. なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ 　　©RAKUS Co., Ltd. 21
22. PHPStan を導入する上では 解析レベルをどうするか 既存エラーへの対処 実行方法 メイン担当者を決める #phperkaigi #c PHPStan 導入 　　©RAKUS Co., Ltd. 22
23. #phperkaigi #c PHPStan 導入：解析レベル Level 概要 0 基本的なチェック、未知のクラス、未知の関数、 $this 上で呼び出された未知のメソッド、それらのメソッドや関数に渡された引数の 数が間違っている、常に未定義の変数をチェック 1 未定義の変数、 __call と __get を持つクラスの未知のマジックメソッドとプロパティがある可能性がある 2 ( $this だけでなく) すべての式で未知のメソッドをチェックし、PHPDocs を検証する 3 戻り値の型、プロパティに割り当てられた型の確認 4 基本的なデッドコードチェック( instanceof やその他の型チェックが常に false / 到達しない else 文/ return 後の到達不能コード) 5 メソッドや関数に渡される引数の型チェック 6 タイプヒントの欠落を報告する 7 部分的に間違っている論理和型の報告。論理和型の一部の型にしか存在しないメソッドを呼び出した場合、レベル7 はそのことを報告し 始めます( その他の不正確な状況も) 8 nullable 型に対するメソッド呼び出しとプロパティへのアクセスを報告する 9|max 混合型に厳密であること。この型で唯一許される操作は、この型を別の混合型に渡すことである 　　©RAKUS Co., Ltd. 23
24. 検討 既存のレガシーコードに高い解析レベルが修正コストが高い 直近でもnull に関連する不具合が頻発していたこともありnull チェックはしたい 対応 解析レベルは8 既存コードは検知対象外とする baseline を作成して対応 #phperkaigi #c PHPStan 導入：解析レベルと既存コード 　　©RAKUS Co., Ltd. 24
25. 既存の解析ツールの課題 PhpStorm のInspection はローカル実行のみ SonarQube の実行も手動でブランチを指定が必要 対応 PR 単位で自動チェックされるようにCI を設定 #phperkaigi #c PHPStan 導入：実行方法 　　©RAKUS Co., Ltd. 25
26. メイン担当者を決めてPHPStan の相談を集約する 役割 課題の対応方針の検討 対応方法の相談 #phperkaigi #c PHPStan 導入：メイン担当者 　　©RAKUS Co., Ltd. 26
27. ところで、気になりませんか？ #phperkaigi #c 　　©RAKUS Co., Ltd. 27
28. レガシーコードに対して 解析レベルをほぼMAX( レベル8) で 実行するとどれぐらいエラーが出るのか #phperkaigi #c 　　©RAKUS Co., Ltd. 28
29. 解析レベル8 で解析実行だ！ #phperkaigi #c 　　©RAKUS Co., Ltd. 29
30. 約60,000 件のエラーを検知！！ #phperkaigi #c ※対象 約5,000 ファイル（約35 万行） 　　©RAKUS Co., Ltd. 30
31. ですよね... #phperkaigi #c 　　©RAKUS Co., Ltd. 31
32. 解析レベルはレベル８ 既存コードはベースラインを設定し指摘対象外へ PR 単位で自動でチェックされるようにCI を設定 PHPStan の相談を集約するためにメイン担当者を決定 #phperkaigi #c PHPStan 導入 　　©RAKUS Co., Ltd. 32
33. なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ 　　©RAKUS Co., Ltd. 33
34. 効果 PHPStan でバグを発見 カスタムルールを追加 コードレビュー対応時間の軽減 etc... #phperkaigi #c 効果①：具体的な効果 　　©RAKUS Co., Ltd. 34
35. 例）PHPStan でバグを発見 関数のオーバーライドの引数の型の不具合の発見 HogeMenuController.php:18:Method HogeMenuController::viewAction() overrides method AbstractMenuController::viewAction() but misses parameter #1 $form. 大規模リファクタリングを実施していたため、コードレビューから漏れていた。 PHPStan を導入していなければ、そのままリリースされていた可能性も... #phperkaigi #c 効果①：具体的な効果 　　©RAKUS Co., Ltd. 35
36. 効果 メンバーのセルフチェックの品質向上 各メンバーの型への意識が向上 #phperkaigi #c 効果②：メンバーの意識向上 　　©RAKUS Co., Ltd. 36
37. なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ 　　©RAKUS Co., Ltd. 37
38. 課題 指摘の意味を調べるのに時間がかかる 1PR のコード量が多く、指摘が多くなる 対応 メンバーが指摘なれることで解決 PR の粒度を小さくする #phperkaigi #c 課題と対応①：指摘の修正に時間がかかる 　　©RAKUS Co., Ltd. 38
39. 例） $ ./vendor/bin/phpstan analyse --no-progress --memory-limit=2G --error-format=raw $(cat $CI_PROJECT_DIR/target.txt) Note: Using configuration file ./phpstan.neon. ... HogeCommand.php:25:Property HogeCommand::$requestParameter has no type specified. HogeCommand.php:704:Method HogeCommand::mailSend() has no return type specified. HogeCommand:704:Method HogeCommand::mailSend() has parameter $condition with no type specified. HogeCommand:704:Method AHogeCommand::mailSend() has parameter $aIsError with no type specified. HogeCommand:749:Offset 'user_name' might not exist on array|null. FileInfo.php:23:Class FileInfo has an uninitialized readonly property $fileId. Assign it in the constructor. FileInfo.php:24:Class FileInfo has an uninitialized readonly property $fileName. Assign it in the constructor. FileInfo.php:25:Class FileInfo has an uninitialized readonly property $filePath. Assign it in the constructor. FileInfo.php:25:Property FileInfo::$filePath is never read, only written. FileInfo.php:26:Class FileInfo has an uninitialized readonly property $fileOrder. Assign it in the constructor. FileInfo.php:26:Property FileInfo::$fileOrder is never read, only written. FileInfo.php:27:Class FileInfo has an uninitialized readonly property $fileType. Assign it in the constructor. ... #phperkaigi #c 課題と対応①：指摘の修正に時間がかかる 　　©RAKUS Co., Ltd. 39
40. 課題 array を引数とする場合に正確な型指定が必要 match 文のdefaut 指定 標準関数が複数の型を返す問題 etc... 対応 除外ルールを設定して検査対象外 #phperkaigi #c 課題と対応②：過剰な指摘が多数発生 　　©RAKUS Co., Ltd. 40
41. 例）array を引数とする場合に正確な型指定 DB へのアクセスした値を配列で持ち回っているために指摘される 指摘に対応すると返値の配列を型も含めてすべて指定する必要がある → 除外ルールへ設定 既存処理の呼び出しで問題になるためすべて対応することは難しいと判断 /** * @return array{"foo": int, "bar": string} */ function fetchHoge(int $id): array { return $this->pdo->query("SELECT foo,bar FROM hoge WHERE id = $id")->fetch(); } #phperkaigi #c 課題と対応②：過剰な指摘が多数発生 　　©RAKUS Co., Ltd. 41
42. 課題 型定義がない PHPDoc の型定義が間違っている 対応 ボーイスカウトルールを設定 型定義を追加 PHPDoc で型定義を追加 ※ 基本は動作に影響のないようにPHPDoc の追加で対応 #phperkaigi #c 課題と対応③：既存コードの型定義不足 　　©RAKUS Co., Ltd. 42
43. 順調に課題を解消していくが、 ここで問題発生... #phperkaigi #c 　　©RAKUS Co., Ltd. 43
44. メイン担当者が不在へ... ( メンバーの異動などが重なり...) #phperkaigi #c 　　©RAKUS Co., Ltd. 44
45. 課題 どの粒度で修正するのかがチーム全体で決まらない どの指摘を修正するか ルールが決まり切らずに対応がバラバラへ... 暫定対応 どこまで修正するかはレビュアーとの認識合わせで合意 というルール 一部の指摘はbaseline に追加し指摘対象外へ #phperkaigi #c 課題と対応④：メイン担当者が不在による課題 　　©RAKUS Co., Ltd. 45
46. 残課題 ローカル実行する手段が少ないため再チェックに時間が掛かる 対応案 ローカルで簡易に実行できる手段の確立 CI の実行時間問題の解消 #phperkaigi #c 今後 　　©RAKUS Co., Ltd. 46
47. なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ 　　©RAKUS Co., Ltd. 47
48. 得られたこと 型への意識が向上しクリーンなコードが作られるようになった レガシーなコードであってもベースラインや除外ルールを設定することで、解析 レベルが高い状態で運用することができた 注意点 自動でチェックされる仕組みにすること メイン担当者を決めてルールを整備すること #phperkaigi #c まとめ 　　©RAKUS Co., Ltd. 48
49. ご清聴ありがとうございました。 #phperkaigi #c 　　©RAKUS Co., Ltd. 49