We Are GRIPHONE / サーバー

PHPの静的解析導入について本気出して考えてみた

MoriKensukePosted by

この記事は GRIPHONE Advent Calendar 2022 12日目の記事です。

こんにちは。サーバーサイドエンジニアの森です。この記事では複数あるPHPの静的解析ツールを比較調査し、プロジェクトに導入した過程を書きます。

PHP Conference 2022のリリースして11年経過したPHPアプリケーションにPHPStanを導入したの発表で静的解析を導入することで「思い切ったリファクタが簡単にできるようになる」、「ライブラリアップデートが安心して行えるようになる」などの利点があったと聞き、弊社のプロジェクトでも試してみようと調査を始めました。

解析対象のプロジェクトは以下になります。

  • 運用3.5年目のゲームのPHPサーバーサイドエンジン
  • PHPファイル数: 18190
  • PHPのバージョン: 8.0.13
  • ライブラリ管理: composer

ツール選びの観点

今回、静的解析ツールを選ぶ観点で重視したのが「エラーを発見できるか」、「バグになりやすいコードを発見できるか」になります。静的解析ツールにはコーディング規約やコードフォーマットを重視するものもありますが、弊社のプロジェクトではIDEとしてIntelliJ/PhpStormを使っており、そのあたりはほとんど問題になっていなかったため、そちらは重視していません。

ツール選びする上で検知を試すエラー

どういうエラーを検知できるか判定する試金石として、上記のプロジェクトをPHP8.1系にあげるのを試した時に発見したエラーを含んだファイルを解析することにしました。

  1. 継承先が継承元と互換性がない
Return type of CLASS_NAME_1::METHOD_NAME() should either be compatible with CLASS_NAME_2::METHOD_NAME(): TYPE, or the #[\\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

2. floatがintに暗黙的に変換されてる

Implicit conversion from float NUMBER to int loses precision

3. int型にnullが渡されてる

preg_split(): Passing null to parameter #3 (VAR_NAME) of type int is deprecated

4. 関数の引数でoptionalパラメータの後に必須パラメータが定義されてる

Deprecated: Optional parameter VAR_NAME_1 declared before required parameter VAR_NAME_2 is implicitly treated as a required parameter

ツールの使用感調査

今回、試してみたものは以下になります。

この中でも特に有力だったphan、PHPStan、Psalmについて詳細を説明します。

phan

バグ発見もできるし、コードスタイルも細かく指摘してくれます。

長所

  • PHP形式のconfigで挙動を細かく設定できる。対象のPHPのバージョンや、除外するディレクトリや報告するissueのタイプを始め、他にも項目が山ほどある。
  • 検知できる項目が多い。
  • 実行が高速で3〜4分で終わる。

短所

  • 無駄と思われる「PHPのバージョンが現在使っているPHPより低い場合に動かない」issueがconfigで抑制しないと消えてくれない。例えば、PHP8で実行しても「PHP8未満だとmatch式が使えない」のようなissueが検知されてしまう。PHP8なので問題なく使える。
  • php-astのextensionを入れる必要がある。

エラー検知

エラー試金石の3, 4を検知できました。1については、継承元が Iterator だと駄目だったのですが、手元で適当にクラスを作ったら検知してくれました。継承元がPHP組み込みの型や、interfaceだと検知してくれないのかもしれません。

input:31: PhanParamReqAfterOpt Required parameter (TYPE_1 VAR_NAME_1) follows optional (?TYPE_2 VAR_NAME_2 = null)

input:2741: PhanTypeMismatchArgumentInternalReal Argument 3 (VAR_NAME) is null of type null but \preg_split() takes int

PHPStan

今回調査した中で一番Githubのstar数が多く、PHPの静的解析ライブラリの中ではかなり有名なようです。バグ発見に重きをおいている印象を受けました。

長所

  • 解析のキャッシュ機能があり、2回目以降の解析がとても早く10秒ほどで終わった。解析対象ファイルの内容が変わらない場合のみ。

短所

  • 初回の解析がとても遅い。3時間では終わらなかった。
  • 解析時にPHPのメモリをかなり食う。1Gでは足りなかった。

エラー検知

試金石の1と3を検知できました。2はissueが出てました。

 64     Return type mixed of method CLASS_NAME_1::METHOD_NAME is not covariant with tentative return type mixed of method CLASS_NAME_2::METHOD_NAME.
         💡 Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.
 
2741   Parameter #3 $limit of function preg_split expects int, null given.

Psalm

Phanと同様にバグ検知もコードスタイルも検知してくれます。

長所

  • エラーメッセージになんで駄目なのかのlinkがついていて、理由を説明してくれる。
  • 実行が高速で3〜4分で終わる。

短所

  • 同じ条件で解析してるのに結果が変わることがある
  • namespaceが異なるクラスを呼んでくれないことがある
  • –set-baselineオプションをつけてbaselineを作成すると解析結果が変わることがある

エラー検知

試金石の1と3を検知できました。2については対応するか議論しているようです。

ERROR: MethodSignatureMustProvideReturnType  - Method CLASS_NAME::METHOD_NAME must have a return type signature! (see https://psalm.dev/282)

ERROR: NullArgument - Argument 3 of preg_split cannot be null, null value provided to parameter with type int (see https://psalm.dev/057)

ツールの決定

どのツールも検知できないエラーがあるので、幅広くカバーできるよう、使い勝手の良かったphanと、Psalmを使うことにしました。

phanは使い勝手が良かったので使うのを決定しました。ただphanでは検知できなかったエラーを検知できたツールをあと1つ使おうと思い、解析結果はよいけどリソース消費が大きいPHPStanか、リソース消費は少ないけど解析結果が不安定なPsalmかで悩みましたが、いったんPsalmを使ってみることにしました。まだ静的解析の経験が少ないため、解析のイテレーションを高速で回したかったのと、社内の他プロジェクトでPHPStanを使っていたため社内に多くのツールの知見を貯めたいという意図です。

導入

phan

まずcomposerでphanをインストールします。

composer require --dev "phan/phan:5.x"

続いて、php-astのextensionをインストールします。

pecl install ast

続いてconfigを作ります。ドキュメントによると phanを叩けばいいと書いてあるのですが、エラーが出てうまくいかなかったので手で作りました。

ドキュメントの厳しい設定をベースに、プロジェクトに合うように検知除外するissueタイプやディレクトリを設定していきます。基本的にはなるべく厳しくしておき、必要に応じて緩めるスタイルです。

<?php

//declare(strict_types=1);

use Phan\Issue;

return [
    'target_php_version'                                             => null,

    'minimum_target_php_version'                                     => '^8.0',

...

    // Backwards Compatibility Checking (This is very slow)
    'backward_compatibility_checks'                                  => false,

...

    'minimum_severity'                                               => Issue::SEVERITY_LOW,

    'suppress_issue_types'                                           => [
        // phpのバージョン固有のissueは不要なので弾く
        'PhanCompatibleAnyReturnTypePHP56',
        'PhanCompatibleScalarTypePHP56',
        'PhanCompatibleNullableTypePHP70',
        'PhanCompatibleNullsafeOperator',
        'PhanCompatibleConstructorPropertyPromotion',
        'PhanCompatibleTypedProperty',
        'PhanPluginNoCommentOnClass', // Classにはコメント不要
        'PhanPluginRedundantMethodComment', // getterのコメントが怒られる？
        'PhanCompatibleMixedType', // mixed型が怒られる
        'PhanCompatibleVoidTypePHP70', // void型が怒られる
        'PhanThrowTypeAbsentForCall', // 深い階層で投げられるエラーのdoc書いてないと怒られる
        'PhanPluginDescriptionlessCommentOnProtectedMethod', // protectedのコメント
        'PhanPluginPossiblyStaticProtectedMethod', // Protected method can be static
        'PhanPossiblyNonClassMethodCall',
        'PhanPluginRedundantReturnComment',
        'PhanCompatibleMatchExpression', // Cannot use match expressions before php 8.0
    ],

...

    'exclude_analysis_directory_list'                                => [
        'WORKSPACE/config/',
        'WORKSPACE/logs/',
        'WORKSPACE/libraries/Api/',
        'WORKSPACE/vendor/',
    ],

...

];

つづいて、baselineを作成します。baselineを使うと現在時点での解析で出ているissueを無視することができます。これを用いることによって過去のissueには目を瞑り、これからの実装で発生するissueに集中することができます。これによって導入のコストを大幅に軽くすることができます。

phan --save-baseline=.phan/baseline.php

最後に、github actionsでプルリクエストを更新した際、解析対象のPHPファイルが含まれてる場合のみ実行するようにします。

name: phan

on:
  pull_request:
    types: [ opened, reopened, synchronize ]
    paths:
    - 'WORKSPACE/**.php'
    - '!WORKSPACE/config/**.php'
    - '!WORKSPACE/libraries/Api/**.php'
    - '!WORKSPACE/vendor/**.php'

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

jobs:
  phan:
    runs-on: ubuntu-latest
    timeout-minutes: 240
    steps:
    - uses: actions/checkout@v2
    - name: Setup PHP
      uses: shivammathur/setup-php@v2
      with:
        php-version: '8.0.13'
        extensions: ast
    - name: phan install
      run: cd WORKSPACE && composer require --dev "phan/phan:5.x" && cd $OLDPWD
    - name: run phan
      run: WORKSPACE/vendor/bin/phan --load-baseline=.phan/baseline.php

Psalm

まずはcomposerでPsalmをインストールします。

composer require --dev vimeo/psalm

続いてconfigを作成します。

./vendor/bin/psalm --init

errorLevelは一番厳しい1にセットします。

phanと異なり、Psalmの場合はignoreFilesに追加したディレクトリは他の解析対象ファイルでロードできなくなるようだったので、他のファイルから読み込みたいけど、issueは無視したいディレクトリはissueHandlersで個別に対応しています（↓の libraries/Api ）。同じ理屈だとcomposerのライブラリが入っているvendorライブラリも他ファイルから読めなさそうですが、vendorは特別なようです。

<?xml version="1.0"?>
<psalm
    errorLevel="1"
    resolveFromConfigFile="true"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config ../codeigniter/application/vendor/vimeo/psalm/config.xsd"
    errorBaseline="baseline.xml"
>
    <fileExtensions>
        <extension name=".php" />
    </fileExtensions>
    <projectFiles>
        <directory name="WORKSPACE" />
        <ignoreFiles>
            <directory name="WORKSPACE/config" />
            <directory name="WORKSPACE/logs" />
            <directory name="WORKSPACE/vendor" />
        </ignoreFiles>
    </projectFiles>

    <issueHandlers>
        <MixedAssignment>
            <errorLevel type="suppress">
                <directory name="WORKSPACE" />
            </errorLevel>
        </MixedAssignment>
        <MixedArgument>
            <errorLevel type="suppress">
                <directory name="WORKSPACE/libraries/Api" />
            </errorLevel>
        </MixedArgument>
    </issueHandlers>
</psalm>

続いてphanと同様にbaselineを作成します。

vendor/bin/psalm --php-version=8.0.13 --set-baseline=baseline.xml

最後にこちらもgithub actionsのyamlを作成します。

name: Psalm

on:
  pull_request:
    types: [ opened, reopened, synchronize ]
    paths:
    - 'WORKSPACE/**.php'
    - '!WORKSPACE/config/**.php'
    - '!WORKSPACE/libraries/Api/**.php'
    - '!WORKSPACE/vendor/**.php'

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

jobs:
  psalm:
    runs-on: ubuntu-latest
    timeout-minutes: 240
    steps:
    - uses: actions/checkout@v2
    - name: Setup PHP
      uses: shivammathur/setup-php@v2
      with:
        php-version: '8.0.13'
    - name: psalm install
      run: cd WORKSPACE && composer require --dev vimeo/psalm && cd $OLDPWD
    - name: run psalm
      run: WORKSPACE/vendor/bin/psalm --php-version=8.0.13 --set-baseline=baseline.xml # --set-baseline for avoid error

理由は不明なのですが、psalm実行の際に --set-baseline をつけてbaselineを作成しないとエラーが出るので仕方なくつけています。Psalmはよくわかっていない挙動がまだいくつかあります。

運用してみて

数日運用してみましたが、両方とも解析レベルを最大にしてるだけあり、issueが大量に出ます。現在はその中から不要と思われるものを除外し、少しずつプロジェクトに合わせた設定を作っている最中となります。初めのうちはissue多すぎて対応するのにコストがかかりすぎるので、必要に応じてbaselineを作成し直すのがよさそうです。

現状はまだ運用期間が短いのもありあまり恩恵を感じていないのですが、テストのようにリファクタリングや、言語やライブラリのアップデートの際に大きな効果が出ると期待しています。

future work

cybozuさんがやっているbaselineを育てていくやり方を取り入れたいと思っています。解析が成功したらbaselineを更新することでbaselineをどんどん小さくしていけるというもので、これは是非取り入れたいです。

CircleCIで勝手に強くなる静的解析の作り方

まとめ

今回は静的解析をいろいろ触ってみて、実際にphanとPsalmをプロジェクトに導入してみました。

そこまで手がかからずに導入できますし、不具合が出る危険もないので少しでも興味があったらやってみてはいかがでしょうか。

このエントリーをはてなブックマークに追加