Slide 1

Slide 1 text

#phperkaigi #c レガシーシステムへの PHPStan 導入から 半年での課題と効果 2024 年03 月08 日 PHPerKaigi 2024 don (頓花)   ©RAKUS Co., Ltd.

Slide 2

Slide 2 text

don (頓花) 出身:大阪府 年齢:29 歳 趣味:ゲーム, アニメ, マンガ, デバイス収集 etc... 略歴 2019 年 株式会社ラクス入社 技術スタック PHP 歴: 5 年 #phperkaigi #c 自己紹介   ©RAKUS Co., Ltd. 2

Slide 3

Slide 3 text

楽楽販売 クラウド型の販売管理システム Web アプリケーション バックエンドはPHP /PostgreSQL 独自フレームワーク PHP5 の時代から 15 年以上 続くソースコード 約35 万行(PHP コードのみで) #phperkaigi #c プロダクトの説明   ©RAKUS Co., Ltd. 3

Slide 4

Slide 4 text

なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ   ©RAKUS Co., Ltd. 4

Slide 5

Slide 5 text

ところで、みなさん #phperkaigi #c   ©RAKUS Co., Ltd. 5

Slide 6

Slide 6 text

静的解析ツールは何を使っていますか? #phperkaigi #c   ©RAKUS Co., Ltd. 6

Slide 7

Slide 7 text

導入していた静的解析ツール PhpStorm のInspection IDE 上で解析を実行 SonarQube 重複コードのチェックなど CI で解析を実行 SonarQube とは 多くのプログラミング言語をサポートしている静的解析ツール ※ https://docs.sonarsource.com/sonarqube/latest/ #phperkaigi #c 静的解析ツール   ©RAKUS Co., Ltd. 7

Slide 8

Slide 8 text

しかし、 それらの静的解析ツールでは こんな課題が発生していました... 。 #phperkaigi #c   ©RAKUS Co., Ltd. 8

Slide 9

Slide 9 text

PhpStorm のInspection の課題 既存コードと新規コードの指摘が判別つかない CI を設定するのが難しい ⇒ 解析は強力であるが、修正可否をチェックしづらい SonarQube の課題 型のチェックなどの解析がない カスタムルールの追加が難しい ⇒ 言語に特化した静的解析が実施しづらい #phperkaigi #c 既存の静的解析ツールでの課題   ©RAKUS Co., Ltd. 9

Slide 10

Slide 10 text

また、 静的解析ツール以外の課題も... #phperkaigi #c   ©RAKUS Co., Ltd. 10

Slide 11

Slide 11 text

null に関連する不具合が頻発 null オブジェクトへの参照エラー 関数の nullable でない型の引数に null を渡す コードレビューの時間増加による開発効率の低下 新任メンバー増加などが要因 #phperkaigi #c 開発チームの課題   ©RAKUS Co., Ltd. 11

Slide 12

Slide 12 text

少し本筋から外れますが #phperkaigi #c   ©RAKUS Co., Ltd. 12

Slide 13

Slide 13 text

PHP の最近の動向について #phperkaigi #c   ©RAKUS Co., Ltd. 13

Slide 14

Slide 14 text

新機能 型付きプロパティの導入 列挙型/交差型/never 型/true ・false ・null 型の追加 readonly アクセス修飾子/readonly クラスの導入 非推奨 PHP 8.0 :非厳密な比較演算子の挙動変更 PHP 8.1 :ビルトイン関数の nullable でない引数に null を渡す PHP 8.2 :動的プロパティが非推奨 #phperkaigi #c 型定義の厳密化するPHP   ©RAKUS Co., Ltd. 14

Slide 15

Slide 15 text

静的型付け言語に近づいていくPHP #phperkaigi #c   ©RAKUS Co., Ltd. 15

Slide 16

Slide 16 text

閑 話 / 休 題 Kan      Wa        Kyu     Dai

Slide 17

Slide 17 text

課題 null に関連する不具合が頻発 コードレビューの時間増加による開発効率の低下 PhpStorm のInspection はCI 化されていない SonarQube はPHP のコード解析力が足りない ⇒ 既存の静的解析ツールでは力不足... 開発チームの課題

Slide 18

Slide 18 text

他に良い解析ツールはないか... ?

Slide 19

Slide 19 text

No content

Slide 20

Slide 20 text

PHPStan PHP コードを静的に解析する 実行時にエラーとなるような問題のあるコードを検知する https://phpstan.org/ PHPStan のメリット 型定義のチェック 独自ルールの追加の容易さ #phperkaigi #c PHPStan とは   ©RAKUS Co., Ltd. 20

Slide 21

Slide 21 text

なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ   ©RAKUS Co., Ltd. 21

Slide 22

Slide 22 text

PHPStan を導入する上では 解析レベルをどうするか 既存エラーへの対処 実行方法 メイン担当者を決める #phperkaigi #c PHPStan 導入   ©RAKUS Co., Ltd. 22

Slide 23

Slide 23 text

#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

Slide 24

Slide 24 text

検討 既存のレガシーコードに高い解析レベルが修正コストが高い 直近でもnull に関連する不具合が頻発していたこともありnull チェックはしたい 対応 解析レベルは8 既存コードは検知対象外とする baseline を作成して対応 #phperkaigi #c PHPStan 導入:解析レベルと既存コード   ©RAKUS Co., Ltd. 24

Slide 25

Slide 25 text

既存の解析ツールの課題 PhpStorm のInspection はローカル実行のみ SonarQube の実行も手動でブランチを指定が必要 対応 PR 単位で自動チェックされるようにCI を設定 #phperkaigi #c PHPStan 導入:実行方法   ©RAKUS Co., Ltd. 25

Slide 26

Slide 26 text

メイン担当者を決めてPHPStan の相談を集約する 役割 課題の対応方針の検討 対応方法の相談 #phperkaigi #c PHPStan 導入:メイン担当者   ©RAKUS Co., Ltd. 26

Slide 27

Slide 27 text

ところで、気になりませんか? #phperkaigi #c   ©RAKUS Co., Ltd. 27

Slide 28

Slide 28 text

レガシーコードに対して 解析レベルをほぼMAX( レベル8) で 実行するとどれぐらいエラーが出るのか #phperkaigi #c   ©RAKUS Co., Ltd. 28

Slide 29

Slide 29 text

解析レベル8 で解析実行だ! #phperkaigi #c   ©RAKUS Co., Ltd. 29

Slide 30

Slide 30 text

約60,000 件のエラーを検知!! #phperkaigi #c ※対象 約5,000 ファイル(約35 万行)   ©RAKUS Co., Ltd. 30

Slide 31

Slide 31 text

ですよね... #phperkaigi #c   ©RAKUS Co., Ltd. 31

Slide 32

Slide 32 text

解析レベルはレベル8 既存コードはベースラインを設定し指摘対象外へ PR 単位で自動でチェックされるようにCI を設定 PHPStan の相談を集約するためにメイン担当者を決定 #phperkaigi #c PHPStan 導入   ©RAKUS Co., Ltd. 32

Slide 33

Slide 33 text

なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ   ©RAKUS Co., Ltd. 33

Slide 34

Slide 34 text

効果 PHPStan でバグを発見 カスタムルールを追加 コードレビュー対応時間の軽減 etc... #phperkaigi #c 効果①:具体的な効果   ©RAKUS Co., Ltd. 34

Slide 35

Slide 35 text

例)PHPStan でバグを発見 関数のオーバーライドの引数の型の不具合の発見 HogeMenuController.php:18:Method HogeMenuController::viewAction() overrides method AbstractMenuController::viewAction() but misses parameter #1 $form. 大規模リファクタリングを実施していたため、コードレビューから漏れていた。 PHPStan を導入していなければ、そのままリリースされていた可能性も... #phperkaigi #c 効果①:具体的な効果   ©RAKUS Co., Ltd. 35

Slide 36

Slide 36 text

効果 メンバーのセルフチェックの品質向上 各メンバーの型への意識が向上 #phperkaigi #c 効果②:メンバーの意識向上   ©RAKUS Co., Ltd. 36

Slide 37

Slide 37 text

なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ   ©RAKUS Co., Ltd. 37

Slide 38

Slide 38 text

課題 指摘の意味を調べるのに時間がかかる 1PR のコード量が多く、指摘が多くなる 対応 メンバーが指摘なれることで解決 PR の粒度を小さくする #phperkaigi #c 課題と対応①:指摘の修正に時間がかかる   ©RAKUS Co., Ltd. 38

Slide 39

Slide 39 text

例) $ ./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

Slide 40

Slide 40 text

課題 array を引数とする場合に正確な型指定が必要 match 文のdefaut 指定 標準関数が複数の型を返す問題 etc... 対応 除外ルールを設定して検査対象外 #phperkaigi #c 課題と対応②:過剰な指摘が多数発生   ©RAKUS Co., Ltd. 40

Slide 41

Slide 41 text

例)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

Slide 42

Slide 42 text

課題 型定義がない PHPDoc の型定義が間違っている 対応 ボーイスカウトルールを設定 型定義を追加 PHPDoc で型定義を追加 ※ 基本は動作に影響のないようにPHPDoc の追加で対応 #phperkaigi #c 課題と対応③:既存コードの型定義不足   ©RAKUS Co., Ltd. 42

Slide 43

Slide 43 text

順調に課題を解消していくが、 ここで問題発生... #phperkaigi #c   ©RAKUS Co., Ltd. 43

Slide 44

Slide 44 text

メイン担当者が不在へ... ( メンバーの異動などが重なり...) #phperkaigi #c   ©RAKUS Co., Ltd. 44

Slide 45

Slide 45 text

課題 どの粒度で修正するのかがチーム全体で決まらない どの指摘を修正するか ルールが決まり切らずに対応がバラバラへ... 暫定対応 どこまで修正するかはレビュアーとの認識合わせで合意 というルール 一部の指摘はbaseline に追加し指摘対象外へ #phperkaigi #c 課題と対応④:メイン担当者が不在による課題   ©RAKUS Co., Ltd. 45

Slide 46

Slide 46 text

残課題 ローカル実行する手段が少ないため再チェックに時間が掛かる 対応案 ローカルで簡易に実行できる手段の確立 CI の実行時間問題の解消 #phperkaigi #c 今後   ©RAKUS Co., Ltd. 46

Slide 47

Slide 47 text

なぜ導入したか PHPStan の導入 効果 課題と対応 まとめ #phperkaigi #c アジェンダ   ©RAKUS Co., Ltd. 47

Slide 48

Slide 48 text

得られたこと 型への意識が向上しクリーンなコードが作られるようになった レガシーなコードであってもベースラインや除外ルールを設定することで、解析 レベルが高い状態で運用することができた 注意点 自動でチェックされる仕組みにすること メイン担当者を決めてルールを整備すること #phperkaigi #c まとめ   ©RAKUS Co., Ltd. 48

Slide 49

Slide 49 text

ご清聴ありがとうございました。 #phperkaigi #c   ©RAKUS Co., Ltd. 49