PHPerKaigi 2022をきっかけにPHPStanに コントリビュートした話 2022/5/27 PHPerのための「静的解析」を語り合うPHP TechCafe muno92 1

自己紹介 twitter: @muno_92 PHP（≒ エンジニア）歴5年くらい 型や静的解析は好き https://github.com/muno92/resharper_inspectcode GitHub ActionsでC#のコードを静的解析するアクション JetBrainsのReSharper command line toolsを使用 2

こんなルールを作りました __construct('2022/1/23'); ↓ > ./vendor/bin/phpstan analyse -c phpstan.neon ------ ------------------------------------------------------------- Line sample.php ------ ------------------------------------------------------------- 4 Call to __construct() on an existing object is not allowed. ------ ------------------------------------------------------------- ※ デフォルトでは有効になっていないため、次ページ記載の設定が必要 3

今回作ったルールを有効にするための設定 phpstan.neon includes: - vendor/phpstan/phpstan/conf/bleedingEdge.neon - vendor/phpstan/phpstan-strict-rules/rules.neon parameters: # レベルは2 以上 level: 2 paths: - 解析対象のパス 4

伝えたいこと PHPStanにPull Requestを投げたときの盛り上がり PHPStanのルール作成は意外とハードル高くない 5

話さないこと PHPStanのルールの詳しい作り方 6

静的解析ツールはソースコードをパーツに分解・解析している https://www.php.net/manual/ja/function.token-get-all.php ライブラリを使っていても基本的な考え方は同じ （PHPStanはPHP Parserを使用） 7

今日はこれだけ分かっていればOK 8

PHPStanのルールの詳しい作 り方はこちら https://speakerdeck.com/nazonoh ito51/make-phpstan-customrule- d596e237-6692-4e6b-b83b- f5fac3618797 https://youtu.be/xemXGgx0w0k 9

話の発端 (PHPerKaigi 2022 day1) 予防に勝る防御なし - 堅牢なコードを導く様々な設計のヒント (twadaさん) 10

既存の静的解析ツールで検知出来る？ PHPerKaigi翌日、4/12(火)の9時頃 PHPerKaigi参加者用のDiscordにtwadaさんが問いかけ 確認したところ、PHPStanやPsalmに明示的にコンストラクタの直接実行を禁止するル ールは無かった (詳しくはブログ参照) 11

コントリビュートチャンス？ twadaさん「コントリビュートのチャンス！！」 （ぼんやりした実装イメージはあるけど、OSSへのコントリビュートはハードルを感じ てしまう・・・） 12

実装イメージ PHPStanは1つ1つのチェック項目をルールクラスとして実装していたので、 1. ソースコード内でメソッド呼び出しを行っている箇所を解析し 2. 呼び出しているメソッドが__constructで 3. 正常な使い方をしていない場合 にエラーを返す良い感じのルールクラスを実装すれば良さそう （1〜3番の具体的な実装方法は思いついていない） 13

良い感じのルールクラス

そうこうしている間に興味を持つ人が増える 20時過ぎ （ハードルは感じるけど無さそうだし作るかなあ・・・）Discordを見る tadsanさん、mpywさんが興味を持つ 自分「スピード勝負になりつつある・・・！？」 15

PHPerKaigi (day3?) ハッカソン爆誕 加速するDiscord 名前どうする？ 「1. ソースコード内でメソッド呼び出しを行っている箇所を解析し」　　 どのノードタイプで引っ掛ける？ 「3. 正常な使い方をしていない場合」　　 弾いてはいけない正常なパターンはどれ？ どこに実装する？ PHPStan本体 phpstan-strict-rules こんな感じのコードでいけそう (from mpywさん、yu-ichiroさん) 16

これはOK class MyDateTimeImmutable extends DateTimeImmutable { public function __construct(string $datetime = "now", ?\DateTimeZone $timezone = null)) { parent::__construct($datetime, $timezone); } } 17

これはNG class MyDateTimeImmutable extends DateTimeImmutable { public function mutate(string $datetime = "now", ?\DateTimeZone $timezone = null)) { parent::__construct($datetime, $timezone); } } $dt = new DateTimeImmutable(); $dt->__construct('2022/1/23'); 18

こういったパターンもある (OK?) from mpywさん class MyDateTimeImmutable extends DateTimeImmutable { public function __construct(string $datetime = "now", ?\DateTimeZone $timezone = null)) { self::__construct($datetime, $timezone); } } ※ 許容する形で実装したが、最終的にマージされたコードではNGに 19

話がまとまり、実装を任して頂ける事に OK/NGのパターンが整理出来た PHPStan本体組み込みのルールとして提案する DateTimeImmutableインスタンスのコンストラクタを直接呼び出す事により、安全 でないコードを再現出来てしまうため 20

実装（PHPStan作者の方による調整が入った最終形） name instanceof Node\Identifier || $node->name->toLowerString() !== '__construct') { // エラー無し return []; } return [ RuleErrorBuilder::message('Call to __construct() on an existing object is not allowed.') ->build(), ]; } } どうやってここに行き着く？ 21

とりあえずvar_dump 3ページ目のコードを解析した場合のvar_dump($node)を抜粋 object(PhpParser\Node\Expr\MethodCall)#3146 (4) { ["attributes":protected]=> array(9) {} // 4 行目にあるdt という名前の変数がメソッドを呼び出している ["var"]=> object(PhpParser\Node\Expr\Variable)#3142 (2) { ["attributes":protected]=> array(9) { ["startLine"]=> int(4) } ["name"]=> string(2) "dt" } // 呼び出しているメソッドの名前は__construct ["name"]=> object(PhpParser\Node\Identifier)#3143 (2) { ["attributes":protected]=> array(6) {} ["name"]=> string(11) "__construct" } // 今回は引数の情報は要らない ["args"]=> array(1) {} } 22

もりもり実装 MethodCall( ->__construct )に対応 StaticCall( ::__construct )も同様にvar_dumpで型情報を確認し、実装 23

シュッとIssueを立てて下さるtadsanさん 実装している間に、PSR-7にIssue作成 https://github.com/Nyholm/psr7/issues/198 Pull Request提出用にコードを整えている間にPHPStanにもIssueを作成し、PHPStan作 者のOndreさんの意見も確認して下さる https://github.com/phpstan/phpstan/issues/7022 24

Pull Request出した、ヨシ！ 25

焦ってCIに引っかかり、修正 この時点で0:32。 焦ってしまい、ローカルで静的解析やLintを走らせていなかった。 テスト Nullsafe Operator ?-> の使用がPHP7系でのテストで引っ掛かる ローカルではPHP8.1でテストを走らせていたが、CIではPHP7.2〜8.1でテスト テスト対象のコードに記載した再帰コンストラクタが原因で一部のテストでメモリ エラーに テスト対象のコードで再帰を1回に抑える 静的解析 tadsanさんが助けて下さる Lint phpcbfで自動修正 26

CIの完了を固唾を飲んで待つ面々 なんと驚きのジョブ数244 27

CI (テスト) 28

CI (E2Eテスト) 29

CI (Lint) 30

CI (静的解析) 31

CI (Pharコンパイルなど) （phpstan-phpunitなど拡張のテストも実行） 32

All グリーン！ 33

その後 4/13 承認され、Ondreさんから後は引き取るとコメント頂く 4/17 調整が入った上でマージされた 4/26 PHPStan 1.6.0でBleeding Edgeとして利用可能に 5/5 PHPStan 1.6.6でphpstan-strict-rulesに移動 34

まとめ PHPerKaigiをきっかけに、ハッカソン的にPHPStanにコントリビュートした 意外とPHPStanのルールを作成するハードルは高くない PHPStanへのコントリビュートが全てでは無い 「こういったコードを検知したい」をルールに落とし込んでみる 自社・自チームのコーディング規約をルールとして実装するなど 手を動かしてみるとなんとかなるかもしれない 35

良い静的解析ライフを 36