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

Slide 1

Slide 1 text

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

Slide 2

Slide 2 text

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

Slide 3

Slide 3 text

こんなルールを作りました __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

Slide 4

Slide 4 text

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

Slide 5

Slide 5 text

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

Slide 6

Slide 6 text

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

Slide 7

Slide 7 text

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

Slide 8

Slide 8 text

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

Slide 9

Slide 9 text

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

Slide 10

Slide 10 text

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

Slide 11

Slide 11 text

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

Slide 12

Slide 12 text

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

Slide 13

Slide 13 text

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

Slide 14

Slide 14 text

良い感じのルールクラス

Slide 15

Slide 15 text

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

Slide 16

Slide 16 text

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

Slide 17

Slide 17 text

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

Slide 18

Slide 18 text

これは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

Slide 19

Slide 19 text

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

Slide 20

Slide 20 text

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

Slide 21

Slide 21 text

実装（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

Slide 22

Slide 22 text

とりあえず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

Slide 23

Slide 23 text

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

Slide 24

Slide 24 text

シュッと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

Slide 25

Slide 25 text

Pull Request出した、ヨシ！ 25

Slide 26

Slide 26 text

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

Slide 27

Slide 27 text

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

Slide 28

Slide 28 text

CI (テスト) 28

Slide 29

Slide 29 text

CI (E2Eテスト) 29

Slide 30

Slide 30 text

CI (Lint) 30

Slide 31

Slide 31 text

CI (静的解析) 31

Slide 32

Slide 32 text

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

Slide 33

Slide 33 text

All グリーン！ 33

Slide 34

Slide 34 text

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

Slide 35

Slide 35 text

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

Slide 36

Slide 36 text

良い静的解析ライフを 36