なぜ Danger-Swift に移行（しようと）したか
以前の記事にも書いたとおり、弊社はソースコードの品質担保の一環として Danger による PR の機械チェックが導入されています。通常のコードフォーマットが合ってるかどうか、ビルドワーニングが発生してるかどうか以外にも、テストのカバレッジがどれくらいか、（バージョンリリース時に）バージョン番号や Changelog がちゃんと更新されているかどうか、更にブランチの設定が正しいかどうかや、そのブランチ名からチケット番号を抽出して自動で PR ページにチケットリンクへ書き込むなどの機能も Danger で実現させています。割と Danger のヘビーユーザです。
ところが、そんな Danger ですが、もちろん多くの機能はプラグインによって実現されており、例えばその「テストのカバレッジがどれくらいか」というのはまさに Danger-slather というプラグインによって実現されています。
そして最近、社内では純粋 SwiftPM プロジェクトが新しくできています。文字通り純粋 SwiftPM 案件なので、通常の Xcode プロジェクトでお馴染みの .xcodeproj ファイルや Scheme の設定などはありません。しかし残念ながら、Danger で Xcode プロジェクトのテストカバレッジを取得してるこの Danger-slather も danger-xcov も、.xcodeproj ファイルと Scheme の設定に強く依存しています。特に Danger-slather は 4 年以上更新されておらず、danger-xcov も以前導入を試みたときはっきり覚えていませんがなぜかカバレッジをうまく読み取れなかったことが何回かあったため結局断念した記憶があります。
ここで思い出したのが Danger-Swift の存在です。Danger-Swift は Danger と全く同じ目的のものですが、Swift で実装されており、Dangerfile も Ruby ではなく Swift で記述されます。そのため Swift 信者の筆者はその当時から導入したかったのですが、Danger と Danger-Swift はプラグインがお互い互換性が全くなく、そして当時の Danger-Swift プラグインライナップは非常に貧弱だったため断念しました。
ところで今調べ直してみたら、SwiftLint はもちろん、当時まだなかったカバレッジを取得する DangerSwiftCoverage やビルドワーニングを取得する DangerXCodeSummary も揃えられたので、必要なものはとりあえず一通り揃えたと感じて今回の導入に踏み切りました。
移行してみてどうか
まだ移行したばかりなので深いところまでは突っ込めませんが、ひとまず今現在の感想を良かったところと悪かったところを分けて述べます。
良かったところ
Swift で Dangerfile 書けるので、Swift 信者に私にとっては非常にやりやすい気持ちです。
まず Swift には型がちゃんとあるので、その型に応じたプロパティーやメソッドの参照が可能になり、「この属性ってどんな型だっけ？」とか「このデータを取得するにはどこ経由でどう取ればいいんだっけ？」と言った仕様に基づいた使用は Ruby と比べて非常に楽です。何より大抵のことは型で保証してくれてますので。
そして Swift の表現力にも非常に助かります。後述しますが例えば共通の出力フォーマットを定義したいとき、とりあえず struct で定義しとけば、あとで算出プロパティーとかメソッドとかクロージャの力でいろんなことが実現可能です。（もちろんプロパティーとかは Ruby もできますが、何せ型がないのでね…）
最後はやはり使い慣れてる言語で書けるのもとてもいいところですね。正規表現の抽出とか配列の操作とか、これは普段の業務でもいつもやってることなのでどう書けばいいかすぐわかりますから、調べながら動作テストしながら書いていくしかない Ruby と比べると作業効率爆上げです（もちろんそもそも Ruby に慣れてる人でしたら Ruby の方が作業効率が高いでしょう）。
ちなみに Danger-Swift は Dangerfile.swift を編集するための Xcode プロジェクト環境を自動で生成できるので、本体プロジェクトに影響を及ぼさずに Dangerfile の編集が可能です。
悪かったところ
残念ながら Danger-Swift は実は罠もたくさん潜んでありますね。まあ Swift はそもそもスクリプト言語ではないのに無理やりスクリプトとして動かしてるから仕方ないのかもしれません。
まず意外なところですが、実は Danger-Swift は JavaScript 製の Danger ツール Danger-JS に依存しています。まだ具体的に Danger-Swift の実装を見ていないのでどこで何のために使ってるかまでは把握していませんが、ただそのせいで時々 Danger-JS のエラーが吐き出されて何が何だかよくわからないことが稀によくあります。そしてこの Danger-JS のコマンドは Ruby 製の Danger ツールと同じ danger のため、Danger-JS と Danger の共存ができず、すでに Ruby 製の Danger をインストール済みの場合は先にこれをアンインストールしなくてはいけないのです。まあ Swift は所詮スクリプト言語ではなくコンパイル言語なので、それは仕方ないのかもしれません（Danger-Swift の詳しい実装見てないので適当に言ってみただけです）。
次に Danger-JS 抜きにしても、Danger-Swift のエラーも比較的にわかりにくいことがあり、しかも Ruby 製 Danger と比べて情報が圧倒的に少ないので、何か躓いたら割とかなり難航します。
さらには Danger では git にあったいくつかのもの、例えば修正量を抽出する lines_of_code とか、は Danger-Swift の場合なぜか github とかのサービス依存のプロパティーに移動されてしまいました；その上 github プロパティーは IUO ですので、それが当たり前のように存在してる気持ちで Dangerfile.swift 書いてたら、danger-swift local で動作確認すると github が nil になって落ちちゃいます。ちなみにこのエラーもメッセージがわかりにくい（どのプロパティーが nil なのかが教えられていない）ため、私はこれの原因を突き止めるのにかなり時間かかりました。
これらの悪いところで更に次に悪かったところを引き出します：github が nil になっちゃうのに、割と多くのデータ取得が github に依存してしまうため、完全にローカルだけで完結できる動作確認（danger-swift local）が非常に限られており、結局 PR つくってそれに基づいたローカル動作確認（danger-swift pr [PR URL]）が必要になります。
そして機能面でも、実は Ruby 製 Danger より低いところがあります。例えば Danger-Swift は直接特定ファイルの diff を抽出できません、それをやりたい場合は残念ながら自分で Swift から shell コマンドを呼び出す必要があります。（そして残念ながら今のところ私はまだこれを使った機能の動作確認ができていないので、本当にうまくいくかどうかの確証がまだないのです…）
最後にまた後述しますが、良かったところでは Dangerfile.swift を編集するための Xcode プロジェクト環境を自動で生成できると書いてありますが、そのための手順があるのでそれが抜けちゃうと編集が反映されないことがあります。
実際の移行手順
Danger-Swift のインストール
Danger-Swift は基本 CI 側で実行されるものですが、Dangerfile.swift ファイルの正規性を確認するためにもローカルで実行できる方が嬉しいです。
ところでここでいきなり最初の罠にハマります：悪かったところにも書きましたが Ruby 製 Danger との共存ができませんが、タイトルで書いたとおり筆者は「移行」でしたので、元々ローカルには Ruby 製の Danger がインストールされているのです。そのためこのまま brew install danger/tap/danger-swift でインストールしても、「すでに danger がありますよ」的なことが言われてインストール失敗します。そのためまず最初は既存の Ruby 製 Danger をアンインストールする必要があります。
Dangerfile.swift の作成
Danger-Swift がインストールされたら、次は確認スクリプトの Dangerfile.swift を作る必要があります。Ruby 製 Danger の場合は自分で Dangerfile を勝手につくっちゃえばいいから、Danger-Swift もまあ同じ方法でやっても問題はないのですが、実は Danger-Swift は Dangerfile.swift の簡易な構築環境も用意してくれています。danger-swift edit コマンドを叩けば、Dangerfile.swift の Xcode プロジェクトが立ち上がりますので、この時点でもし Dangerfile.swift がまだなかったら自動で最低限のものを生成してくれますし、あったらそれをそのまま使ってくれます。Swift は Playground と main.swift 以外で直接関数の呼び出しができないので、この構築環境は自動でプロジェクト内で main.swift をつくってくれます。また danger-swift edit コマンドは自動終了しないので、編集が終わったら main.swift を保存すればその内容が自動的に Dangerfile.swift に反映されてくれます。そして完全に作業が終わったら danger-swift edit を叩いたターミナルに行って return キーを押せば終了できます。
ただし注意しないといけないのは、この構築環境のプロジェクトは Dangerfile.swift のみに基づいて自動生成されるものなので、このプロジェクト内で main.swift 以外のソースファイルとか追加しても Dangerfile.swift に反映されませんし実行できません。必要なものは全て main.swift 内で完結する必要があります。
また悪かったところにも軽く書いてありますが、その作業が終わったらターミナルだけ終了して Xcode プロジェクトを開いたままにすると、今度は danger-swift edit を叩かずにそのまま開いた状態の Dangerfile プロジェクト編集しちゃっても、それを自動で Dangerfile.swift に反映される術はありません。またその修正された状態で danger-swift edit を叩くと、反映される前の古いコードの Dangerfile.swift の内容が main.swift に適用されちゃいますので、今までの編集が全て失われてしまいます。なのでここでの TIPS としては、Dangerfile プロジェクトは保存すると修正が自動的に Dangerfile.swift に反映されるので、作業が本当に終わるまでは danger-swift edit をそのまま閉じずに動かしてください。そして作業が終わったら必ず Dangerfile プロジェクトを閉じちゃった方が無難です。
プラグインの利用
Danger-Swift はあくまで CI 環境で動かすものであってプロジェクト自体にとって必須ではありません。そのため私は基本的に Danger-Swift を Package.swift とかのパッケージ管理ファイルには追加しない方が好きです。しかしそうすると今度は逆に自分の Dangerfile.swift が依存しているプラグインの管理が難しくなります。
でもご安心ください、幸いなことに Danger-Swift は Marathon（すでに Deprecated にはなってますが）を使って依存解決が可能です。方法としては Dangerfile.swift の import の行の最後に、コメントで // package: PackageURL を書いとけばいいです。例えば DangerSwiftCoverage の導入は、下記のようなコードで書けばいいです：
import DangerSwiftCoverage // package: https://github.com/f-meloni/danger-swift-coverage.git
こうすると、danger-swift edit コマンドを叩くと、Danger-Swift は自動的に DangerSwiftCoverage を落としてきてプロジェクト内で使える状態にしてくれます。非常に便利ですし、ライブラリー配布の場合も Danger による Package.swift の不必要な依存が増える心配もないです。
ちなみにもしそれ書いても No such module のエラーが出るなら、一回 Xcode を終了してもう一回 danger-swift edit やり直してみてください、私の場合はこれで解決しました。
Dangerfile.swift の動作確認
悪かったところにも書いてありますが、残念ながら danger-swift local で確認できる動作は割と限られています。必要最小限の動作確認（例えば修正があるファイルのパスだけ出力してみる確認とか）なら danger-swift local で確認できますが、github に依存している動作確認は必ず danger-swift pr PR_URL で確認する必要があります。
CI で導入
Danger-Swift は CI 側で使うツールなので、最終的に CI に導入しないと意味がありません。
まずは CI での Danger-Swift のインストールですが、これはローカルでのインストールと全く同じです。ただし場合によってはキャッシュに Ruby 製の Danger が残ってる可能性がありますので、Danger-Swift をインストールする前に Ruby 製 Danger をアンインストールするスクリプトを組み込むか、キャッシュを削除して作り直す必要があります（個人的にはキャッシュの作り直しがいいと思います）。
次に Ruby 製 Danger と同じく、GitHub のリポジトリーの場合 DANGER_GITHUB_API_TOKEN の環境変数を設定する必要があります。これは Ruby 製の Danger のものを流用すればいいですし、もしそもそも Ruby 製 Danger を今まで使ったことがなければ、GitHub で Personal access token を新たにつくって CI にそれを設定すればいいです。
プラグインは先ほどにも書いた通り、Marathon 形式で書いていれば Danger-Swift が自動で解決してくれるので、Ruby 製の Danger みたいにわざわざ手動でインストールする必要は特にありません。
そして最後は実行ですが、CI で実行するときのコマンドは danger-swift ci です。
サンプル Dangerfile
当然ながら流石に業務のプロジェクトを見せるわけにはいけませんので、その Dangerfile だけひとまずサンプルとしてあげておきます。プロジェクトに依存している情報は適当に書き換えていますので、自分のニーズに合わせて直せばいいです。Swift コードなので多分読むのはそんなに難しくないはずです。チェックする項目が多いのでクッソ長いですが（時間あったら個人プロジェクトにこれを組み込みますので、終わったらそのプロジェクトもこの記事に入れる予定です）。
import Foundation
import Danger
import DangerXCodeSummary // package: https://github.com/f-meloni/danger-swift-xcodesummary.git
import DangerSwiftCoverage // package: https://github.com/f-meloni/danger-swift-coverage.git
// swiftlint:disable file_length function_body_length
// MARK: - Variables those may change according to each project
let changelogPath = "CHANGELOG.md"
let versionSpecifyingFile = "Sources/Agen/main.swift"
let versionSpecifyingText = "let version = "
// MARK: - A structure to introduce checking result
struct CheckResult {
let title: String
private(set) var warningsCount = 0
private(set) var errorsCount = 0
enum Result {
case good
case acceptable
case rejected
var markdownSymbol: String {
switch self {
case .good:
return ":tada:"
case .acceptable:
return ":thinking:"
case .rejected:
return ":no_good:"
}
}
}
typealias Message = (content: String, result: Result)
private var messages: [Message] = []
private var todos: [String] = []
init(title: String) {
self.title = title
}
mutating func askReviewer(to taskToDo: String) {
todos.append(taskToDo)
}
mutating func check(_ item: String, execution: () -> Result) {
let result = execution()
messages.append((item, result))
switch result {
case .good:
break
case .acceptable:
warningsCount += 1
case .rejected:
errorsCount += 1
}
}
var markdownTitle: String {
"### " + title
}
var markdownMessage: String {
let chartHeader = """
Checking Item | Result
| ---| --- |
"""
let chartContent = messages.map {
"\($0.content) | \($0.result.markdownSymbol)"
} .joined(separator: "\n")
return chartHeader + chartContent
}
var markdownTodos: String {
let todoContent = todos.map {
"- [ ] \($0)"
}
return todoContent.joined(separator: "\n")
}
}
// MARK: - DangerDSL computed properties
extension DangerDSL {
private var headBranch: String {
github.pullRequest.head.ref
}
private var baseBranch: String {
github.pullRequest.base.ref
}
private var additions: Int {
github.pullRequest.additions ?? 0
}
private var deletions: Int {
github.pullRequest.deletions ?? 0
}
private var diffLinesCount: Int {
additions + deletions
}
private func hasModifiedFile(at filepath: String) -> Bool {
git.modifiedFiles.contains(where: { $0 == filepath })
}
var githubIssue: String? {
headBranch.substring(of: #"issue/(\d+)"#, options: .regularExpression)
}
var hasBootstrapSHFileBeenModified: Bool {
hasModifiedFile(at: "bootstrap.sh")
}
var hasBrewfileBeenModified: Bool {
hasModifiedFile(at: "Brewfile")
}
var hasChangelogBeenModified: Bool {
hasModifiedFile(at: changelogPath)
}
var hasMarketingVersionBeenModified: Bool {
let diff = Process.runShell("git diff -- \(versionSpecifyingFile)").components(separatedBy: "\n")
let additions = diff.filter({ $0.hasPrefix("+") })
return additions.contains(where: { $0.contains(versionSpecifyingText) })
}
var isDevelopPR: Bool {
// Treat PRs merging into develop branch as a develop PR
baseBranch == "develop"
}
var isReleasePR: Bool {
// Treat PRs merging into master branch as a release PR
baseBranch == "master"
}
}
// MARK: - DangerDSL PR content check
extension DangerDSL {
enum BranchType {
case master
case develop
case feature
case refactor
case fix
case issue
case version
case ci
static func parsed(from branchName: String) -> BranchType? {
switch branchName {
case "master":
return .master
case "develop":
return .develop
case let feature where feature.hasPrefix("feature/"):
return .feature
case let refactor where refactor.hasPrefix("refactor/"):
return .refactor
case let fix where fix.hasPrefix("fix/"):
return .fix
case let issue where issue.hasPrefix("issue/"):
return .issue
case let version where version.hasPrefix("version/"):
return .version
case let ci where ci.hasPrefix("ci/"):
return .ci
case _:
return nil
}
}
}
func isHeadBranch(_ branchType: BranchType) -> Bool {
BranchType.parsed(from: headBranch) == branchType
}
func isHeadBranch(anyOf branchTypes: [BranchType]) -> Bool {
branchTypes.contains(where: { isHeadBranch($0) })
}
func isBaseBranch(_ branchType: BranchType) -> Bool {
BranchType.parsed(from: baseBranch) == branchType
}
}
// MARK: - DangerDSL modifications content check
extension DangerDSL {
// Auto PR created by CI
func checkCIAutoPRModification(into result: inout CheckResult) {
result.askReviewer(to: "Check if the automatically-created-by-CI content is correct.")
warn("This PR is created by CI automatically. Please check if the content is correct.")
}
func checkDevelopmentModification(into result: inout CheckResult) {
// It's encouraged to edit changelog in a develop PR
let doChangelogModificationCheckTitle = "Changelog Modification Check"
result.check(doChangelogModificationCheckTitle) {
if hasChangelogBeenModified {
return .good
} else {
warn("This PR doesn't contain any modifications to changelog. Please consider if it's necessary to edit it.")
return .acceptable
}
}
}
func checkReleaseModification(into result: inout CheckResult) {
// It's encouraged to check if there's any issue left before releasing a version
result.askReviewer(to: "Check open issues")
warn("This is a release PR. Please check if there's any issue left to be resolved.")
// It's required to change version number.
let doVersionModificationCheckTitle = "Version Modification Check"
result.check(doVersionModificationCheckTitle) {
if hasMarketingVersionBeenModified {
warn("This is a release PR. Please check if the version has been correctly modified.")
return .acceptable
} else {
fail("This is a release PR, but it seems there's no version modification, which is requried.")
return .rejected
}
}
if hasMarketingVersionBeenModified {
result.askReviewer(to: doVersionModificationCheckTitle)
}
// It's strongly encouraged to edit changelog in a release PR.
let doChangelogModificationCheckTitle = "Changelog Modification Check"
result.check(doChangelogModificationCheckTitle) {
if hasChangelogBeenModified {
warn("This is a release PR. Please check if the changelog has been correctly modified.")
return .acceptable
} else {
fail("This is a release PR, but it seems there's no changelog modification, which is strongly encouraged.")
return .rejected
}
}
if hasChangelogBeenModified {
result.askReviewer(to: doChangelogModificationCheckTitle)
}
}
}
// MARK: - DangerDSL PR review flow
extension DangerDSL {
func doDevelopPRCheck() -> CheckResult {
var result = CheckResult(title: "Develop PR Check")
// Develop PR should be created from a branch which begins with either `feature/`、`refactor/` 、`fix/`、`issue/`, `version/` or `ci/`.
let doHeadBranchCheckTitle = "PR Head Branch Check"
let validBranches: [BranchType] = [
.feature,
.refactor,
.fix,
.issue,
.version,
.ci,
]
result.check(doHeadBranchCheckTitle) {
if isHeadBranch(anyOf: validBranches) {
return .good
} else {
fail("Please create a develop PR from either a feature, refactor, fix, issue or version branch.")
return .rejected
}
}
// Develop PR should be created into develop branch
let doBaseBranchCheckTitle = "PR Base Branch Check"
result.check(doBaseBranchCheckTitle) {
if isBaseBranch(.develop) {
return .good
} else {
fail("Please create a develop PR into develop branch.")
return .rejected
}
}
// Develop PR shouldn't contain any merge commits.
let doNoMergeCommitsCheckTitle = "Merge Commits Excluded Check"
result.check(doNoMergeCommitsCheckTitle) {
if github.commits.allSatisfy({ $0.commit.parents == nil }) {
return .good
} else {
fail("Develop PR should not contain any merge commits. Please consider rebasing if needed.")
return .rejected
}
}
// The volume of diff should not be over 1,000 lines.
let doDiffAmountCheckTitle = "Diff Volume Check"
result.check(doDiffAmountCheckTitle) {
if diffLinesCount <= 1000 {
return .good
} else {
warn("There's too much diff. Please consider splitting this PR.")
return .acceptable
}
}
if isHeadBranch(.ci) {
// If the PR is created from the branch created by CI, do the CI auto PR modification check.
checkCIAutoPRModification(into: &result)
} else if isHeadBranch(.version) {
// If the PR is created from a version branch, do the release modification check.
checkReleaseModification(into: &result)
} else {
// Otherwise, do the develop modification check.
checkDevelopmentModification(into: &result)
}
return result
}
func doReleasePRCheck() -> CheckResult {
var result = CheckResult(title: "Release PR Check")
// Release PR should be created from develop branch.
let doHeadBranchCheckTitle = "PR Head Branch Check"
result.check(doHeadBranchCheckTitle) {
if isHeadBranch(.develop) {
return .good
} else {
fail("Please create a release PR from develop branch.")
return .rejected
}
}
// Develop PR should be created into master branch
let doBaseBranchCheckTitle = "PR Base Branch Check"
result.check(doBaseBranchCheckTitle) {
if isBaseBranch(.master) {
return .good
} else {
fail("Please create a release PR into master branch.")
return .rejected
}
}
// Do the release modification check.
checkReleaseModification(into: &result)
return result
}
enum RoutineError: Error {
case failedToFindCheckRoutine
}
func checkPR() throws -> CheckResult {
if isDevelopPR {
return doDevelopPRCheck()
} else if isReleasePR {
return doReleasePRCheck()
} else {
throw RoutineError.failedToFindCheckRoutine
}
}
}
// MARK: - Other convenient extensions
private extension Git {
var diffFiles: [File] {
createdFiles + deletedFiles + modifiedFiles
}
}
private extension XCodeSummary {
convenience init(filePath: String, onlyShowSummaryInDiffFiles: Bool) {
if onlyShowSummaryInDiffFiles {
let diffFiles = Danger().git.diffFiles
self.init(filePath: filePath) { [diffFiles] in
guard let path = $0.file else { return false }
return diffFiles.contains(path)
}
} else {
self.init(filePath: filePath)
}
}
}
private extension String {
func substring <S: StringProtocol> (of string: S, options: CompareOptions) -> String? {
guard let range = range(of: string, options: options) else {
return nil
}
return String(self[range])
}
}
private extension Process {
static func runShell(_ commands: String...) -> String {
let task = Process()
task.launchPath = ProcessInfo().environment["SHELL"]!
task.environment = ProcessInfo().environment
task.arguments = ["-c"] + commands
let pipe = Pipe()
task.standardOutput = pipe
task.launch()
let data = pipe.fileHandleForReading.readDataToEndOfFile()
let output = String(data: data, encoding: .utf8)!
return output
}
}
// MARK: - Check routine
let danger = Danger()
// If there's any modification in bootstrap.sh file, ask the reviewer to check if he needs to update Bitrise workflows.
if danger.hasBrewfileBeenModified {
markdown("- [ ] Check bitrise workflow")
warn("There's modification in bootstrap.sh file. Please remember to check if needed to update Bitrise workflow.")
}
// If the head branch refers to a GitHub issue, comment it in the PR page.
if let githubIssue = danger.githubIssue {
message("Resolve #\(githubIssue)")
}
// SwiftLint format check.
SwiftLint.lint(.modifiedAndCreatedFiles(directory: nil), inline: true)
// Xcode summary warnings check.
XCodeSummary(filePath: "result.json", onlyShowSummaryInDiffFiles: true).report()
// Xcode test coverage check.
Coverage.spmCoverage(minimumCoverage: 60)
// PR routine check.
do {
let result = try danger.checkPR()
markdown(result.markdownTitle)
if !result.markdownMessage.isEmpty {
markdown(result.markdownMessage)
}
if !result.markdownTodos.isEmpty {
markdown(result.markdownTodos)
}
if result.warningsCount == 0 && result.errorsCount == 0 {
message("Well Done :white_flower:")
}
} catch {
fail("Failed to find out the correct check routine. Please check if your PR is created from or into a correct branch.")
}