自分の今までの #Scalaz への merge された pull request が18回になってた件 #scalaz_advent

この記事は、 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に存在しないので、「自前でそれぞれのプロジェクトのソースを一箇所に集めてきて生成する」というのが、いつかのライブラリで行われています。それで、
「普通にコンパイルは通る状態だったのだけれども、全部を集めて一度にコンパイルした場合に限り依存関係と名前空間の関係で、コンパイルに失敗する」
というバグを直したものです。もっと細かくいうと

  1. coreのほうで「xml.Text」という相対参照の形でscala標準のxml以下を参照している箇所があった
  2. あとからscalazにxmlというモジュールができた(そのモジュールのpackageはscalaz.xml)
  3. coreはxmlに依存しているので通常は問題にならないが、全部混ぜてビルドした場合は、xml.Textというのが「scala.xml.Text」ではなく「scalaz.xml.Text」と認識されてしまい、コンパイルエラー
  4. なので、単に相対参照を使わず、フルネームで参照するように直した

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

おそらく勘違いというか、間違って全く使っていない必要のないメソッドが入っていたので、単にそれを消したというものです。細かいことをいうと

と、MetricSpaceのtypeclassのインスタンスをまとめてintInstanceのほうに定義できるならするべきだけれど、contramapという名前のメソッドがOrderと被っていて定義できないので、しかたなく別に「implicit val intMetricSpace」を定義した。わけですが、最初にintInstanceのほうに実装をやりかけたメソッドを消し忘れた?というのが原因のようです。


12 fix EitherT ===

これもわかりにくいですが明らかにバグだと思います。しかも厄介なバグというか、場合によってはうまくいってしまうという感じです。
Equalのtypeclassを要求しているのにもかかわらず、実際それを使っていませんでした。なので
「===と==の結果が同じ型の場合はバグにならない」
ですが
「===と==の結果が異なる型の場合のみ予想外の挙動になる」
というものです。


13 relaxed ListT methods constraint

twitter上でこんな会話がありましたが

ねこはる先生のいうとおり、「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 typefix nonEmptyList instance type と同じです。Pointed実装してるのに、なぜか型がFunctorになってました。


17 delete unnecessary inheritance

と思いたち、考えても理由なんてわからないので、とりあえずpull request送ってみたらあっさりmergeされました。まぁミスだったようです。べつにもとのままでもほとんど不都合ないし、修正後も基本的に動作は変わりません。余計な継承が減った分100byteくらいjarが小さくなってました・・・(すごくどうでもいい)


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送りましょう。