理系学生日記

おまえはいつまで学生気分なのか

Checkstyle、SpotBugsのエラー(Violation)を、Reviewdogを使ってGitLabのMergeRequestにコメントする

CheckstyleやSpotBugsのエラーを、Reviewdogを使ってGitLabのMerge Requestにコメントできるようにしました。 それぞれハマりポイントがあって、かなり苦戦しました。このため、それぞれに関する内容を記載しておきます。

Checkstyle

Reviewdog自体はネイティブでCheckstyleのレポートフォーマットに対応しています。

reviewdog also accepts checkstyle XML format as well. If the linter supports checkstyle format as a report format, you can use -f=checkstyle instead of using 'errorformat'.

checkstyle format

一方で、Checkstyleに関するハマりポイントは、Checkstyleのレポートの中のファイルパスが絶対パスであることです。 reviewdogはファイルのパス・行と対応づけてコメントをしてくれるので、reviewdogに認識させるパスはGitLabの認識しているパスと一致させなければなりません。 この一致を図るため、sedで変換させるようにしています。

  script:
    - mvn ${MAVEN_CLI_OPTS} -f app/movie-uploader/pom.xml test
    # Checkstyleのレポートを相対パスに変更
    - sed --in-place "s|${CI_PROJECT_DIR}/||g" app/movie-uploader/target/checkstyle-result.xml
    # CheckStyleのViolationをMerge Requestのディスカッションにコメント
    - >-
      cat app/movie-uploader/target/checkstyle-result.xml 
      | bin/reviewdog -reporter=gitlab-mr-discussion -name="checkstyle" -f checkstyle -tee

これにより、Merge Requestに対して以下のようなコメントが可能になります。

SpotBugs

SpotBugsについてはさまざまな課題があります。

XML形式のエラーレポートが解析しづらい問題

SpotBugsのXML形式のエラーレポートはパースの難易度がかなり高いです。

前提として、Reviewdogがコメントをするために最低限必要とするのは以下の3つという認識です。

  • エラーメッセージ
  • ファイスパス
  • 対象行

SpotBugsのXMLレポートでは、エラーメッセージは//BugInstance/LongMessage、対象行は//BugInstance/SourceLine/@startといった形です。 問題はBugInstance要素とSourceLine要素が1対多関係になり得ることで、シェルスクリプトでこれらを解析するのはかなり厳しい。

SARIF形式という解決策

一方で、SpotBugsはSARIF形式でのレポート出力が可能です。

なお、SARIFというのは静的解析レポートの標準としてOASISが定義したフォーマットです。これから流行りますかね…。

SARIF形式はJSONなので、これをjqで解析すれば良いでしょう。

解決策

以下のように、SARIF形式のSpotBugsレポートをjqで解析してCSV化し、それをReviewdogの-efmオプションで解析するという方法を取りました。 spotbugs-maven-pluginでは、 SARIF形式のレポートはspotbugs:spotbugsゴールでないと出力されません。 このため、Mavenのtestフェーズに当該ゴールを組み込む形を取りました。

.gitlab-ci.yml上でのジョブ定義の一部を抜粋すると以下の通りです。

  script:
    - mvn ${MAVEN_CLI_OPTS} -f app/movie-uploader/pom.xml test
    # SpotBugsのViolationをMerge Requestのディスカッションにコメント
    - >- 
      cat app/movie-uploader/target/spotbugsSarif.json
      | jq -r '.runs[].results[] | [ "`" + .ruleId + "`: " + .message.text, "app/movie-uploader/src/main/java/" + .locations[].physicalLocation.artifactLocation.uri, .locations[].physicalLocation.region.startLine ] | @csv'
      | bin/reviewdog -reporter=gitlab-mr-discussion -efm '"%m","%f",%l' -name="spotbugs" -tee

具体的にSARIF形式をjqで処理すると以下のようになります。これをreviewdogで解析し、GitLabにコメント投稿させています。

$ cat app/movie-uploader/target/spotbugsSarif.json \
| jq -r '.runs[].results[] | [ .message.text, "app/movie-uploader/" + .locations[].physicalLocation.artifactLocation.uri, .locations[].physicalLocation.region.startLine ] | @csv'
"既知の定数の雑な値を見つけた","app/movie-uploader/de/kiririmo/memory/keymapper/QuickTimeCreationTimeKeyMapper.java",75
"ログの潜在的な CRLF インジェクション","app/movie-uploader/de/kiririmo/memory/keymapper/QuickTimeCreationTimeKeyMapper.java",60
"hashCode メソッドを定義して Object.equals() を使用しているクラス","app/movie-uploader/de/kiririmo/memory/keymapper/QuickTimeCreationTimeKeyMapper.java",71

最終形

unittest:
  stage: test
  image: maven:3.8.4-openjdk-17-slim
  variables:
    GIT_STRATEGY: clone
    GIT_DEPTH: 0
    MAVEN_OPTS: "-Dmaven.repo.local=${CI_PROJECT_DIR}/.m2/repository -Duser.language=ja"
    MAVEN_CLI_OPTS: "--batch-mode"
  before_script:
    - curl -sfL https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s ${REVIEWDOG_VERSION}
    - apt-get update && apt-get -y install jq git
  script:
    - mvn ${MAVEN_CLI_OPTS} -f app/movie-uploader/pom.xml test
    # Checkstyleのレポートを相対パスに変更
    - sed --in-place "s|${CI_PROJECT_DIR}/||g" app/movie-uploader/target/checkstyle-result.xml
    # CheckStyleのViolationをMerge Requestのディスカッションにコメント
    - >- 
      cat app/movie-uploader/target/checkstyle-result.xml 
      | bin/reviewdog -reporter=gitlab-mr-discussion -name="checkstyle" -f checkstyle -tee
    # SpotBugsのViolationをMerge Requestのディスカッションにコメント
    - >- 
      cat app/movie-uploader/target/spotbugsSarif.json
      | jq -r '.runs[].results[] | [ "`" + .ruleId + "`: " + .message.text, "app/movie-uploader/src/main/java/" + .locations[].physicalLocation.artifactLocation.uri, .locations[].physicalLocation.region.startLine ] | @csv'
      | bin/reviewdog -reporter=gitlab-mr-discussion -efm '"%m","%f",%l' -name="spotbugs" -tee
  cache:
    key: ${CI_COMMIT_REF_SLUG}
    paths:
      - .m2/repository
  rules:
    - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'
      changes:
        - "app/**/*"