はじめに
こんにちは。サーバーサイドエンジニアの佐野きよ（@Kiyo_Karl2）です。
今回とある理由で、特定のメソッド内で特定のメソッドを呼んでいるかチェックするPHPStanのカスタムルールを作成しましたので、その内容について紹介したいと思います。
カスタムルールを作成した背景
とある日、あるデータをダウンロードするジョブがそこまで高負荷な処理でないにもかかわらず、ずっとタイムアウトし続けるという事象が発生しました。
原因は、ジョブ内でWithoutOverlappingミドルウェアを利用したことによりRedisのロックが解放されないままずっと残っているということが原因でした。
上記ドキュメントを見ると、
WithoutOverlappingミドルウェアはLaravelのアトミックロック機能により動作します。時々、ジョブは予期せずに失敗したり、ロックが解放されないような原因でタイムアウトしたりすることがあります。そのため、expireAfterメソッドを使用して、ロックの有効期限を明示的に定義することができます。たとえば、以下の例では、ジョブが処理を開始してから３分後にWithoutOverlappingロックを解除するようにLaravelへ指示します。
といった記載があるため、ロックの有効期限をexpireAfter()で指定することで予期せぬロックを避けることができるようです。
そこで、WithoutOverlappingミドルウェアを利用している箇所でexpireAfter()が呼ばれているかチェックするカスタムルールを作成しようと考えました。
作成したカスタムルール
以下のようなカスタムルールを作成しました。
middlewareメソッド内でnew WithoutOverlappingした直後にexpireAfter()がチェーン呼び出しされているかどうか確認するシンプルなカスタムルールです。
PHPStanのカスタムルールの基礎知識については以下の記事で解説しているので、良ければ是非ご覧下さい。
<?php declare(strict_types=1); namespace phpstan\Rules; use Illuminate\Queue\Middleware\WithoutOverlapping; use PhpParser\Node; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\New_; use PhpParser\Node\Identifier; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\ShouldNotHappenException; /** * @implements Rule<New_> */ class MustCallExpireAfterRule implements Rule { private const MUST_CALL_METHOD = 'expireAfter'; private const SCOPE_FUNCTION = 'middleware'; public function getNodeType(): string { return MethodCall::class; } /** * @throws ShouldNotHappenException */ public function processNode(Node $node, Scope $scope): array { // middleware メソッド内でなければlintエラーを出さない if ($scope->getFunctionName() !== self::SCOPE_FUNCTION) { return []; } // そもそもメソッド内でインスタンス化の記述がなければlintエラーを出さない if (!$node->var instanceof New_) { return []; } // 名前空間を持つNodeであればlintエラーを出さない(メソッド呼び出しや定数などは名前空間を持たない) if (!$node->name instanceof Identifier) { return []; } if (!$this->isWithoutOverlappingInstance($node->var, $scope)) { return []; } if ($this->isDirectlyChainedWithExpireAfter($node->name)) { return []; } return [ RuleErrorBuilder::message( sprintf( 'You must call ->expireAfter() when creating a new %s instance (e.g. (new %s($key))->expireAfter(...))', WithoutOverlapping::class, WithoutOverlapping::class ), )->build(), ]; } /** * new しているクラスが WithoutOverlapping かどうかを判定する */ private function isWithoutOverlappingInstance(New_ $node, Scope $scope): bool { // クラス名が明示されていなければ false if (!$node->class instanceof Node\Name) { return false; } // クラスの完全修飾名を取得 $fqcn = $scope->resolveName($node->class); return $fqcn === WithoutOverlapping::class; } /** * (new WithoutOverlapping($key))->expireAfter(100) * というチェーン呼び出しになっているか判定する */ private function isDirectlyChainedWithExpireAfter(Identifier $node): bool { return $node->name === self::MUST_CALL_METHOD; } }
補足
簡易化のため以下のようなケースはオーバーエンジニアリングになると考えたため対応していません。
より厳密にチェックしたいケースの場合は、以下のようなケースも想定する必要があります。
一旦変数に入れられるケース
この場合だと上記のカスタムルールだとexpireAfter()を呼んでいないという扱いになってしまいます。現状だとこのような一旦変数に入れるような書き方をしている箇所はゼロだったので対応するのをやめました。
<?php $w = new WithoutOverlapping(); $w->expireAfter();
インスタンス化直後にメソッドチェーンされないケース
これを許容しようとすると、「チェーンされたすべてのメソッドを抽出し、その中に特定のメソッドがあるか確認する」といったやや複雑な実装をしないといけなくなりそうので、今回は許容しないようなカスタムルールとしました。むしろ記述が統一されるので許容しない方が良い気もします。
<?php (new isWithoutOverlappingInstance()) ->releaseAfter() ->expireAfter();
カスタムルールのテスト
RuleTestCaseを継承することで以下のようにカスタムルールのテストを書くことができます。
テストケースとして解析対象のファイルデータを用意し、analyse()メソッドに解析対象ファイルのパスを渡すことでPHPUnit上でlintを実行することができます。
MustCallExpireAfterRuleTest.php
<?php declare(strict_types=1); namespace Tests\PHPStan\Rules; use phpstan\Rules\MustCallExpireAfterRule; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; final class MustCallExpireAfterRuleTest extends RuleTestCase { protected function getRule(): Rule { return new MustCallExpireAfterRule(); } /** * @testdox WithoutOverlappingをインスタンス化した直後にexpireAfterを呼んでいる場合はlintエラーが出ない */ public function testNoMissingExpireAfter(): void { $this->analyse( [__DIR__ . '/testdata/NoMissingExpireAfter.php'], [] ); } /** * @testdox WithoutOverlappingをメソッド呼び出ししている場合はlintエラーが出ない(Console/Kernel.phpのスケジュール設定など) */ public function testFuncCallWithoutOverlapping(): void { $this->analyse( [__DIR__ . '/testdata/MethodCallWithoutOverlapping.php'], [] ); } /** * @testdox expireAfterを呼んでいない場合はlintエラーが出る */ public function testMissingExpireAfter(): void { $this->analyse( [__DIR__ . '/testdata/MissingExpireAfter.php'], [ // [エラーメッセージ, 行番号] [ 'You must call ->expireAfter() when creating a new Illuminate\Queue\Middleware\WithoutOverlapping instance (e.g. (new Illuminate\Queue\Middleware\WithoutOverlapping($key))->expireAfter(...))', 14, ], ] ); } }
NoMissingExpireAfter.php
<?php declare(strict_types=1); namespace Tests\Unit\PHPStan\Rules\testdata; use Illuminate\Queue\Middleware\WithoutOverlapping; class NoMissingExpireAfter { public function middleware(): array { return [ (new WithoutOverlapping('key')) ->expireAfter(120), ]; } }
MethodCallWithoutOverlapping.php
<?php declare(strict_types=1); namespace Unit\PHPStan\Rules\testdata; use Illuminate\Console\Scheduling\Schedule; class MethodCallWithoutOverlapping { public function schedule(Schedule $schedule): void { $schedule->command('test:command') ->hourly() ->onOneServer() ->withoutOverlapping(); } }
MissingExpireAfter.php
<?php declare(strict_types=1); namespace Tests\Unit\PHPStan\Rules\testdata; use Illuminate\Queue\Middleware\WithoutOverlapping; class MissingExpireAfter { public function middleware(): array { return [ (new WithoutOverlapping('key')) ->releaseAfter(120), ]; } }
まとめ
上記のように、カスタムルールは標準ルールでは実現できないかなり柔軟なlintチェックを実行することができます。
他にもPHPのswitchを禁止するとか、メソッドのプロジェクト独自の命名規則を強制するとか、いろいろ応用できそうなので、是非この機会に簡単なカスタムルールから作成してみてはいかがでしょうか。
さいごに
ヤプリではサーバーサイドエンジニアを随時募集しています! 興味を持った方、是非一度カジュアル面談を受けてみませんか…??
最後まで読んでいただきありがとうございました!
