Upgrade to Pro
— share decks privately, control downloads, hide ads and more …
Speaker Deck
Features
Speaker Deck
PRO
Sign in
Sign up for free
Search
Search
レガシーシステムへのPHPStan導入から半年での課題と効果
Search
don
February 09, 2024
Programming
0
1.2k
レガシーシステムへのPHPStan導入から半年での課題と効果
PHPerKaigi 2024 で発表した内容になります。
don
February 09, 2024
Tweet
Share
More Decks by don
See All by don
レガシーシステムへのPHPStan導入から半年での効果と課題
bosshawk
0
1.4k
息の長いサービスの PHP8バージョンアップで見えた 課題と解決法 / Problems and solutions found when upgrading long-term services to PHP8
bosshawk
0
2.3k
レガシーシステムにおけるPHP8バージョンアップのアプリ対応記録
bosshawk
0
1.8k
[おすすめの技術書 LT会 - vol.2] 体系的に学ぶ安全なWebアプリケーションの作り方
bosshawk
0
20k
[PHPerKaigi 2021 (LT)] 新社会人のコード品質カイゼン記録
bosshawk
1
1.1k
Other Decks in Programming
See All in Programming
App Router 悲喜交々
quramy
7
380
Remix × Cloudflare Pages × Sentry 奮闘記 / remix-pages-sentry
nkzn
1
420
PHPを書く理由、PHPを書いていて良い理由 / Reasons to write PHP and why it is good to write PHP
seike460
PRO
5
460
型付きで行うVSCode拡張機能開発 / VSCode Meetup #31
mazrean
0
240
How to debug Xdebug... or any other weird bug in PHP
dunglas
1
870
Modern Functional Fluent CFML REST by Luis Majano
ortus24
0
140
VS Code extension: ドラッグ&ドロップでファイルを並び替える
ttrace
0
170
sqlcを利用してsqlに型付けを
kamiyam
0
240
実践Dash - 手を抜きながら本気で作るデータApplicationの基本と応用 / Dash for Python and Baseball
shinyorke
2
330
データマイグレーションの成功戦略~サービスリニューアルで失敗しないための実践ガイド~
tkzwtks
6
630
Micro Frontends for Java Microservices - dev2next 2024
mraible
PRO
0
210
ポケモンで考えるコミュニケーション / Communication Lessons from Pokémon
mackey0225
4
170
Featured
See All Featured
Build your cross-platform service in a week with App Engine
jlugia
229
18k
Refactoring Trust on Your Teams (GOTO; Chicago 2020)
rmw
31
2.6k
RailsConf 2023
tenderlove
28
840
jQuery: Nuts, Bolts and Bling
dougneiner
61
7.5k
Building Flexible Design Systems
yeseniaperezcruz
327
38k
Practical Orchestrator
shlominoach
186
10k
Why Our Code Smells
bkeepers
PRO
334
57k
The Success of Rails: Ensuring Growth for the Next 100 Years
eileencodes
43
6.5k
The Art of Delivering Value - GDevCon NA Keynote
reverentgeek
4
120
Web development in the modern age
philhawksworth
205
10k
GraphQLとの向き合い方2022年版
quramy
43
13k
The Invisible Customer
myddelton
119
13k
Transcript
#phperkaigi #c レガシーシステムへの PHPStan 導入から 半年での課題と効果 2024 年03 月08 日
PHPerKaigi 2024 don (頓花) ©RAKUS Co., Ltd.
don (頓花) 出身:大阪府 年齢:29 歳 趣味:ゲーム, アニメ, マンガ, デバイス収集 etc...
略歴 2019 年 株式会社ラクス入社 技術スタック PHP 歴: 5 年 #phperkaigi #c 自己紹介 ©RAKUS Co., Ltd. 2
楽楽販売 クラウド型の販売管理システム Web アプリケーション バックエンドはPHP /PostgreSQL 独自フレームワーク PHP5 の時代から 15
年以上 続くソースコード 約35 万行(PHP コードのみで) #phperkaigi #c プロダクトの説明 ©RAKUS Co., Ltd. 3
なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ ©RAKUS
Co., Ltd. 4
ところで、みなさん #phperkaigi #c ©RAKUS Co., Ltd. 5
静的解析ツールは何を使っていますか? #phperkaigi #c ©RAKUS Co., Ltd. 6
導入していた静的解析ツール PhpStorm のInspection IDE 上で解析を実行 SonarQube 重複コードのチェックなど CI で解析を実行 SonarQube
とは 多くのプログラミング言語をサポートしている静的解析ツール ※ https://docs.sonarsource.com/sonarqube/latest/ #phperkaigi #c 静的解析ツール ©RAKUS Co., Ltd. 7
しかし、 それらの静的解析ツールでは こんな課題が発生していました... 。 #phperkaigi #c ©RAKUS Co., Ltd. 8
PhpStorm のInspection の課題 既存コードと新規コードの指摘が判別つかない CI を設定するのが難しい ⇒ 解析は強力であるが、修正可否をチェックしづらい SonarQube の課題
型のチェックなどの解析がない カスタムルールの追加が難しい ⇒ 言語に特化した静的解析が実施しづらい #phperkaigi #c 既存の静的解析ツールでの課題 ©RAKUS Co., Ltd. 9
また、 静的解析ツール以外の課題も... #phperkaigi #c ©RAKUS Co., Ltd. 10
null に関連する不具合が頻発 null オブジェクトへの参照エラー 関数の nullable でない型の引数に null を渡す コードレビューの時間増加による開発効率の低下
新任メンバー増加などが要因 #phperkaigi #c 開発チームの課題 ©RAKUS Co., Ltd. 11
少し本筋から外れますが #phperkaigi #c ©RAKUS Co., Ltd. 12
PHP の最近の動向について #phperkaigi #c ©RAKUS Co., Ltd. 13
新機能 型付きプロパティの導入 列挙型/交差型/never 型/true ・false ・null 型の追加 readonly アクセス修飾子/readonly クラスの導入
非推奨 PHP 8.0 :非厳密な比較演算子の挙動変更 PHP 8.1 :ビルトイン関数の nullable でない引数に null を渡す PHP 8.2 :動的プロパティが非推奨 #phperkaigi #c 型定義の厳密化するPHP ©RAKUS Co., Ltd. 14
静的型付け言語に近づいていくPHP #phperkaigi #c ©RAKUS Co., Ltd. 15
閑 話 / 休 題 Kan Wa Kyu Dai
課題 null に関連する不具合が頻発 コードレビューの時間増加による開発効率の低下 PhpStorm のInspection はCI 化されていない SonarQube はPHP
のコード解析力が足りない ⇒ 既存の静的解析ツールでは力不足... 開発チームの課題
他に良い解析ツールはないか... ?
None
PHPStan PHP コードを静的に解析する 実行時にエラーとなるような問題のあるコードを検知する https://phpstan.org/ PHPStan のメリット 型定義のチェック 独自ルールの追加の容易さ #phperkaigi
#c PHPStan とは ©RAKUS Co., Ltd. 20
なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ ©RAKUS
Co., Ltd. 21
PHPStan を導入する上では 解析レベルをどうするか 既存エラーへの対処 実行方法 メイン担当者を決める #phperkaigi #c PHPStan 導入
©RAKUS Co., Ltd. 22
#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
検討 既存のレガシーコードに高い解析レベルが修正コストが高い 直近でもnull に関連する不具合が頻発していたこともありnull チェックはしたい 対応 解析レベルは8 既存コードは検知対象外とする baseline を作成して対応
#phperkaigi #c PHPStan 導入:解析レベルと既存コード ©RAKUS Co., Ltd. 24
既存の解析ツールの課題 PhpStorm のInspection はローカル実行のみ SonarQube の実行も手動でブランチを指定が必要 対応 PR 単位で自動チェックされるようにCI を設定
#phperkaigi #c PHPStan 導入:実行方法 ©RAKUS Co., Ltd. 25
メイン担当者を決めてPHPStan の相談を集約する 役割 課題の対応方針の検討 対応方法の相談 #phperkaigi #c PHPStan 導入:メイン担当者 ©RAKUS
Co., Ltd. 26
ところで、気になりませんか? #phperkaigi #c ©RAKUS Co., Ltd. 27
レガシーコードに対して 解析レベルをほぼMAX( レベル8) で 実行するとどれぐらいエラーが出るのか #phperkaigi #c ©RAKUS Co., Ltd.
28
解析レベル8 で解析実行だ! #phperkaigi #c ©RAKUS Co., Ltd. 29
約60,000 件のエラーを検知!! #phperkaigi #c ※対象 約5,000 ファイル(約35 万行) ©RAKUS Co.,
Ltd. 30
ですよね... #phperkaigi #c ©RAKUS Co., Ltd. 31
解析レベルはレベル8 既存コードはベースラインを設定し指摘対象外へ PR 単位で自動でチェックされるようにCI を設定 PHPStan の相談を集約するためにメイン担当者を決定 #phperkaigi #c PHPStan
導入 ©RAKUS Co., Ltd. 32
なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ ©RAKUS
Co., Ltd. 33
効果 PHPStan でバグを発見 カスタムルールを追加 コードレビュー対応時間の軽減 etc... #phperkaigi #c 効果①:具体的な効果 ©RAKUS
Co., Ltd. 34
例)PHPStan でバグを発見 関数のオーバーライドの引数の型の不具合の発見 HogeMenuController.php:18:Method HogeMenuController::viewAction() overrides method AbstractMenuController::viewAction() but misses
parameter #1 $form. 大規模リファクタリングを実施していたため、コードレビューから漏れていた。 PHPStan を導入していなければ、そのままリリースされていた可能性も... #phperkaigi #c 効果①:具体的な効果 ©RAKUS Co., Ltd. 35
効果 メンバーのセルフチェックの品質向上 各メンバーの型への意識が向上 #phperkaigi #c 効果②:メンバーの意識向上 ©RAKUS Co., Ltd. 36
なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ ©RAKUS
Co., Ltd. 37
課題 指摘の意味を調べるのに時間がかかる 1PR のコード量が多く、指摘が多くなる 対応 メンバーが指摘なれることで解決 PR の粒度を小さくする #phperkaigi #c
課題と対応①:指摘の修正に時間がかかる ©RAKUS Co., Ltd. 38
例) $ ./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
課題 array を引数とする場合に正確な型指定が必要 match 文のdefaut 指定 標準関数が複数の型を返す問題 etc... 対応 除外ルールを設定して検査対象外
#phperkaigi #c 課題と対応②:過剰な指摘が多数発生 ©RAKUS Co., Ltd. 40
例)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
課題 型定義がない PHPDoc の型定義が間違っている 対応 ボーイスカウトルールを設定 型定義を追加 PHPDoc で型定義を追加 ※
基本は動作に影響のないようにPHPDoc の追加で対応 #phperkaigi #c 課題と対応③:既存コードの型定義不足 ©RAKUS Co., Ltd. 42
順調に課題を解消していくが、 ここで問題発生... #phperkaigi #c ©RAKUS Co., Ltd. 43
メイン担当者が不在へ... ( メンバーの異動などが重なり...) #phperkaigi #c ©RAKUS Co., Ltd. 44
課題 どの粒度で修正するのかがチーム全体で決まらない どの指摘を修正するか ルールが決まり切らずに対応がバラバラへ... 暫定対応 どこまで修正するかはレビュアーとの認識合わせで合意 というルール 一部の指摘はbaseline に追加し指摘対象外へ #phperkaigi
#c 課題と対応④:メイン担当者が不在による課題 ©RAKUS Co., Ltd. 45
残課題 ローカル実行する手段が少ないため再チェックに時間が掛かる 対応案 ローカルで簡易に実行できる手段の確立 CI の実行時間問題の解消 #phperkaigi #c 今後 ©RAKUS
Co., Ltd. 46
なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ ©RAKUS
Co., Ltd. 47
得られたこと 型への意識が向上しクリーンなコードが作られるようになった レガシーなコードであってもベースラインや除外ルールを設定することで、解析 レベルが高い状態で運用することができた 注意点 自動でチェックされる仕組みにすること メイン担当者を決めてルールを整備すること #phperkaigi #c まとめ
©RAKUS Co., Ltd. 48
ご清聴ありがとうございました。 #phperkaigi #c ©RAKUS Co., Ltd. 49