はじめに
こちらの記事で構築した静的解析の改善紀となります。（3ヶ月後とか言いながら4日後に更新するとは、、、）
ちょうどレビュー環境を整えた期間と同時期に、2つのバックエンドAPIサービスを1つに統合するという開発をしていました。
統合のため廃止されるサービスのModel層はほとんど移植する形で進めていたので、丸ごと差分が発生する形となり、その中でいくつか課題感が顕在化してきました。
改善するのに一番手っ取り早いのはやはり自らが最初に運用してみることですね。
思ったより分量が多くなってしまったので、今回は掲題を①として2つの事例をピックアップして記載していきます。
運用していく中で出てきた課題
①Bakeで生成したPHPDocが静的解析の違反に引っかかる
CakePHPの開発でController,Model,Migrationなど基本的にはBakeを利用して生成しているのですが、そのコマンドで自動生成されたPHPDocが違反に引っかかりました。
例）Entity
例）Migration
改善方針
幾つかの方法があります。
- 生成されたPHPDocを修正していく運用とする
- neonファイルでignoreErrorsの設定をする
- Bakeテンプレートをカスタマイズする
- Bakeバージョンを更新する
1.0. 生成されたPHPDocを修正していく運用とする
これは直感的にもなしとしたいですね。
単に運用の負担になりますし、自動生成のありがたみが薄れてしまいます。
2.0. neonファイルでignoreErrorsの設定をする
こちらも個人的にはあまり好ましい対応とは感じられないです。
このように正規表現を利用してエラーを無視することができますが、設定ファイルをガチャガチャカスタマイズして独自性を持たせていくのは過度に複雑になってしまったり、後から意図を汲み取りづらくなってしまうのでなるべく標準のまま運用していきたいものです。
parameters:
ignoreErrors:
-
message: '#Method [a-zA-Z0-9\\_]+::up() has no return type specified.'
path: root/config/Migration/*.php
3.0. Bakeテンプレートをカスタマイズする
最初はこちらにしようかなと考えていました。
Bakeテンプレートの修正はデフォルトのものをカスタマイズするか、テーマを作成してプラグイン化するかの2択があります。
今回は違反に引っかかる状態のものをテーマとして残していく必要もなかったので前者はありかなと思いました。
BakeはPHPで実装されているTwigというOSSテンプレートエンジンを利用しており、いずれもこちらのテンプレートファイルを修正するという方法で可能です。
ただし、ここで疑問が、、、
CakePHPはリポジトリをみてもわかる通り、PHPStanを導入しています。
そしてBakeというのはReadmeに記載されている通りCakePHPのコード生成機能を提供するプラグインです。
さすがに標準で用意されているPHPStanのルールで引っかかるテンプレートを放置してるハズなくない・・・？🤔（というかBakeプロジェクト自体もPHPStanでした汗）
4.0. Bakeバージョンを更新する
ここで、「もしかしたらバージョン古くて未対応なのかな〜」という気がしてきて調べてみました。
やっぱり対応してくれてるくれてるﾁﾞｬｰﾝ！！（さすがにね）
一方で現環境のBakeのバージョンを見てみると見てみると2.6.1でした。
修正されたコミットはこちらなので、2.7.0のリリースですね。
* be mass assigned. For security purposes, it is advised to set '*' to false
* (or remove it), and explicitly make individual fields accessible as needed.
*
- * @var array
+ * @var array<string, bool>
*/
protected $_accessible = {{ Bake.exportVar(accessible, 1)|raw }};
{% endif %}
リリースノートにも記載がありました。
このリリースではPHPStanのアップデートもしているので、その関係で一気に更新したのかもしれません。
- Fix up templates for generics in docblocks, more precise assoc def. by @dereuromark in #792
- update to PHPStan 1.5 by @LordSimal in #818
4.1. Bake及びMigrationsのバージョンを更新する
対応方針をアップデートしました。
唐突に出てきたMigrationsですが、こちらはPhinxをラップしたCakePHPプラグインです。
Migrationファイルの自動生成はBakeではなくMigrationsが用意してくれているコマンドを実行します。
なのでこちらに関しても調べていきましょう！ｳｷｳｷ
対応されていますね！
該当のコミットはこちらです。
final class $className extends $baseClassName
{
- public function up()
+ public function up(): void
{
}
- public function down()
+ public function down(): void
{
}
リリースタグは0.13.4でした。
- Restored migration template method return types by @Bilge in #2160
Migrationsには、3.7.0のリリースでPhinxのバージョンアップがされていました。
ということで対応バージョンは確認とれたものの、BakeとPhinxをなるべく上げてしまいたいということでサラッと依存関係を確認してあとは更新していこうと思います。
ちなみに依存関係の調査は普段頻繁にしないので調べてみると、composerのwhy-notコマンドが便利でした。
# composer why-not cakephp/migrations 3.7.0
cakephp/migrations 3.7.0 requires robmorgan/phinx (^0.13.2)
cakephp/app - does not require robmorgan/phinx (but 0.12.10 is installed)
②重複して同じコメントがされる
CIのtrigger eventを以下のように設定していました。
on:
pull_request:
types:
- opened
- synchronize
- reopened
ここでsynchronizeを設定していることで、同一のコメントが何度も付けられてしまいました。
何回かpushした後にまとめて解析のエラーを修正しようとしている場合に発生します。
クリティカルな問題では無いのですが、PRを汚してしまうのがどうしても気になってしまうのでなんとかできないかな〜と思っていました。
GitHubAPIでpull request review commentsというものがあるので、これでPRのコメント一覧を取得して突き合わせをしてあげればできるかなと考えていると、まさにやりたいことをしてくれるようなSticky Pull Request CommentというActionを見つけました！
今回はこちらを試してみようと思います💡
中身の仕組みとしては一緒で、GitHub APIを利用してコメントを取得し、headerとbodyで更新するか判断しているのかな？と思います。
PHPStan解析結果のコメントは行数の情報など、追加pushによって変動する部分が無いのでこれであれば固定のものとしてちゃんと判断してくれそうです！
まとめ
今回は①は基本的な型のエラー検知だったものの、利用しているフレームワークや主要なライブラリと静的解析のツールが同じであればバージョンアップで対応できるという恩恵がありますね。デファクトスタンダート、もしくはそれに準ずるものを利用するメリットの一つだと思います。同じ静的開発ツールを利用しているという前提があれば、次回からは初手で4の対応策を検討することができそうです。
そしてそれはアプリケーション開発においてサードパーティを利用することの恩恵でもあるので、開発しなければいけない部分に集中するためにもフレームワークなりライブラリの機能を最大限に使い倒していきたいと思います。
この育てていくような感覚は運用していく中での醍醐味ですね★
LiB Consultingに所属するエンジニア・デザイナーの個人記事を集めています。 記事内容はあくまでも個人の見解であり、会社としてレビュー等はしておりません。あらかじめご了承ください。
Discussion