PHPerKaigi 2022をきっかけにPHPStanにコントリビュートした話 / PHPerKaigi makes me PHPStan's Contributor

Transcript

1. PHPerKaigi 2022をきっかけにPHPStanに コントリビュートした話 2022/5/27 PHPerのための「静的解析」を語り合うPHP TechCafe muno92 1
2. 自己紹介 twitter: @muno_92 PHP（≒ エンジニア）歴5年くらい 型や静的解析は好き https://github.com/muno92/resharper_inspectcode GitHub ActionsでC#のコードを静的解析するアクション JetBrainsのReSharper command line toolsを使用 2
3. こんなルールを作りました <?php $dt = new DateTimeImmutable(); $dt->__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
4. 今回作ったルールを有効にするための設定 phpstan.neon includes: - vendor/phpstan/phpstan/conf/bleedingEdge.neon - vendor/phpstan/phpstan-strict-rules/rules.neon parameters: # レベルは2 以上 level: 2 paths: - 解析対象のパス 4
5. 伝えたいこと PHPStanにPull Requestを投げたときの盛り上がり PHPStanのルール作成は意外とハードル高くない 5
6. 話さないこと PHPStanのルールの詳しい作り方 6
7. 静的解析ツールはソースコードをパーツに分解・解析している https://www.php.net/manual/ja/function.token-get-all.php ライブラリを使っていても基本的な考え方は同じ （PHPStanはPHP Parserを使用） 7
8. 今日はこれだけ分かっていればOK 8
9. PHPStanのルールの詳しい作 り方はこちら https://speakerdeck.com/nazonoh ito51/make-phpstan-customrule- d596e237-6692-4e6b-b83b- f5fac3618797 https://youtu.be/xemXGgx0w0k 9
10. 話の発端 (PHPerKaigi 2022 day1) 予防に勝る防御なし - 堅牢なコードを導く様々な設計のヒント (twadaさん) 10
11. 既存の静的解析ツールで検知出来る？ PHPerKaigi翌日、4/12(火)の9時頃 PHPerKaigi参加者用のDiscordにtwadaさんが問いかけ 確認したところ、PHPStanやPsalmに明示的にコンストラクタの直接実行を禁止するル ールは無かった (詳しくはブログ参照) 11
12. コントリビュートチャンス？ twadaさん「コントリビュートのチャンス！！」 （ぼんやりした実装イメージはあるけど、OSSへのコントリビュートはハードルを感じ てしまう・・・） 12
13. 実装イメージ PHPStanは1つ1つのチェック項目をルールクラスとして実装していたので、 1. ソースコード内でメソッド呼び出しを行っている箇所を解析し 2. 呼び出しているメソッドが__constructで 3. 正常な使い方をしていない場合 にエラーを返す良い感じのルールクラスを実装すれば良さそう （1〜3番の具体的な実装方法は思いついていない） 13
14. 良い感じのルールクラス <?php declare(strict_types = 1); class MyRule implements Rule { public function getNodeType(): string { // 一致するコードがprocessNode でチェックされる return Node\Expr\ メソッド呼び出しを行っているパーツ::class; } public function processNode(Node $node, Scope $scope): array { // __construct 実行をチェック if ( エラーでない場合) { return []; } return [' エラーです']; } } 14
15. そうこうしている間に興味を持つ人が増える 20時過ぎ （ハードルは感じるけど無さそうだし作るかなあ・・・）Discordを見る tadsanさん、mpywさんが興味を持つ 自分「スピード勝負になりつつある・・・！？」 15
16. PHPerKaigi (day3?) ハッカソン爆誕 加速するDiscord 名前どうする？ 「1. ソースコード内でメソッド呼び出しを行っている箇所を解析し」　　 どのノードタイプで引っ掛ける？ 「3. 正常な使い方をしていない場合」　　 弾いてはいけない正常なパターンはどれ？ どこに実装する？ PHPStan本体 phpstan-strict-rules こんな感じのコードでいけそう (from mpywさん、yu-ichiroさん) 16
17. これはOK class MyDateTimeImmutable extends DateTimeImmutable { public function __construct(string $datetime = "now", ?\DateTimeZone $timezone = null)) { parent::__construct($datetime, $timezone); } } 17
18. これは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
19. こういったパターンもある (OK?) from mpywさん class MyDateTimeImmutable extends DateTimeImmutable { public function __construct(string $datetime = "now", ?\DateTimeZone $timezone = null)) { self::__construct($datetime, $timezone); } } ※ 許容する形で実装したが、最終的にマージされたコードではNGに 19
20. 話がまとまり、実装を任して頂ける事に OK/NGのパターンが整理出来た PHPStan本体組み込みのルールとして提案する DateTimeImmutableインスタンスのコンストラクタを直接呼び出す事により、安全 でないコードを再現出来てしまうため 20
21. 実装（PHPStan作者の方による調整が入った最終形） <?php declare(strict_types = 1); namespace PHPStan\Rules\Methods; // use は行数の都合で省略 class IllegalConstructorMethodCallRule implements Rule { public function getNodeType(): string { // 戻り値のNodeType （今回で言えばメソッド呼び出し）が解析対象のコードに含まれていた場合、processNode で解析する return Node\Expr\MethodCall::class; } public function processNode(Node $node, Scope $scope): array { if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== '__construct') { // エラー無し return []; } return [ RuleErrorBuilder::message('Call to __construct() on an existing object is not allowed.') ->build(), ]; } } どうやってここに行き着く？ 21
22. とりあえず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
23. もりもり実装 MethodCall( ->__construct )に対応 StaticCall( ::__construct )も同様にvar_dumpで型情報を確認し、実装 23
24. シュッと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
25. Pull Request出した、ヨシ！ 25
26. 焦ってCIに引っかかり、修正 この時点で0:32。 焦ってしまい、ローカルで静的解析やLintを走らせていなかった。 テスト Nullsafe Operator ?-> の使用がPHP7系でのテストで引っ掛かる ローカルではPHP8.1でテストを走らせていたが、CIではPHP7.2〜8.1でテスト テスト対象のコードに記載した再帰コンストラクタが原因で一部のテストでメモリ エラーに テスト対象のコードで再帰を1回に抑える 静的解析 tadsanさんが助けて下さる Lint phpcbfで自動修正 26
27. CIの完了を固唾を飲んで待つ面々 なんと驚きのジョブ数244 27
28. CI (テスト) 28
29. CI (E2Eテスト) 29
30. CI (Lint) 30
31. CI (静的解析) 31
32. CI (Pharコンパイルなど) （phpstan-phpunitなど拡張のテストも実行） 32
33. All グリーン！ 33
34. その後 4/13 承認され、Ondreさんから後は引き取るとコメント頂く 4/17 調整が入った上でマージされた 4/26 PHPStan 1.6.0でBleeding Edgeとして利用可能に 5/5 PHPStan 1.6.6でphpstan-strict-rulesに移動 34
35. まとめ PHPerKaigiをきっかけに、ハッカソン的にPHPStanにコントリビュートした 意外とPHPStanのルールを作成するハードルは高くない PHPStanへのコントリビュートが全てでは無い 「こういったコードを検知したい」をルールに落とし込んでみる 自社・自チームのコーディング規約をルールとして実装するなど 手を動かしてみるとなんとかなるかもしれない 35
36. 良い静的解析ライフを 36