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に関するハマりポイントは、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/**/*"