Danger-JS + Android Lintの話 - Continuity is The Father of Success

Danger-JSにAndroidLintを動かすプラグインがぱっと見なさそうだったので、TSの勉強も兼ねて作ってみました。

作る中で学んだこととか、考えたことなどをまとめておきます。

Danger
- 2つのDanger
- Android LintとDanger
プラグイン開発
RubyとJSのどちらをAndroidの開発で利用するのが良いのか
終わりに

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 オプションを実行時に付与するようです。細か〜な違いが、多分他にもあるはず。

Android LintとDanger

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でコードは書きたい(型情報をちゃんと持たせたい)
- とりあえず開発時の最新バージョンを使いたい

Android Lintを実行する

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.
##[error]Process completed with exit code 1.

調べてみると 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の状態に応じて型がおかしくならないように
  - 型を受け取ってパースしてくれるライブラリがあれば、もっとスムーズなのかも

Android Lintの結果を出力する

最後に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}/`, '') // 絶対pathで出力されていた場合に、相対pathに変換
    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:
          // nop
    }
}

これでメッセージの送信周りが実装できました。なお今見てたら直したい箇所ができてきたので、このコードは執筆当時のものになる恐れがあります。

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とJSのどちらをAndroidの開発で利用するのが良いのか

最後に私見です。

現状では「RubyのDangerを使うべき」ではないかなと思っています。理由としては、下記2点です。

JSよりもRubyの方が既存のビルドシステムに入っていることが多い
- 主に fastlane を意識しています
- Firebase App Distribution用のプラグインが提供されていたり、Google Play Storeへのアップロードコマンドが存在するなど、すでにRubyを利用している環境であれば、あえてnpmを追加する必要性が薄いと感じます
  - プラグインのv0.2.0よりJS製firebse-toolsが不要になりました
  - Remove plugin's dependency on the Firebase CLI (v0.2.0.pre.4) by lfkellogg · Pull Request #142 · firebase/fastlane-plugin-firebase_app_distribution · GitHub
Ruby版の方が安定している
- JS版を使うことで、新たな問題を踏み抜くことがあります
  - 踏み抜いたら報告し、修正のPRを作成すればいいのですが、Android デベロッパーがJSの解析をするのは時間がかかります
  - まだまだ発展途中なため、PRのレビュー高速化のための試作で試行錯誤の時間が大きくなるのは避けたいかなと感じます
- 導入事例などの日本語ドキュメントも豊富
  - Android開発のコードレビューbotを乗り換えた話 - クックパッド開発者ブログ
  - 検索するとざくざく出てきます

一方で、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のダウンロードがなぜか成長していたので早めに気になるところとか直していこうと思います。

www.oreilly.co.jp

せっかく家にこの本あるし、この機会にちゃんと読んでおこう……。

*1:プラグインの一覧はGitHub - danger/awesome-danger: An awesome list of all things Dangerから確認できます。

*2:https://danger.systemsを見るとSwift/Kotlin/Pythonのものも存在しますが、それぞれ柄が同じDangerを各言語から呼び出せるようになっている様子です。個人的に、この「Kotlin書く人ならKotlinでDSL書きたいでしょ」というサポート思想は好きです。ただ、ある組織内で知見を共有するときにTSで統一した方が共有しやすそうなので、今回TSでプラグインを作ってみています。

*3:会社のブログ文章管理リポジトリで書いたTextLintで再現したので、適当な文章とDangerfile作ればなんとかなるはず。

*4:Create project by koji-1009 · Pull Request #1 · koji-1009/pending_pull_request_review_id · GitHub

*5:Using custom github app doesn't work well · Issue #1054 · danger/danger-js · GitHub

*6:GitHub - loadsmart/danger-android_lint: A Danger plugin for Android Lint

*7:https://github.com/koji-1009/EmptyRecyclerView/runs/1120485095