長期インターンで知識0からRectorのルールを実装した話

こんにちは！pixiv事業本部ウェブエンジニアリング部で長期インターンをしていた@jiroooooと申します！

今回、型や構文解析などのプログラミング言語論の知識がほとんどない状態からRectorで特定のコードを一括で置換するルールを作成しました。

コードを書く以上のことをあまりしたことがない私でもここまでできるようになったということを伝えたく今回記事を執筆しました

empty($array)を置き換えたい

emptyは変数が空かどうかを返す言語構造ですが、空と判定するパターンがかなり広く(0や”0”、はたまた未定義変数まで空であると判定してしまいます)思わぬバグを引き起こすことがあります。

そのためemptyからcount($array)===0のように置き換える必要があり、指定したルールでコードの置換をしてくれるRectorというツールを使うことにしました。

ということで実際にRectorに組み込まれているルールでコードを書き換えてみました！ルールを指定してvendor/bin/rector process <filePath> を打つだけなので楽勝ですね

これでタスク完了！と思いきや、動作確認すると期待通りに動作しない箇所があることがわかりました。このことをメンターの@tadsanに相談したところ、「使用したルールは配列変数以外を想定していないようで、これが不具合を引き起こしているのではないか」と推測していました。そこで@tadsanは「じゃあ配列以外もカバーするルールを自作してみよう！」とおっしゃり、構文木などを学習しながらルールを実装することになりました。(ただし後述するように、@tadsanの見解はこの問題の根本的な原因ではなかった可能性があります)

Rectorとは

Rectorは指定したルールに基づいてプロジェクト内のコードを置換してくれるツールです。内部的な振る舞いとしては、置き換えたいノードを特定し新規のノードを作成し置換しています。

文章だけだと分かりにくいと思うのでコードを用いて説明します

今回だと、置き換え前後のノードの名前が必要なため以下のコードを構文解析します。

<?php
empty($array);
count($array)===0;

RectorはPHP-Parserというライブラリで構文解析を行なっています。付属する php-parse というツールでソースコードを構文解析すると、以下のような構文木が出力されます。

見慣れていないとあまり何が書かれているかわからないと思いますが、empty のノード名はExpr_Empty、countのノード名はExpr_Function内のNameで呼ばれているように全てノードの組み合わせで表されていることがわかります。

array(
    0: Stmt_Expression(
            // empty();
        expr: Expr_Empty(
                // 変数
            expr: Expr_Variable(
                name: array
            )
        )
    )
    1: Stmt_Expression(
            // "==="
        expr: Expr_BinaryOp_Identical(
            left: Expr_FuncCall(
                name: Name(
                    name: count
                )
                args: array(
                    0: Arg(
                        name: null
                        value: Expr_Variable(
                            name: array
                        )
                        byRef: false
                        unpack: false
                    )
                )
            )
            right: Scalar_Int(
                value: 0
            )
        )
    )
)

先ほど調べた名前はPHP-Parserの PhpParser\Node を継承したクラスとして存在しています。

この対応するクラスを利用し、置換前後のノードを生成しコードを書いていきます。

このようにして完成したコードは

Expr_Empty が含まれるノードのみに絞り込む
empty() に渡される引数の型によって条件分岐を行いノードを生成
対応するノードに置換

というような流れで実装されています。　

参考までにコードの一部をお見せします。

public function refactor(Node $node): ?Node
    {
        $realNode = $node;
        $isBooleanNot = false;
        if ($node instanceof BooleanNot && $node->expr instanceof Empty_) {
            $realNode = $node->expr;
            $isBooleanNot = true;
        }

        if (!$realNode instanceof Empty_) {
            return null;
        }

        $subjectType = $this->getType($realNode->expr);

        echo $subjectType->describe(VerbosityLevel::cache()), ' => ', $subjectType->isNull()->describe(),PHP_EOL;
        if ($subjectType->describe(VerbosityLevel::cache()) === 'array') {
            var_dump($subjectType);
        }

        $conditions = [];

        if (!$subjectType->isArray()->no()) {
            $left = new FuncCall(
                new Name('\count'),
                [new Arg($realNode->expr)]
            );
            $right = new LNumber(0);
            $conditions[] = $isBooleanNot ? new NotIdentical($left, $right) : new Identical($left, $right);
        }

        $possibleEmptyValues = [];

        if (!$subjectType->isFalse()->no()) {
            $possibleEmptyValues[] = new ConstFetch(new Name('false'));
        }

        if (!$subjectType->isNull()->no()) {
             $possibleEmptyValues[] = new ConstFetch(new Name('null'));
        }

特徴的なところは、引数に渡される型によって最適な比較コードを生成することです。

Rectorは以下のようなテストコードで ----- の前後にbefore/afterを並べることでコードが実際にどのように書き換えられるかテストすることができます。

<?php

function array_false(array|false $test)
{
    return empty($test);
}

?>
-----
<?php

function array_false(array|false $test)
{
    return $test === false || \count($test) === 0;
}

?>

テストが落ちる！！！

ルール完成後、テストを実行したのですが、引数の型がarray|nullであるテストケースが落ち続けるという問題が発生しました。

@tadsanとデバッグなどをしながら調べたところ、Rectorがnull許容型をうまく扱えず、getType()がnullを除いた型を返しているということが発覚しました。

つまり、引数の型が array|null の時は array 、 array|string|null の時は array|string というように解釈してしまいます。Rectorは型解析にPHPStanを利用していますが、PHPStan単体で実行したときは期待通りに解釈されました。

いくら原因を特定しようとしても解決できず、時間の問題もあるため泣く泣くこのタスクは中断することになってしまいました…

よくわからんのでtadsanと一緒に考えてみた

冒頭で、@tadsanは「使用したルールは配列変数以外を想定していないようで、これが不具合を引き起こしているのではないか」と推測していましたが、Rectorのルールを見るとemptyの引数が配列でなかった場合はコードを書き換えない処理になっていました。

ここから考えられる可能性はいくつかあります

型宣言ではなくPHPDocでarrayとして宣言されているが、実際にはfalseやnullなどの別の値が渡されてしまっていてemptyから$hoge===[]のようなコードに書き換えられてしまったため、条件が満たされなくなってしまった。
書き換えに特に問題はなかったが、別の要因で開発環境が正常に動作してなかった。

後者も考えにくい状況ではあるのですが、ユニットテストは特に失敗していなかったので可能性としては否定できません。この記事を書く際、最初に作業して不具合が見られたブランチで再確認をしてみたところ、本番にデプロイされている最新のソースコードと同じ表示になり不具合の再現はできませんでした。

そもそも型が信頼できず配列以外の可能性があるならば、型チェックを行わず(0やfalseなどの)全てのemptyになる値との比較に置き換えなければ等価な置き換えにならない恐れがあります…