PHPStanというツールを知りませんでした
こんにちは、無資格・無免許エンジニアの三木です。
表題の通り、PHPStan(Larastan)という存在を見逃しており
圧倒的無免許の悲しみに打ちひしがれています。
タイトルは悔しさを表しているだけで、見逃した理由に言及する記事ではありません。
クイックの標準的な開発環境として以下を定めています。
バージョンは基本的に全てlatestです
| 名称 | 用途 | 備考 |
|---|---|---|
| Laravel(PHP) | FW | PHPはstrict_types=1で利用 |
| Inertia.js | - | |
| Pint | PHP整形ツール | php-cs-fixerのラッパー |
| React | フロント全般 | |
| TypeScript | - | |
| MUI | - | |
| Vite | ビルドツール | |
| Lighthouse | GraphQL利用 | |
| GraphQL Codegen | GraphQLのHooks自動出力 | |
| ESlint | TypeScriptの静的解析ツール | |
| Prettier | TypeScript整形ツール | |
| lefthook | 変更したファイルのみ上記適用 | 前はlint-stagedを利用 |
| husky | pre-commit時にlefthookを起動 |
これでなければいけないというものではなく
プロジェクトで特に要件がないときはこちらを…というものです。
vueのプロジェクトも、根本から全く違う環境のプロジェクトもあります。
方針としては
- 共通のFWとしてLaravelを利用し、Inertiaでフロントと接合
- フロントはReact+TypeScriptを主として、ESLint+Prettierで静的解析+自動整形
- バックはPHPを主としてPintで自動整形
- GraphQLやMUIは必要に応じて
- 以上の処理をlefthookでstagedのファイルだけに実施
- huskyにて、上記をパスしたもののみがコミットされる
この恩恵として
- レビューコストを大きく下げる
- コードの書き方のように正解が決めにくいものの議論を生まない
というところでしょうか。
そしてお気づきでしょうか。
ここまでやって
「あー、PHPも静的解析あったら良いのになー」と思ってちょっと調べたにも関わらず
PHPStanのようなPHPの静的解析ツールが入っていません。
あまりの失態… もしかしたら寝てたのかもしれません。
失意と悲しみのあまりQiitaを開き記事を書いています。
…
いや、今から目覚めれば良いですね。
本日から覚醒した無資格・無免許エンジニアとして頑張ります。
PHPStan(Larastan)
PHPに対する静的解析ツールで、型の整合性のチェック、nullアクセスの可能性、デッドコードの検出などを行ってくれます。
ただしPHPStanをLaravelのプロジェクトに適用するとEloquentBuilder関連などの記述で大量のエラーが発生してしまいます。
これらを吸収してくれているラッパーがPHPStan公式でUnofficial extensionsとして紹介されているLarastanとなります。
導入方法
GitHub記載の通りですが、composerで導入が可能です
composer require --dev larastan/larastan
インストールしたらphpstan.neonをアプリケーションルートに作成します
includes:
- vendor/larastan/larastan/extension.neon
- vendor/nesbot/carbon/extension.neon
parameters:
paths:
- app/
# Level 10 is the highest level
level: 5
# ignoreErrors:
# - '#PHPDoc tag @var#'
#
# excludePaths:
# - ./*/*/FileToBeExcluded.php
paths:に解析したいディレクトリを指定し、0(甘)～10(厳)の範囲でチェックレベルを指定します。
もし除外したいルールやエラーがあればコメントアウトされている部分を有効にして記載します。
実行は下記のコマンドです
vendor/bin/phpstan analyse
なのでlefthookなどに組み込んでおけば、静的解析結果に問題がある場合はコミットを抑止することができます
phpstan:
glob: '*.{php}'
run: |
./app-root/vendor/bin/phpstan analyse {staged_files}
最初にレベルを10などに設定すると大量のエラーが出るかもしれません
ひとまずレベル0から始めて、エラー抑止ができたらレベルを段階的に上げていき
必要に応じてignoreErrorsやexcludePathsを決めていくのが良いかと考えています。
対応記録(随時追記)
段階的にレベルを上げていく過程で対応が必要だった項目を抜粋して記載していきます
level-0 (2024/12/05)
簡単なnull可能性などの修正のみ
level-1 (2024/12/05)
モデルのプロパティアクセスでcreated_atやupdated_atへのマジックメソッドアクセスで発生
Line app/Models/ModelFileName.php
------ --------------------------------------------------------------------------------------
43 Access to an undefined property App\Models\ModelFileName::$created_at.
🪪 property.notFound
💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
マジックメソッドの可否はmigrationを参照して判定してくれているようなのですが
created_atなどのtimestampsによって生成されたプロパティが引っかかってしまうので
ignoreErrors:
# 問題ないマジックメソッドの除外.
- '#Access to an undefined property App\\Models\\(.*)created_at#'
問題ないプロパティアクセスをignoreErrorsで除外
不要なignoreErrorsであれば、指定自体がErrorとして表示される
-------------------------------------------------------------------------------------------------------------------------
Error
-------------------------------------------------------------------------------------------------------------------------
Ignored error pattern #Access to an undefined property App\\Models\\(.*)updated_at# was not matched in reported errors.
-------------------------------------------------------------------------------------------------------------------------
level-2 (2024/12/05)
引き続きマジックメソッド関連の検知をしてしまうので除外範囲を拡大
ignoreErrors:
# 問題ないマジックメソッドの除外.
- '#Access to an undefined property App\\Models\\(.*)#'
- '#Call to an undefined method Illuminate\\Database\\Eloquent\\Model::toPayload#'
その他mixed相当の指定でクラスが不定になっていた箇所にクラスキャストの処理を追加
level-3 (2024/12/05、2024/12/08追記)
should return *** but returns Illuminate\Database\Eloquent\Model|nullの対応で
マジックメソッド経由のモデル取得箇所に@varを明記
/** @var TrnDivision */
return TrnDivision::query()
->with($with)
->alive()
->findOrFail($id);
不足しているPHPDocを修正
/**
* The attributes that are mass assignable.
*
* @var list<string> <-- @var array
*/
protected $fillable = [
'name',
'email',
'password',
];
level-4 (2024/12/05)
記法による意図しないアルゴリズムのミスや、未使用メンバーの指摘
基本利用でもここまでは上げておきたいと思いました。
level-5 (2024/12/05、2024/12/09修正)
Collectionクラスのeachやmapで型違反が出る
Eloquent経由などでCollectionを取得した際に@varで型を明示する
/** @var Collection<TrnUser> $trnUserList */
$trnUserList = TrnUser::query()->get();
$trnUserList->each(function (TrnUser $trnUser) use ($instance) {
level-6にしたときにCollectionの@varでspecify its types: TKey, TValueが出てしまうため下記でも良い
/** @var Collection<int, TrnUser> $trnUserList */
$trnUserList = TrnUser::query()->get();
$trnUserList->each(function (TrnUser $trnUser) use ($instance) {
アロー演算子を経由している場合
Modelで受けて@varする方法と
->where('notification_type', ENotificationType::SLACK_CHANNEL->value)
->each(function (Model $trnProjectNotification) use ($message) {
/** @var TrnProjectNotification $trnProjectNotification */
$this->slackApiPostMessage($trnProjectNotification->notification_value, $message);
});
一旦each前に型明示した変数へ格納する方法
/** @var Collection<int, TrnProjectNotification> $notificationList */
$notificationList = TrnProjectNotificationService::listByTrnProjectId($targetProjectIdList)
// 通知タイプがSlackのものを抽出.
->where('notification_type', ENotificationType::SLACK_CHANNEL->value);
// それぞれの通知.
$notificationList->each(function (TrnProjectNotification $trnProjectNotification) use ($message) {
$this->slackApiPostMessage($trnProjectNotification->notification_value, $message);
});
構成を壊さないのは前者か。
アロー演算子の間に@varは…入らないか
PHPDoc上のgenericsにちゃんと対応させる必要がある
ModelやFactoryなど、Laravel側で@templateが宣言されているクラスを継承したら
* @extends ModelsBase<TrnFacility>
という形で宣言を追加したり、use節やHasManyやHasOneにもgenericsの設定がいる。
/** @use HasFactory<TrnFacilityFactory> */
use HasFactory;
/**
* リレーション:TrnFacilityTag
*
* @return HasMany<TrnFacilityTag, covariant TrnFacility>
*/
public function TrnFacilityTag(): HasMany
引数や戻り値の型宣言がなければ検出してくれるので有効にしたいが
チームの習熟感と相談しないと開発効率を落としかねない。
level-6 (2024/12/05、2024/12/09修正)
PHPDoc上で不明瞭なarrayが許されなくなるので、内容を指定するかmixedとする
mixedは抵抗あるが利用するライブラリ側でこの形式で返してくることもあるのであまり気にしないのが吉か
* @return array{bool, string}
* @return array<mixed>
level-7 (2024/12/06、2024/12/09修正)
genericsの指定がより厳密になる
また、変数型がstring|falseなど特定できない場合などに関数に投げられなくなるので修正
Requestからの取得をintにcastできないのは無理があるので下記のignoreを追加
- '#function array_merge expects array, (.*) given#'
- identifier: cast.int
call_user_funcなどを利用する際に可変関数はcallbleを確定させないとエラーが出るのでチェックを追加
$function = "$model::truncate";
if (is_callable($function)) {
call_user_func($function);
}
level-8 (2024/12/08)
オブジェクトの型がCollection|nullなどのケースで
十分なnullチェックがない箇所を修正
level-9 (2024/12/08)
意図的な部分に/** @phpstan-ignore-next-line */のコメントを追加
標準関数の除外を追加、キャストの許可を追加、withパラメータはクロージャーとstringいずれも受け入れるものの、都度genericsの指定をDocに書くのを避けたいので除外に追加
- '#function call_user_func expects callable\(\): mixed, (.*) given#'
- '#function array_merge expects array, (.*) given#'
- '#function array_map expects \(callable\(mixed\): mixed\)\|null, (.*) given#'
- identifier: cast.int
- identifier: cast.string
- identifier: cast.double
- '#Illuminate\\Database\\Eloquent\\Builder<Illuminate\\Database\\Eloquent\\Model>::with\(\) expects (.*) given#'
処理を修正するより、PHPDocの整合性を保つ作業の方が多い
level-10 (2024/12/08)
型の限定不足の対応、Laravelに準拠する箇所は@phpstan-ignore-next-lineの対応が多くなる
level-10 から level-5 へ (2024/12/08)
高いlevelの対応はPHPDocと@varの対応が多く
これをチームで必須としてコミット抑制まですると生産性を落としそう
…ということでlevelを5に戻すと
/** @phpstan-ignore-next-line */で除外した箇所がエラーでなくなるので
下記のエラーが発生してしまう。
116 No error to ignore is reported on line 116.
これだと開発チームの運用で気軽にlevelを変更することができない。
メンバーはlevel-5で開発し、不定期にリードがlevel-10まで確認…という運用はできない or /** @phpstan-ignore-next-line */は使わないという決まりが必要そう。
チームの力量やプロダクトの目標などによってちょうど良いレベル設定が必要そうです。
最後に
静的解析によるコミット抑止はメンバーの流動性が高い現場ほどバグ防止の効果が高く
またレビューコストを大幅に下げられると思います。
皆様の何かの参考になれれば幸いです。
もっと早く気づいておきたかった…