LoginSignup
32
4

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

株式会社カオナビAdvent Calendar 2024

Day 1
@uhyoin株式会社カオナビ

reviewdogの謎のエラーを乗り越えてESLintのCIに導入した話

Last updated at Posted at 2024-11-30

皆さんこんにちは。この記事は株式会社カオナビ Advent Calendar 2024の1日目の記事です。このアドベントカレンダーでは、筆者が所属する株式会社カオナビの有志が記事を投稿していく予定です。

筆者はフロントエンドの開発環境改善を含むさまざまな改善を行っていますが、今回はESLintのCIにreviewdogを導入した話をご紹介します。

また、途中でreviewdogが発するexit status: 128 という謎のエラーに苦しめられたので、その原因と解決策もお伝えします。

ESLintのルールに対する継続的な改善とreviewdog

コーディングルールの運用やコード品質の向上に際してESLintのようなリンターは強力なツールです。レビュー等で人間がチェックする手間を省いてくれます。

コーディングルールの改善に合わせて、継続的にESLintのルール設定もアップデートしていくことになりますが、その際に問題となるのが既存コードに対する対応です。新たなlintルールを追加する場合、ルールを満たさない既存コードをどうするのかという問題が発生します。何も対応しなければCIが落ちてしまうため、lintルールをアップデートできません。

対応の方針としては、おおよそ3つ考えられます。

  1. lintエラーになる既存コードを全部修正する
  2. eslint-interactiveのようなツールを利用して全ての既存lintエラーに対してeslint-disable-next-lineコメントを一括で追加する
  3. 既存コードのエラーは許容し、マージリクエスト1で変更が加わったところだけCIで落ちるようにする

筆者は最初2で進めていましたが（eslint-interactiveにバグ報告した際はすみやかに対応いただきました。ありがとうございました）、あまりにeslint-disable-next-lineコメントが増えてしまってつらかったので方針を3に変えました。3の方針についてもう少し説明します。

ESLintのルールを2つに区分する

この方法では、ESLintのルール設定を「メインのルールセット」と「移行期間のルールセット」に区分し、前者をerror、後者をwarningとします。

そして、errorはコード全体のどこかで発生していたらCIが落ちるようにする一方で、warnはMRのdiff内で発生していたらエラー、それ以外のコードではOKという扱いにします。

これにより、最初はコードベース内にwarningが残った状態になりますが、基本的には新規にwarningが増えることは無くなります。また、既存コードを修正する場合にwarning箇所が修正対象になる場合は、一緒に直してもらうことにします。

筆者の考えでは、問題のあるコードが新規に増えないことが一番重要だと考えており、コード内から違反箇所が1つ残らず無くなることはそこまで重要視していません（万一コピペされても新規コードであればCIが通りませんしね）。そのため、この方法に満足しています。

既存コードでもエディタ上ではwarningを表す黄色い表示がされるので、ちょっとだけ既存コードの修正を促す効果もあります。

そして、この仕組みを実践するために用いた道具がreviewdogです。

reviewdogをESLintと組み合わせる

reviewdogはいくつかの機能を持っていますが、今回使いたかったのはdiff内に含まれるlintエラーのみを抽出する機能です。他にも、GitHubやGitLabのAPIを叩いていい感じにlintエラーを報告する機能などもありますが、今回はそれは使いません。既存の運用ではCIが落ちたときはCIのログを見て原因を確認するようになっていたので、それは維持します。

reviewdogは様々なリンターが出力するエラーフォーマットをパースする能力を持っており、ESLintの出力（ESLintのデフォルトのstylishフォーマッター）にも対応しています。

そのため、ESLintの出力をそのままreviewdogに食わせればMRのdiffでフィルタリングしてくれます。

まず、ESLintの出力をテキストファイルに保存します。

yarn lint > .output-eslint.txt

次に、このように.reviewdog.ymlを設定します。cat .output-eslint.txtの結果をeslintの結果として読み込むという意味です。

runner:
  eslint:
    # 事前に yarn lint でESLintを実行してある想定
    cmd: 'cat .output-eslint.txt'
    format: 'eslint'
    level: 'warning'

そして、reviewdogの実行はこのようにします。

reviewdog -diff="git diff $CI_MERGE_REQUEST_DIFF_BASE_SHA" -runners=eslint -filter-mode=diff_context -fail-level=warning

どのコミットとのdiffを取ればいいかはGitLabが環境変数で与えてくれるので、それをもとに-diffでdiffを出力するコマンドを与えています。diff内にワーニングがあった場合、次のように出力されます。

time=2024-11-08T10:32:11.672+09:00 level=INFO msg="reviewdog: [start] runner=eslint"
time=2024-11-08T10:32:11.709+09:00 level=INFO msg="reviewdog: [finish] runner=eslint"
reviewdog: found at least one issue with severity greater than or equal to the given level: warning
path/to/xxx.tsx:73:7: [eslint] Do not use any type assertions  @typescript-eslint/consistent-type-assertions

エラーとワーニングの振り分け

実際のCIでは、ESLintを実行してコード全体のerrorを報告するジョブと、reviewdogを実行してdiff内のwarningを報告するジョブが分かれています。前者から後者へ.output-eslint.txtをアーティファクトとして転送します。

そうなると、全体のerrorを報告するジョブでは、warningの報告は省略したくなります。前述のとおりCIが落ちた場合は開発者がCIのログを見に行ってlintエラーの内容を確認する運用になっていますので、errorとwarningが入り混じったログの中から本当に修正すべきerrorの内容を探すのは、特にwarningが増えるこの運用では骨が折れます。

そこで、全体のerrorを報告するジョブでは、warningを省略してerrorのみを抽出してCIのログに吐き出したいです。これはいくつかの方法が考えられますが、今回は簡単にすませる方針で、シェルスクリプトで.output-eslint.txtの結果を加工することにしました。筆者はシェルスクリプトが得意ではありませんが、弊社では業務にも活用できるAIチャットボット「ナービィ」がいますので、「awkかsedを使ってこういう加工をするスクリプトを書いて」とお願いすれば何往復かの会話で意図通りのものができました。

これが実際のスクリプトです。細かい説明は省きますが、ESLintの出力からwarningを省いて出力するようになっています。

if ! yarn lint >.output-eslint.txt; then
  cat .output-eslint.txt | awk 'BEGIN {file=""} 
    /^\// {file=$0}
    /^  / && /error/ && !/warning/ {
      if (file != last_file) {
          if (NR > 1) print ""
          print file
          last_file = file
      }
      print $0
    }'
  exit 1
else
  echo "ESLintでエラーはありませんでした。"
  echo "warning一覧は.output-eslint.txtを参照してください。"
fi

以上がESLintとreviewdogで差分に対するlintを行う方法の全容です。同じことをやりたい場合は参考にしてください。

ポイントは、ESLintの実行は1回だけで、errorの報告とwarningの報告のジョブで.output-eslint.txtを共有していることです。これは、ESLintの実行が遅い（typed lintingを活用しているため）ので複数回実行したくないからです。

reviewdogの謎のエラー exit status: 128

この仕組みを導入した当初は、うまく動きませんでした。正確には、動くときもあったのですが、変な落ち方をする場合がありました。実際には問題ないのに、reviewdogのジョブが失敗してしまうのです。

変な落ち方をする場合、reviewdogは次のようなログを出力していました。

time=2024-10-22T16:08:59.287+09:00 level=INFO msg="reviewdog: [start] runner=eslint"
time=2024-10-22T16:08:59.319+09:00 level=INFO msg="reviewdog: [finish] runner=eslint"
reviewdog: exit status 128

reviewdog: exit status 128 というログしか出ていません。この原因究明に少し苦労したので、この記事の後半ではこのエラーの原因と対処法を説明します。

エラー発生源の考察

このエラー文からは「終了コードが128だ」ということは分かりますが、主語がなくてよく分かりません。

ちなみに、reviewdogに-log-level=debugを渡して実行したら何か情報が増えないかと思って最初に試してみましたが、何も増えませんでした。

ただ、推察できることとして、reviewdogが内部的に呼び出した何らかのプロセスが終了コード128で終わったのだろうということは分かります。reviewdog自体が終了コード128で終了したとしてもこのログの出方にはならないからです。

また、「reviewdog exit status 128」で検索するとこのようなディスカッションも出てきます。

ただ、エラーメッセージがreviewdog: failed to get merge-base commit: exit status 128となっておりこの記事のものとは違います。

他にも以下のような記事が出てきますが、全てエラーメッセージが違うので、この記事とは微妙に状況が違うようです。

「gitのsafe.directoryの件」は結局根本原因がこれではないかと疑って調べもしましたが、違うようでした。

ただ、上記の記事等から察せられることとして、「gitコマンドを実行したがエラーが発生して終了コード128で終了した」点が共通しているように思われます。gitは終了コード128で終了する癖があるようです。

ということで、今回もreviewdogがgitコマンドを呼び出して失敗したのではないかと推測できます。ここでもう一度今回reviewdogを走らせているスクリプトを見てみましょう。

:eyes:

reviewdog -diff="git diff $CI_MERGE_REQUEST_DIFF_BASE_SHA" -runners=eslint -filter-mode=diff_context -fail-level=warning

:eye: :eye:

-diff="git diff $CI_MERGE_REQUEST_DIFF_BASE_SHA"

ここが怪しいですね。これは、reviewdogに対して、diffを取得するために実行すべきコマンドを与えるところです。reviewdogがこれを実行して失敗したという仮説が考えられます。

なぜgit diffコマンドが終了したのかですが、$CI_MERGE_REQUEST_DIFF_BASE_SHAが指すコミットが存在しないことです（この環境変数はGitLabが提供してくれるもので、コミットSHAらしき文字列はちゃんと入っていました）。

ということで、試しに存在しないコミットに対してgit diffしてみましょう。

$ git diff aaaaaaaaaaaaaa
fatal: ambiguous argument 'aaaaaaaaaaaaaa': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
$ echo $?
128

確かに終了コード128になります。ということで、存在しないコミットとgit diffしている可能性が高いですね。

結論と対応

ではなぜ$CI_MERGE_REQUEST_DIFF_BASE_SHAに存在しないコミットが入っているのでしょうか。筆者は少し頭を悩ませなければなりませんでしたが、結論は単純でした。ジョブの定義を改めて見直してみるとこう書かれている箇所がありました。

  variables:
    GIT_DEPTH: 10

もうお分かりですね。CI環境ではシャロークローンが行われていたため、$CI_MERGE_REQUEST_DIFF_BASE_SHAが指すコミットがフェッチされていない可能性があったのです2。

これであれば、動くときもあるし動かないときもあるという挙動が説明できます。$CI_MERGE_REQUEST_DIFF_BASE_SHAがシャロークローンの範囲内であればうまく動いていたのです。また、前のジョブの.gitを再利用する可能性もあるためそれで動く場合もあるでしょう。

実際にはこの時点で原因が確定したわけではなく、以上が原因ではないかと当たりをつけてCIジョブを修正してみました。そうすると問題が起きなくなったのでこれが問題だと判断できたわけです。

修正は簡単で、コミットが無かったらgit fetchするだけです。

if ! git cat-file -e $CI_MERGE_REQUEST_DIFF_BASE_SHA^{commit} 2>/dev/null; then
  echo git fetch origin $CI_MERGE_REQUEST_DIFF_BASE_SHA
  git fetch origin $CI_MERGE_REQUEST_DIFF_BASE_SHA
fi

こうするとシャロークローンの利点が消えるので完璧ではないのですが、動くようにはなりました。

そもそも今時はシャロークローンで頑張るよりもパーシャルクローン（ブロブレスツリーなど）にする方が筋が良さそうなので、どうせ直すのであればこの方針で直したいところです。

ちなみに、今回reviewdog: exit status 128というエラーメッセージに情報が少なくて調査に手間取ったので、エラーメッセージの改善を要望するissueも建てておきました。

まとめ

この記事では、ESLintのルールを途中から厳しくするためにreviewdogを活用する方法を紹介しました。

また、シャロークローンが原因でreviewdogがreviewdog: exit status 128という変なエラーで落ちてしまう問題も紹介しました。

  1. 弊社ではGitLabを使っているので、PR（プルリクエスト）ではなくMR（マージリクエスト）です。 ↩

  2. 実は最初に紹介したディスカッションをよく見ると「シャロークローンを消したら直った」というコメントがあるのですが、エラーメッセージが違うことに気を取られて気づいていませんでした。 :innocent: ↩

32
4
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
32
4

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?