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
1
1.8k
レガシーシステムへのPHPStan導入から半年での課題と効果
PHPerKaigi 2024 で発表した内容になります。
don
February 09, 2024
Tweet
Share
More Decks by don
See All by don
レガシーシステムへのPHPStan導入から半年での効果と課題
bosshawk
0
2k
息の長いサービスの PHP8バージョンアップで見えた 課題と解決法 / Problems and solutions found when upgrading long-term services to PHP8
bosshawk
0
2.7k
レガシーシステムにおけるPHP8バージョンアップのアプリ対応記録
bosshawk
0
2.1k
[おすすめの技術書 LT会 - vol.2] 体系的に学ぶ安全なWebアプリケーションの作り方
bosshawk
0
20k
[PHPerKaigi 2021 (LT)] 新社会人のコード品質カイゼン記録
bosshawk
1
1.4k
Other Decks in Programming
See All in Programming
ソフトウェア設計の実践的な考え方
masuda220
PRO
4
550
どの様にAIエージェントと 協業すべきだったのか?
takefumiyoshii
2
640
大規模アプリのDIフレームワーク刷新戦略 ~過去最大規模の並行開発を止めずにアプリ全体に導入するまで~
mot_techtalk
1
430
dynamic!
moro
10
7.3k
詳しくない分野でのVibe Codingで困ったことと学び/vibe-coding-in-unfamiliar-area
shibayu36
3
4.9k
いま中途半端なSwift 6対応をするより、Default ActorやApproachable Concurrencyを有効にしてからでいいんじゃない?
yimajo
2
400
Software Architecture
hschwentner
6
2.3k
Catch Up: Go Style Guide Update
andpad
0
210
私達はmodernize packageに夢を見るか feat. go/analysis, go/ast / Go Conference 2025
kaorumuta
2
530
Le côté obscur des IA génératives
pascallemerrer
0
140
2分台で1500examples完走!爆速CIを支える環境構築術 - Kaigi on Rails 2025
falcon8823
3
3.5k
AI Coding Meetup #3 - 導入セッション / ai-coding-meetup-3
izumin5210
0
3.3k
Featured
See All Featured
Keith and Marios Guide to Fast Websites
keithpitt
411
23k
CSS Pre-Processors: Stylus, Less & Sass
bermonpainter
358
30k
RailsConf & Balkan Ruby 2019: The Past, Present, and Future of Rails at GitHub
eileencodes
140
34k
Principles of Awesome APIs and How to Build Them.
keavy
127
17k
Speed Design
sergeychernyshev
32
1.2k
StorybookのUI Testing Handbookを読んだ
zakiyama
31
6.2k
Understanding Cognitive Biases in Performance Measurement
bluesmoon
29
2.7k
Performance Is Good for Brains [We Love Speed 2024]
tammyeverts
12
1.2k
Let's Do A Bunch of Simple Stuff to Make Websites Faster
chriscoyier
507
140k
Easily Structure & Communicate Ideas using Wireframe
afnizarnur
194
16k
Building a Scalable Design System with Sketch
lauravandoore
462
33k
Into the Great Unknown - MozCon
thekraken
40
2.1k
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