LINTを見直した話
僕は基本的にLINTが大嫌いで、理由は
- いくつかのLINTはあまり上手く実装されていない
- いくつかのよくあるルールは、プログラミング言語の設計者がわざと導入したルールを制限するものである
という二つが主である。（既に使われているLINTにわざわざ文句は言わないし、開発チームで決めたことには従いますが。）一番ばかばかしいと思うルールの一つに、
- リストの最後のカンマを入れるか入れないか
がある。これなんかは、設計者がわざわざ導入した自由を逆に制限するものであり、しかも見た目が揃う以外の利点がなにもないルールなので、こういうルールを有効にするのは止めたほうが良いと真顔で発言できる。
で、ずっとLINT嫌いの立場を貫いてきたんだけど、実はLINTって便利じゃないかと思ったことがあるので、書いておきたい。
OCLintのCoveredSwitchStatementsDontNeedDefaultを知って、もしかしたらLINTって便利なものなのかもしれないと、不明を恥じたのだった。これは、Objective-Cプログラムのswitchが網羅的な場合にdefaultがあったら警告するもので、つまり無駄な空のdefaultを書かなくて良くなる効果がある。
switch (enumValue) { case SomeCase: ... break; case AnotherCase: ... break; default: // Unreachable NSAssert(false); break; }
みたいなプログラム、皆さん書きますよね。現時点ではdefaultはいらないんだけど、将来にenumのcaseが増えたことを考えると、defaultを書いておいてせめて実行時にはエラーになって欲しい。でもこのdefaultとか明らかに無駄なので、できればソースコードから無くしてしまいたい。
この葛藤から逃れられるのであれば、LINTは使うべきだなあと思ったのでした。
これは、止めて欲しいと思うルールとなにが違うかというと、
- 止めて欲しいのは、ある種のプログラムを書くことを禁止するルール
- 良いと思うのは、コンパイラの賢さが足りないせいで保守的にならざるを得なくてこれまで書けなかったプログラムを、書けるようにするルール
と言える気がする。
なお、OCLintのCoveredSwitchStatementsDontNeedDefaultについて言えば、最近のClangはNS_ENUMとかの新しい構文を使って定義したenumであれば網羅性の検査をしてくれるので、特にOCLintを使う必要はありません。（Cの素朴なenumを書くのを禁止するルールがないかなーと思って眺めてたら見つけました。）
長い文字列をNSDecimalNumberに変換するとおしりが0になる
なにかの金額を入力する画面などで、ユーザーに数字を入力してもらうUIを作ることがある。金額なので、NSDecimalNumberで受けることになる。このとき大きな数字を入力すると、入力とは違う値になることがある。*1
NSDecimalNumber(string: "999999999999999999999999999999999999999").stringValue // => "999999999999999999999999999999999999990"
これよりも長い文字列を入力すると、0が続く。
NSDecimalNumber(string: "9999999999999999999999999999999999999999").stringValue // => "9999999999999999999999999999999999999900"
これは、NSDecimalNumberの内部表現による制限である。NSDecimalNumberは、128bitの符合なし整数と32bitの符合つき整数の組で表現されている。（下の定義では_mantissaと_exponent。）
struct NSDecimal { var _exponent: Int32 var _length: UInt32 var _isNegative: UInt32 var _isCompact: UInt32 var _reserved: UInt32 var _mantissa: (UInt16, UInt16, UInt16, UInt16, UInt16, UInt16, UInt16, UInt16) init() init(_exponent _exponent: Int32, _length _length: UInt32, _isNegative _isNegative: UInt32, _isCompact _isCompact: UInt32, _reserved _reserved: UInt32, _mantissa _mantissa: (UInt16, UInt16, UInt16, UInt16, UInt16, UInt16, UInt16, UInt16)) }
例えば、123.45であれば、12345 * 10^-2として、12345と-2の組で表現する。*2
つまり、_mantissaに対応する値は、128bitで表現できる範囲でなくてはならない。
Rubyで試してみると999999999999999999999999999999999999999を表現するには130bit必要とのことなので、128bitでは足りずに切り捨てられたということがわかる。
> 999999999999999999999999999999999999999.bit_length => 130 > 99999999999999999999999999999999999999.bit_length => 127
つまり、999999999999999999999999999999999999990というのは、
_mantissa=9999999999999999999999999999999999999_exponent=1
になっているはず。
NSDecimalNumber(floatLiteral:)は使ってはいけない
私がSwiftで気に入っている改善として、NSDecimalNumberのインスタンスがちょう簡単に作れることがあります。
let d : NSDecimalNumber = 15.97
すばらしい。Objective Cだとこうなって、辛い。
NSDecimalNumber *d = [NSDecimalNumber decimalNumberWithString:@"15.97"];
このSwiftの機能はFloatLiteralConvertibleというプロトコルを使って実装されていて、floatのリテラルの型がFloatLiteralConvertibleなクラス（構造体もできる？）のときに、適当なイニシャライザを呼んで変換してくれるというものです。
init(floatLiteral: Double)
わーかっこいい。かっこいいのですが、ところがしかし型をよく見ると一回Doubleが作られています。NSDecimalNumberをDoubleから作るのは安全なのかと聞かれて、一瞬悩んでしまったのでした。とりあえず、NSDecimalNumberについて言えば明らかにダメです。
let d : NSDecimalNumber = 15.97 _ = d.stringValue // => "15.970000000000004096"
15.97とは似ても似つかない値になりました。Stringから作る場合と見比べれば明らかですね。
let d = NSDecimalNumber(string: "15.97") _ = d.stringValue // => "15.97"
やはり、NSDecimalNumberを作るときには、整数か文字列を与えるのが安全なようです。
さて、これはなぜでしょう？「浮動小数点はそんなもんだよ」と言われて、一瞬納得してしまいそうになったのですが、やっぱりよく考えた結果、これはNSDecimalNumberが壊れているというのが私の現時点の理解です。
浮動小数点に関しては、僕は全然経験がないので、ここから先まったく間違ったことを言うかもしれません。 自信はありません。 間違っていたら、教えてもらえると嬉しいです。
Floatのリテラルとは
0.1を10回足しても1.0にならないことはよく知られていると思います。
let a = 0.1 + 0.1 + 0.1 + 0.1 + 0.1 + 0.1 + 0.1 + 0.1 + 0.1 + 0.1 _ = (a as NSNumber).stringValue // => "0.9999999999999999"
ソースコードに0.1と書いても、それは実数の0.1ではありません。Doubleで表現できる中で一番実数の0.1に近い数です。この辺りの感覚があると、15.97が15.970000000000004096になると言われても納得しても良さそうな気がする感じもします。
ところが、0.1を一度Doubleにしてから文字列表現に直すと、"0.1"が得られます。0.1ではないのに。少し不思議な気がしますね。これは、Doubleを表現する10進数の中に0.1よりも近いものが他にないことが理由です（多分）。Floatから文字列への変換をどういうアルゴリズムで実装するかによって違いが生まれますが、SwiftとかNSNumberではそうなっているようです。Swiftで試すと、0.1に対応するFloatの文字列表現は"0.1"ですし、15.97に対応するFloatの文字列表現は"15.97"です。
FloatをNSDecimalNumberに変換するというのは10進数に変換するのと同じことですから、NSDecimalNumberにDoubleを与えるとおかしくなるというのは辻褄が合わない話に思えます。
もちろん、変な数を考えるとうまく行かなくなることはあります。
let x = 0.09999999999999986 let y = 0.09999999999999987 _ x == y // => true
ここで、xとyは区別できないので、文字列表現も元には戻りません。
_ = (x as NSNumber).stringValue // => "0.09999999999999987" _ = (y as NSNumber).stringValue // => "0.09999999999999987"
まあ仕方ないですね。NSDecimalNumberにすればこの二つは区別できるようになります。
let x = NSDecimalNumber(string: "0.09999999999999986") let y = NSDecimalNumber(string: "0.09999999999999987") _ = x.stringValue // => "0.09999999999999986" _ = y.stringValue // => "0.09999999999999987"
15.970000000000004096
それでは、15.970000000000004096とはなんなのでしょうか。15.97は15.970000000000004096だったのでしょうか。
let s = 15.97 let t = 15.970000000000004096 _ = s == t // => false
もちろん違います。この二つは別々の数として認識されています。15.97と15.970000000000004096の間の数すら、Doubleは表現できます。
let u = (s + t)/2 _ = s < u && u < t // => true
15.970000000000004096は、意味がわからない謎の数字なのです。同じ例は、0.09999999999999986でも見ることができます。
let x : NSDecimalNumber = 0.09999999999999986 _ = x.stringValue // => "0.09999999999999983616"
0.09999999999999983616はどこから出てきたのでしょうか。ちなみに0.09999999999999986と0.09999999999999983616の間には0.09999999999999985があります。
正しい実装の例
さて、DoubleをNSDecimalNumberに変換する正しいコードは次のようになります。この関数にFloatの15.97を与えると15.97を指すNSDecimalNumberが得られます。0.09999999999999986と0.09999999999999987のような場合に注意は必要ですが、それは仕方ないでしょう。
ちょっと難しいかもしれませんが、がんばって読んでみてください。
func decimalNumberWithDouble(double: Double) -> NSDecimalNumber { let number = double as NSNumber return NSDecimalNumber(string: number.stringValue) }
参考にしたもの
直接この問題について理解するというよりは、ぼんやり眺めて浮動小数点の周りの空気がわかった気がした。
特定のMacでビルドできないXcodeプロジェクト
Xcodeに登録されているファイルの名前とファイルシステム上の名前が大文字小文字で違っている場合、Apple Storeから買ってきたばかりのあなたのMacではビルドできるかもしれませんが、Case Sensitiveなファイルシステムに設定しているMacではビルドできません。
この画像でUBTypedJSONDictionary.hとなっているソースコードは、ファイルシステム上はUBTypedJsonDictionary.hでした。なんでこうなったかというと、コードレビューで「NSJSONSerializationがあるのだからここはJSONであるべき」と指摘されて、なるほどと思ってXcodeでRenameしたらファイルシステムがついてこなかったからです。
注意しましょう。
注意したくない人は、Xcodeプロジェクトに登録されたソースコードをスキャンして、Case Sensitiveなファイルシステムで問題になりそうな場合に警告するプログラムを作ったので、CIで実行したら良いと思います。
CI完了を待ってPRをマージするbotを作るためのライブラリを公開しました
CIを待って自動でPRをマージするbotを作ると便利だった - soutaroブログで、紹介した、CIが完了したときに自動でPRをマージするbotですが、皆さんが簡単に実装できるようnpm packageにしました。
動くと良いですね。（npm packageはまだ試してない。）
CIを待って自動でPRをマージするbotを作ると便利だった
GitHubを使っている場合「Pull Requestが作られたら自動でCIを実行し、グリーンになることを確認してからマージする」というのは割と広く採用されている開発のフローだと思います。PRごとにCIすることを確実にすることで、うっかりビルドを壊すようなPRをマージしてしまうことを防ぎ、いつの間にかmasterが壊れることを防ぎます。*1「CIが通っていないPRをマージしない」というのは、GitHubでそういう制約を設けることもできますし、そこまでやらずにチーム内の約束として共有しても良いでしょう。（ユビレジでは後者を採用しています。）
問題は、CIは一瞬では終わらないことです。ユビレジの場合、RailsのWeb appのCIには15分ほどかかりますし、iPad appのCIは30分かかります。CIのためのリソースにも限りがあるので、状況によっては既に15個CIのジョブが溜まっていて、完了までに1時間かかるなんてこともあります。もちろんこれは仕方がないことで、丁寧にコードレビューしてからPRをマージするような場合には問題にはなりません。どうせレビューするのに時間がかかるので、CI待ちの時間は相対的に小さなものとなります。しかし、ちょっとしたtypoの修正とか、コードレビューでokが出てからrebaseしたような場合だと、結構辛い。typoの修正は30秒くらいでレビューが終わったりしてすぐにマージしたくなるものですし、rebaseしただけのようなものも何十分もCIを待つのは苦痛です。CIを待たずにマージすることもできるようにはなっていますが、これはCIが動かないとか本当に急いでデプロイしたい場合の最後の手段なので、CIを待つのがダルい程度の場合にやっちゃうのは避けたい（避けるように訓練しています）。CIが完了するまでの間に他の作業を始めてしまったりすると、今度はマージするのを忘れたりするわけです。これはこれで辛くて、Webサイトのtypoの修正であれば早くデプロイした方が良いですし、下手をすると別のPRがマージされて再度rebaseしなくてはいけなくなったりして、とても辛い。
問題は、CIが完了すればマージして良いPRがあったら、CIが完了するまでそのことを覚えておいて、CI後に確実にマージしなくてはいけないことです。どう考えても、これは人類がやるべき仕事ではなくて、コンピュータにやらせるべき仕事です。つまり「CIが終わったらPRを自動でマージしてくれるbot」が必要です。必要だったので作りました。何ヶ月か動かしていますが、良い感じです。
- CI後に自動でPRをマージするには、
ShipItというLabelを付けます- やっぱりこれはまずいな、と思ったら、Labelを取れば良い
- 不安な場合は、コメントを付ければSlackに通知が行くので、どうしても止めたい人がそれを見たら反応します
- GitHubのHookでHubotにCIの完了が通知されたら、PRを探して、
ShipItラベルが付いていたらマージします
これが便利に使えるための暗黙の背景としては、
- 開発チームは誰でもPRをマージできる（基本的には、PRを作った人が覚悟を決めてマージボタンを押す）
- マージされるとWeb appはProduction環境に自動でデプロイされる
- うっかり間違ったバージョンをデプロイしてしまっても、Herokuの機能で簡単にロールバックできる
などがあると思います。*2
とても便利なので、皆さんも簡単に実装できるようnpm packageにしました。
Swiftでas!と書く場合のガイドライン
as!って言うのは、要するにダウンキャストできない場合にプログラムを終了させるものである。
if let x = expr as? SomeClass { f(x) } else { fatalError() }
と書くのであれば、
f(expr as! SomeClass)
と、同じことなので、as!で書いた方が良い。その方がタイプ数が減るし、コードの見た目も簡潔になって理解しやすくなる。長いコードはそれだけで苦痛だし、なにか間違ったプログラミングをしていることを示すシグナルにもなる。ダウンキャスト失敗の場合に終了するためだけにifを書くことは、全体的なコードの乱雑さを増してしまい、本当の問題の見落としに繋がる。
ただし、as?とかas!とかダウンキャストをしてる時点で、そのプログラムには潜在的な問題があると考えることもできて、本当にダウンキャストが必要なのか3回くらい考えた方が良い。
as!とObjective Cのキャストの違い
- Objective Cのキャストは常に成功する
as!は失敗してエラーになることがある
この違いをまず頭に入れよう。次のObjective Cプログラムはエラーにならずに実行できる。
NSArray *x = (NSArray *)@"Hello World";
キャストは、コンパイルエラーにならないし実行時エラーにもならない。NSArrayにあってNSStringにないメソッドを呼び出すと実行時エラーになるが、キャストそのものは失敗しない。この性質は、3年に1回くらいは便利なこともあるが、大体は不便である。不正にキャストしたxが、ずっと先の全然ちがうところで実行時エラーを起こしたりするので、間違いを探すのに苦労することになる。
一方、次のSwiftプログラムはコンパイルはできるが実行するとエラーになる。
let x = "Hello World" as! Array<String>
Swiftは、不正なキャストに失敗してエラーを発生させる能力を獲得したのだ*1。これは、Objective Cに比べるとかなり改善されていて、不正なキャストからずっと先の全然違うところで実行時エラーになってデバッグに苦労することがなくなる。
この点で、Objective CのキャストとSwiftのキャストはかなり性質が違うものである。isKindOfClass:でテストせずにキャストするObjective Cのコードはかなり書かないほうが良いが、as!するSwiftのコードはかなりマシであると言える。
さらに、キャストに失敗したときに何が起きるかは決まっている。あなたのプログラムが終了するだけである。iOSに影響が及ぶことはないし（あったとしたらそれはOSの問題だ）、iPhoneが壊れることもないし、鼻から悪魔が出てくることもない。（ただし、アプリケーションやユーザーの性質によっては、それでも強制終了しない方が良い場合はあるとは思う。）
as! した方が良い場合
ダウンキャストに失敗した場合にそこから回復する手段がないとき、つまり直ちにプログラム終了するしかない場合はas!してしまう方が良い。「キャストに失敗したらプログラム終了」というのがas!の意味なので、わざわざ冗長にifを書く必要はどこにもない。
as?した方が良い場合
ダウンキャストに失敗しても処理が続けられる場合は、as!してはいけない。例えば、View Controllerに書かれているような、ユーザーとのインタラクションを担うようなコードの場合は、ダウンキャストの失敗というなにかプログラム実行の大前提が崩れるような事態にも、適切なエラー回復の手段を提供できる可能性がある。
どのくらい頑張ってエラー回復するかというのは、アプリケーションやユーザーの性質に関する問題なのでなんとも言えない。
コードレビューをしていてas!を見つけたら
本当にプログラムを終了する以外に回復の方法がないのか、議論して良いし、議論するべきである。適切な回復の方法があるのであれば、そちらに直す。また、as!もas?もしないことについても検討する。標準ライブラリとかObjective Cとの相互運用を除けば、「enumにしてswitchする」「protocolを追加して分岐しなくて良いようにする」などの方法がある。
Lintとどうつきあうか
個人的には、as!に警告するルールはオフにするのがお勧めだけど（僕はLint大嫌いなので、多分これは少し極端な見解だろうとは思う）。まー現実的には、堂々と
// swiftlint:disable:next force_cast
と書くのが良いだろう。これを見たレビュアーは強制終了するほかにエラー回復の方法がないのかあなたに説明を求めるだろうし、あなたは説明できなくてはいけない。
