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
Shinjuku.rb#95:心の技術書紹介
free_world21
1
110
Android開発以外のAndroid開発経験の活かしどころ
konifar
2
1k
AndroidアプリのUIバリエーションをあの手この手で確認する / Check UI variations of Android apps by various means
tkmnzm
1
180
オートマトン学習しろ / Do automata learning
makenowjust
3
130
o1モデルのプロンプトエンジニアリングって?
ktc_wada
0
270
Scala アプリケーションのビルドを改善してデプロイ時間を 1/4 にした話 | How I improved the build of my Scala application and reduced deployment time by 4x
nomadblacky
1
180
GenU導入でCDKに初挑戦し、悪戦苦闘した話
hideg
0
170
Understand the mechanism! Let's do screenshots tests of Compose Previews with various variations / 仕組みから理解する!Composeプレビューを様々なバリエーションでスクリーンショットテストしよう
sumio
3
800
いつか使える ObjectSpace / Maybe useful ObjectSpace
euglena1215
2
140
Lessons by WebAssembly app in production on CDN Edge Computing Service
tetsuharuohzeki
0
210
Jakarta EE meets AI
ivargrimstad
1
520
マルチモジュールにおけるテスト最適化
fxwx23
0
210
Featured
See All Featured
Principles of Awesome APIs and How to Build Them.
keavy
125
16k
GraphQLの誤解/rethinking-graphql
sonatard
65
9.8k
A Philosophy of Restraint
colly
202
16k
BBQ
matthewcrist
83
9.2k
The Psychology of Web Performance [Beyond Tellerrand 2023]
tammyeverts
36
2.1k
What the flash - Photography Introduction
edds
67
11k
Designing for humans not robots
tammielis
248
25k
GitHub's CSS Performance
jonrohan
1030
450k
Product Roadmaps are Hard
iamctodd
PRO
48
10k
The Art of Delivering Value - GDevCon NA Keynote
reverentgeek
1
54
Fashionably flexible responsive web design (full day workshop)
malarkey
401
65k
How to Ace a Technical Interview
jacobian
274
23k
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