Danger-JSにAndroidLintを動かすプラグインがぱっと見なさそうだったので、TSの勉強も兼ねて作ってみました。
github.com
作る中で学んだこととか、考えたことなどをまとめておきます。
Danger
Dangerを全く知らない人は、下記ブログをざっと読むのをお勧めします。
日本語記事もたくさんありますし、GitHubに参考実装もたくさんあるので、読まなくとも動かすならなんとでもなります。Dangerの文脈把握にはブログが一番、ツールは文脈把握が一番、みたいな思想です。
artsy.github.io
なお、日本語ブログだとコネヒトさんのブログがざっくり概要をつかむのに適している気がしました。
tech.connehito.com
実装についてはは、おおよそGitHubやBitbucketへのAPIアクセスをラップし、メッセージや警告をPR上にコメントしてくれるラッパーライブラリと捉えています。
そして、ありがたいことにプラグインにより様々な状況に対応することができます。*1
2つのDanger
さて、そんなDangerですがRubyとJSの2つの実装が存在します。*2
それぞれの比較は下記のページから。
danger.systems
大きな違いというとそこまで違いはないのですが、
Danger on Ruby is more mature, has a solid plugin eco-system and is very stable.
とあるようにRuby版の方が安定しています。
実際に今踏み抜いて困っているのが、Danger-JS依存で発生する「同一ファイルに複数のインラインコメントをつけようとするとエラーが起きる」問題です。
github.com
上記IssueはSwift版のリポジトリに建てられているIssueなのですが、Danger-JSのみを利用しているプロジェクトでも発生しているため、Danger-JS由来の問題の様子です。 pull_request_review_id must be pending エラーの説明が特にないので、色々と困る。
ざっとRubyとJSのコードを見比べてみたのですが、多分きっとおそらく、PRへのコメント時にRuby側が1つのリクエストJSONにまとめている箇所が、JSでは Promise.all しているために発生している……ような……雰囲気です。
再現できる処理は見つかっているので、時間のある時に差し障りのない感じで再現リポジトリ作ってみようかなと思ってます。*3
※再現するリポジトリ作って*4、issueで報告しました*5
そのほか、Ruby版は github.dismiss_out_of_range_messages をDangerfile内で書き込むことで変更がない点へのコメントを控えますが、JS版は--ignoreOutOfDiffComments オプションを実行時に付与するようです。
細か〜な違いが、多分他にもあるはず。
GitHubであれば、PRのDescriptionであったりタグであったり、変更総行数であったりに対する message や warning の独自のルールを策定する場合には Dangerfile (JS版なら dangerfile.js か dangerfile.ts)内で github オブジェクトをこねこねすれば事足ります。
しかし、せっかくPRにコメントの形で指摘が記載できるのであれば、Lintのチェックもしたくなるのが人情です。
「不要なimportが含まれています」とか「変数名はsnake_caseにしてください」なんかを指摘したくないですよね。そんなことは機械に任せたい。
最近忘れがちになるのですが、自分はAndroidエンジニアなのでAndroid Lintを走らせて指摘させたくなりがちです。
ざっと調べてみたところ、RubyのDangerに対応するpluginはある*6ものの、JSのDangerに対応するpluginは見つかりませんでした。
そんなわけで、作っていきます。
Danger-JSのプラグインを作成することは、npmにJSの(TSの)ライブラリを登録することになります。
package.json に依存関係を書くことで、プラグインを導入し実行するイメージです。
以下、やりたいことを列挙してみます
danger.js (danger.ts)で1~2行書けばAndroid Lintの実行とインラインコメントがなされるようにしたい
- 最初は
app の決め打ちで良さそうだが、変更できるようにしておいた方がよさそう
- 最初はインラインコメントの決め打ちで良さそうだが、変更できるようにしておいた方がよさそう
- TSでコードは書きたい(型情報をちゃんと持たせたい)
CI上でAndroid Lintを走らせたいということは、CIで gradlew lint を実行すればいいことになります。
幸いNode.jsには execSync メソッドによりコマンドを実行できるので、execSync('gradlew lint')を動くように出来ればよさそうです。
たまたまメンテナンスをしていた自分のOSSライブラリで適当に実行してみると、ファイルパスに問題があることがわかってきます。
Run yarn danger ci --ignoreOutOfDiffComments
yarn danger ci --ignoreOutOfDiffComments
shell: /bin/bash -e {0}
env:
GITHUB_TOKEN: ***
yarn run v1.22.5
$ /home/runner/work/EmptyRecyclerView/EmptyRecyclerView/node_modules/.bin/danger ci --ignoreOutOfDiffComments
/bin/sh: 1: gradlew: not found
Error: Error: Command failed: gradlew lint --no-deamon
/bin/sh: 1: gradlew: not found
at checkExecSyncError (child_process.js:616:11)
at Object.execSync (child_process.js:652:15)
at Object.<anonymous> (/home/runner/work/EmptyRecyclerView/EmptyRecyclerView/node_modules/dangerjs-android-plugin/dist/index.js:35:29)
at Generator.next (<anonymous>)
at /home/runner/work/EmptyRecyclerView/EmptyRecyclerView/node_modules/dangerjs-android-plugin/dist/index.js:8:71
at new Promise (<anonymous>)
at __awaiter (/home/runner/work/EmptyRecyclerView/EmptyRecyclerView/node_modules/dangerjs-android-plugin/dist/index.js:4:12)
at Object.androidlint (/home/runner/work/EmptyRecyclerView/EmptyRecyclerView/node_modules/dangerjs-android-plugin/dist/index.js:26:12)
at Object.<anonymous> (dangerfile.ts:4:27)
at Module._compile (internal/modules/cjs/loader.js:1075:30) {
status: 127,
signal: null,
output: [
null,
<Buffer >,
<Buffer 2f 62 69 6e 2f 73 68 3a 20 31 3a 20 67 72 61 64 6c 65 77 3a 20 6e 6f 74 20 66 6f 75 6e 64 0a>
],
pid: 2969,
stdout: <Buffer >,
stderr: <Buffer 2f 62 69 6e 2f 73 68 3a 20 31 3a 20 67 72 61 64 6c 65 77 3a 20 6e 6f 74 20 66 6f 75 6e 64 0a>
}
Failing the build, there is 1 fail.
Feedback: https://github.com/koji-1009/EmptyRecyclerView/pull/4#issuecomment-687710583
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
*7
調べてみると process.cwd() により実行中のディレクトリが取得できるとのことなので、 const dir = process.cwd() と置いてpathを修正することで、TSからGradleを実行することができました。
Android Lintの実行結果をTSで扱う
Android LintのはHTMLやxmlで出力されます。
このため、xmlファイルをJSONとして扱う(TSのコード内の文脈でJSONと書くのが正しいのか……?)方法を探してみると、いくつかのライブラリがあることがわかってきました。今回は、StackOverflowなどでお勧めされていた xml2js を利用してみます。
www.npmjs.com
LintResult としてパースしたい型を定義し、先ほどの dir にAndroid Lintがxmlを出力する先の path を組み合わせてあげます。
これでAndroid Lintの結果を扱えるようになりました。
const lintRaw = readFileSync(`${dir}/${path}`, 'utf-8')
const json = await parseStringPromise(lintRaw, { mergeAttrs: true })
const lintResult = json as LintResult
与えた型はこんな感じです。
interface LintResult {
format: string
by: string
issues: Issues
}
interface Issues {
issue: [Issue]
}
interface Issue {
id: [string]
severity: [string]
message: [string]
category: [string]
priority: [string]
summary: [string]
explanation: [string]
includedVariants: [string]
excludedVariants: [string]
url: [string]
urls: [string]
errorLine1: [string]
errorLine2: [string]
location: [Location]
}
interface Location {
file: [string]
line: [string]
column: [string]
}
なお xml2jsの利用時に注意が必要なのは、下記2点です。
mergeAttrs: true を設定する
xml2js でパースを行うと $ によって階層が一つ深くなるようにパースされるため、 mergeAttrs オプションを有効にします- デフォルトの文字を変えることもできるようなのですが、階層を浅くした方が扱いやすいのではないかなと思います
- 全ての要素を配列として扱う
xml2js でパースを行うと、すべての要素が配列としてパースされるのですが、これをうかつに解除すると事故がおきます
explicitArray: false をする時には計画的に
- xmlをJSONにパースする際、xmlの構造からは「その子要素が単数か複数か」を判断することができないためです
- xmlの子要素が1つの時には単数に、複数の時には配列に、与えるxmlの状態に応じて型がおかしくならないように
- 型を受け取ってパースしてくれるライブラリがあれば、もっとスムーズなのかも
最後にDangerの message warn fail メソッドを呼び出して、PRにコメントをつけていきます。
なお fail を呼び出すとPRのstatusがfailになるので、注意が必要です。
Android Lintはコード全体のLintをかけるので、出力する先が今回の変更範囲内かどうか(少なくとも同一ファイルかどうか)をチェックしつつ、コメントをつけてみます。
const editFiles = danger.git.modified_files.filter(element => !danger.git.deleted_files.includes(element))
const createFiles = danger.git.created_files
const files = [...editFiles, ...createFiles]
for (const issue of lintResult.issues.issue) {
const location = issue.location[0]
const filename = location.file[0].replace(`${dir}/`, '')
if (!files.includes(filename)) {
continue
}
const line = parseInt(location[0]['line'] ?? '0')
send(issue.severity[0], issue.message[0], filename, line)
}
function send(severity: string, messageText: string, file: string, line: number) {
switch (severity) {
case Severity.WARNING:
warn(messageText, file, line)
break
case Severity.INFORMATIONAL:
message(messageText, file, line)
break
case Severity.FATAL:
case Severity.ERROR:
warn(messageText, file, line)
break
default:
}
}
これでメッセージの送信周りが実装できました。
なお今見てたら直したい箇所ができてきたので、このコードは執筆当時のものになる恐れがあります。
Pluginとして登録
npmにアカウントを作成し、yarn publish あたりでアップロードしましょう。
なお、ここで yarn build してから yarn publish することを失念し、数バージョンコードの更新がされるずにアップロードされてしまう問題にハマりました。
慣れてないツールは手間取りますね。
www.npmjs.com
あとは danger typescript と共に dangerjs-android-plugin を依存関係に加え、GitHub Actionsなどの任意のCIでDanger-JSを実行すれば、Android Lintが走ってくれるようになります。
デフォルト設定であれば dangerfile.ts は下記のようにしてください。
なお、この辺りも改善の余地しかないので、しばらくすると更新されREADMEをアップデートすると思います。
import { androidlint } from 'dangerjs-android-plugin'
androidlint()
お疲れ様でした。
最後に私見です。
現状では「RubyのDangerを使うべき」ではないかなと思っています。
理由としては、下記2点です。
- JSよりもRubyの方が既存のビルドシステムに入っていることが多い
- Ruby版の方が 安定 している
- JS版を使うことで、新たな問題を踏み抜くことがあります
- 踏み抜いたら報告し、修正のPRを作成すればいいのですが、AndroidデベロッパーがJSの解析をするのは時間がかかります
- まだまだ発展途中なため、PRのレビュー高速化のための試作で試行錯誤の時間が大きくなるのは避けたいかなと感じます
- 導入事例などの日本語ドキュメントも豊富
一方で、JS版を試していくと下記のようなメリットがあると感じています。
- 楽しい
- 未舗装の分野を進んでいる感じがするので、そもそも楽しい
- TSで型のサポートが入るため、Rubyよりも馴染みやすい
- 知識の移植性が高い
- TSをshell scriptとして利用することができるので、他の箇所や分野でも利用できそうな気になってきます
- 様々な環境をサポートしやすい(ブログの思想より)ので、今後の活用の幅が広そうに感じています
- nodeを利用したツールがある場合に統合できる
- apollo-androidを意識しています
- GraphQLは採用事例が増えてくると思われるので、firebase-toolsとの組み合わせでなんとかするとスッキリしそうです
終わりに
8月半ばからいろんなものと並行して進めていたものが、ベータ版にまで進んだのでよかったです。
当初は「fastlaneのFirebase App Distribution Pluginにfirebase-toolsにnodeが必須だし、PRレビューをnode依存で完結されられたら、色々と効率的なのでは？」ぐらいの思いつきで始めていたので、v0.2.0でfirebse-toolsの依存が不要になった時にはずっこけそうになりました。
とりあえず動くところまで来た、ぐらいの感じなのですが、チラッと見たnpmのダウンロードがなぜか成長していたので早めに気になるところとか直していこうと思います。
せっかく家にこの本あるし、この機会にちゃんと読んでおこう……。
