この記事は、 Scalaz Advent Calendar 2012 の8日目です。
タイトルのとおり、気がついたらこれを書いている時点で今まで(mergeされたものだけで)合計 18 回も送っていたので、それらについて淡々と説明します。
ほかのプロジェクトにも細かいpull requestは結構送ってますが、なぜかScalazがダントツで1番多く貢献してるプロジェクトになってました。どうしてこうなった・・・
https://github.com/scalaz/scalaz/pulls/xuwei-k?direction=desc&page=1&sort=created&state=closed
1 fix scalaz-seven unidoc Task fail
sbtでプロジェクトをそれぞれ分けておいた場合、「すべてをまとめたScaladocを作る機能」が現状sbtに存在しないので、「自前でそれぞれのプロジェクトのソースを一箇所に集めてきて生成する」というのが、いつかのライブラリで行われています。それで、
「普通にコンパイルは通る状態だったのだけれども、全部を集めて一度にコンパイルした場合に限り依存関係と名前空間の関係で、コンパイルに失敗する」
というバグを直したものです。もっと細かくいうと
2 fix compile error ScalazBuild.scala
おそらくマージのミスにより、「ビルドのファイルが明らかにコンパイル通らない変な状態になっていた」ので直しただけ
3 fix scaladoc
最初「scalaz.syntax.GroupV」とか「scalaz.syntax.MonoidV」という、「最後にVをつける命名規則」だったのが、途中から「scalaz.syntax.GroupOps」や「scalaz.syntax.MonoidOps」とOpsをつけるように変わった。が、ソースコード本体は変更されていても、Scaladocの部分だけ古いままだったのでなおしたもの。
4 fix NSInfo annotName
tonymorrisさんが作ったxmlのモジュールで、実装的にはなにをやっているのか今もあまり理解できていないが、コンパイラが
comparing values of types (List[Char], List[Char]) and List[Char] using `==' will always yield false
[warn] prefixes find (_ == w) map (_._2)
といって、「型から考えると明らかにこの比較は毎回falseになるよ!」といっていた。ので、ポーティングしてきたもとのHaskellのライブラリ *1 を見ながら、その部分だけなおしたもの。
これ、本当はもっとテスト書くべきはずなのだが、そこまでの力はなかった・・・。現状も多分テスト足りないはずなので、誰か・・・
5 change to case object LazyNone
Scalazには、Scala標準のOptionとほとんど目的は同じだが、Lazyに評価されるLazyOptionというものを独自実装している。とくにバグではないというかそれほど困らないが、LazyNoneはclassではなくobjectでいいと思ったので、それを変えただけ。Scala標準のNoneもclassではなくobjectになっているし。細かい利点を考えれば、メモリ節約になるとか、LazyNoneの型を書く必要が無くなる場合があるとか?
6 fix nonEmptyList instance type
メソッドは実装したのに戻り値の型を書きかえてなかったことにより、実際使おうとした場合使える状態になってなかった。なので、戻り値の型を修正した、というだけです。
7 use type alias options
これはまったくバグではないですが、便利なaliasがすでに定義されていて、せっかくなのでそっち使ったほうが読みやすい(?)のと、(あまり期待できないが)あとから変更する場合に変更がしやすいかもしれないので、たんにaliasのほうを使うように変えただけです。実際の動きは全く変更ありません。
8 fix ephemeralStreamInstance type
fix nonEmptyList instance type のバグと全く同じです。
ところで、この件に関して、
「こういうバグが起こり得るから、どうせならtypeclassのインスタンス戻り値の型は省略しよう!」
とか
「いや、ちゃんと戻り値の型は必ず書きましょう」
という議論がまったく起こってなくて、かなり書き方がバラバラだったりします。まぁ気になるなら、自分がメーリングリストで提案して議論すればいいんですが、そこまで大きな問題じゃないのと、おもに英語力的にちゃんと議論する自信とか気力もないので結局放置(´・ω・`)
9 add isMZero method to MonoidSyntax
Scalaz 7では、typeclassに直接定義してあるメソッドとは別にユーザーができるだけ型を書かないで済むようにするために、syntaxというオブジェクトがそれぞれのtypeclassに対応するように定義されています。
もうちょっとくわしい解説は、公式のこのあたりのドキュメント
https://github.com/scalaz/scalaz/tree/v7.0.0-M5#syntax
https://github.com/scalaz/scalaz/blob/v7.0.0-M5/doc/DeveloperGuide.md#syntax
読んでください。
それで、基本的にはtypeclass本体に定義されているメソッドは、syntaxの方にも定義するべきはずなのですが、なぜかisMZeroメソッドが定義するのを忘れられていたので、付け加えただけというもの。
10 fix self recursive bug
これは明らかなバグでした。外側にあるメソッドを呼びたいはずだったのに、新しく作っている無名クラス自身のメソッドを呼んでしまっていて、明らかになにもせずに自身を呼びつづけて無限に再帰する状態になってました。これに気がついた経緯が fix NSInfo annotName と同様に、コンパイラが警告を出していたことによるものです。コンパイラすごいですね。
ところで、誰かが使っていればこんなバグはすぐに判明するはずなのに、まだconcurrentのモジュールは全然使われていないんですかね、残念ですね・・・(´・ω・`)
11 delete intInstance distance
おそらく勘違いというか、間違って全く使っていない必要のないメソッドが入っていたので、単にそれを消したというものです。細かいことをいうと
URL intInstanceの型変えずにメソッドだけ追加してて、これじゃ呼べないから単に削除していいのかな。あと「MetricSpace[Int]のインスタンスを別に下に定義している」意図がよくわからないが /cc @halcat0x15a
2012-11-21 00:24:25 via web
あぁ contramap が Order と MetricSpace 両方にあるのかURLURLURL
2012-11-21 17:16:10 via web
12 fix EitherT ===
これもわかりにくいですが明らかにバグだと思います。しかも厄介なバグというか、場合によってはうまくいってしまうという感じです。
Equalのtypeclassを要求しているのにもかかわらず、実際それを使っていませんでした。なので
「===と==の結果が同じ型の場合はバグにならない」
ですが
「===と==の結果が異なる型の場合のみ予想外の挙動になる」
というものです。
13 relaxed ListT methods constraint
twitter上でこんな会話がありましたが
scalaz7のListTこんな感じに修正できるんだが、さらに修正するところあるかもだしそもそもこの修正あってるのか微妙に自信ないし、まだmergeされてないpull req残ってるし、もう少し落ち着いて考えてからpull req送ろう URL
2012-11-23 08:45:48 via web
@xuwei_k ScalazってPointed,BindでいいところがMonadになってたりっていうのはよくありますよね。これでいいのでは?
2012-11-23 11:03:21 via Tweet Button to @xuwei_k
.@halcat0x15a これでいいとは、このままpull req送っていいってこと?こういう箇所まだ残ってる気がするんだけど、なにか効率的に見つける方法ないんだろうか・・・
2012-11-23 11:05:01 via web to @halcat0x15a
@xuwei_k こういうのは一杯あると思うので、私は使ってる時に不便だなと感じたらpull req投げてます!
2012-11-23 11:10:20 via web to @xuwei_k
ねこはる先生のいうとおり、「Monadでなくていいのに、制限が強すぎる場合が多い」ことが多々あるので、それをなおしたもの。この"制限"が何なのかは、kazu yamamotoさんのこのスライド
http://mew.org/~kazu/material/2012-monad.pdf
とかわかりやすい(51ページ目あたり)のでおすすめです。あとは Typeclassopedia 読むといいですね
http://snak.tdiary.net/20091020.html
http://typeclassopedia.bitbucket.org/index-ja.html
14 fix BooleanOps scaladoc
Scaladocの記法をhtmlからwiki記法になおしている最中にコピペミスしたらしく、明らかにおかしな記述になってしまっていたのを修正したものです。
15 fix scalacheck url
Scalacheckはgoogle codeからgithubにだいぶ前に移行しました。Scalacheckのgoogle codeのページを見に行っても「githubに移行したよ」と書いてあるので、全然大きな問題ではないのですが、1クリックの手間を減らすだけのものですね。
16 fix history instance type
これもまた fix ephemeralStreamInstance type やfix nonEmptyList instance type と同じです。Pointed実装してるのに、なぜか型がFunctorになってました。
17 delete unnecessary inheritance
URL URL URL MonadPlusとApplicativePlus両方mixinしてあるけど、ApplicativePlusいらないよな、それともなにか理由あるのか
2012-12-04 19:41:36 via web
18 fix MapTest
このpull requestを送った日からみて、
6日前に、MapのOrderのinstanceが修正されていて
https://github.com/scalaz/scalaz/commit/10966058000d82e9869b93ad380f92b374cc18f2
3日前にこのテストが加えられていました。
https://github.com/scalaz/scalaz/commit/14ec5a5ac8a8902a604ba552d60038751d78dd8a
それで、テストのほうがたまに失敗することがあるので修正したもの。
具体的には、テストコードの中で、List[(String,Int)]に対してtoMapメソッドを呼んでMapに変換していますが、KeyとなるStringに同じ物があった場合のみ、テスト自体がおかしくなってしまい失敗する可能性がでてくるというもの。
Genから真面目に作りなおすのが正当かもしれないけど、それは面倒なので、とりあえずKeyのStringがダブっていた場合は無条件でpassするようにしてしまいました。そしたら、mergeされた後にすぐ直されてましたね。
https://github.com/scalaz/scalaz/commit/2b8fbfa795e29a98bec72aa027e329bc54370c00
==> というメソッドが Scalacheck にあるんですね・・・
https://github.com/rickynils/scalacheck/blob/1.10.0/src/main/scala/org/scalacheck/Prop.scala#L100
以上、いままで自分が送ったpull requestを淡々と解説してみました。
どれもだいぶ細かいものばかりですが、明らかに間違っていて修正するべきものは、べつに英語のコメントが一切なくてもサラッとマージしてくれるし、Scalazに限らず気がついたらもっとみんな(twitterとかで文句だけ言うのではなく)どんどんpull request送りましょう。