ISUCON で型がパチパチっとハマった開発ができるとかなり開発体験変わってくるのでは?と思い、 ISUCON の過去問に型をつけていくのをやってみています。
モチベーションに対してもう少し詳しい記事はこちら
まずは、初期実装の状態から挙動を変えずに型だけをつけてみることに取り組みます。
また、アプリケーションに対応する型は rbs ファイルは直接触らずに rbs-inline のみを使って生成することにしました。おそらく ISUCON 本番でも別ファイルをいじっている余裕はないと思うためです。
ISUCON13 で rbs-inline を使って steep check が通るところまで行けたので、やっていく中で感じたことやこうだったらもっと便利なのにと思ったことをまとめてみます。ちょっとしたスクリプトに対して使ってみた記事はたまに見かけますが、ちゃんとしたアプリケーションで使ってみた記事はあまり見たことがないので何かの参考になれば幸いです。
初期実装からの差分はこちらです。差分を眺めながら記事を読むと内容が掴みやすくなると思います。
https://github.com/euglena1215/isucon13-rbs/compare/3bb0e7c..main?diff=unified&w=
ISUCON でよく使われるライブラリに型が全然書かれてない
まあこれはそうだろうなと思って始めました。これに関しては型を書いていけばいいことです。ISUCON13 で使われているライブラリに関しては一通り型を書きました。
- https://github.com/ruby/gem_rbs_collection/pull/561
- https://github.com/ruby/gem_rbs_collection/pull/562
- https://github.com/ruby/gem_rbs_collection/pull/563
- https://github.com/ruby/gem_rbs_collection/pull/564
- https://github.com/ruby/gem_rbs_collection/pull/565
- https://github.com/ruby/gem_rbs_collection/pull/566
- https://github.com/ruby/gem_rbs_collection/pull/567
ISUCON で頻出ライブラリは限られているので次の過去問に型をつけるときはもっと楽になっているはずです。
Enumerable#first
の nil チェックをめっちゃ書く必要ある
正直型を書くだけで型チェックが通ると思ってました。
Enumerable#first
はリストの要素数が 1個以上であればその1つ目を返し、0個であれば nil を返すメソッドです。この nil が返ってくる挙動を無視して first を使った際に常に nil 以外が返ってくることを前提とした実装になっていたので nil チェックを書いていく必要が結構ありました。
具体的には以下のような実装です。
user_model = Mysql2::Client.new.xquery('SELECT * FROM users WHERE id = ?', 1).first user_model.fetch(:id) # ここで user_model 変数は nil が返ってくる可能性があるのに nil でない前提の実装になっている
上記の実装で steep check を実行すると下記のようなエラーが表示されます。
app.rb:208:19: [error] Type `(::Hash[::Symbol, ::Mysql2::row_value_type] | nil)` does not have method `fetch` │ Diagnostic ID: Ruby::NoMethod │ └ user_model.fetch(:id) ~~~~~
上記を解消するには下記ような nil チェックが必要です。
user_model = Mysql2::Client.new.xquery('SELECT * FROM users WHERE id = ?', 1).first raise if user_model.nil? # ここ user_model.fetch(:id)
nil チェックを追加すればいいだけなので難しいことはないんですが、ISUCON13 の初期実装で20箇所くらい対処した気がします。塵も積もれば山となるので、時間が惜しい ISUCON においては Enumerable#first!
のような nil を返さないメソッドを用意しておいた方がいいかもしれません。
自分はこんな感じのお手製 Enumerable#first
を app.rb に添えておきました。これで変更差分がかなり減りました。
# @rbs generic unchecked out Elem module Enumerable def first! #:: Elem first || raise('empty') end end
instance(T) 的なものがほしい
ISUCON13 には request body を Data.define
を使って定義したクラスにマッピングしてくれる便利メソッドがあります。
def decode_request_body(data_class) body = JSON.parse(request.body.tap(&:rewind).read, symbolize_names: true) data_class.new(**data_class.members.map { |key| [key, body[key]] }.to_h) end ReserveLivestreamRequest = Data.define(...) # こんな感じで使える。req には request body の中身がマッピングされた # ReserveLivestreamRequest クラスのインスタンスが入っている。 req = decode_request_body(ReserveLivestreamRequest)
この便利メソッドである decode_request_body
メソッドの型をどうつけるか悩みました。
これにどう対処したかというと、引数として取りうるデータクラスの Union を作って、返り値としてはその Union に対応するクラスのインスタンスを返すようにしました。ただ、これだとこのメソッドの呼び出し側で毎回データクラスのチェックをしないといけないんですよね。
# :: (singleton(ReserveLivestreamRequest) | singleton(PostLivecommentRequest) | singleton(ModerateRequest) | singleton(PostReactionRequest) | singleton(PostIconRequest) | singleton(PostUserRequest) | singleton(LoginRequest) data_class) -> (ReserveLivestreamRequest | PostLivecommentRequest | ModerateRequest | PostReactionRequest | PostIconRequest | PostUserRequest | LoginRequest) def decode_request_body(data_class) body = JSON.parse(request.body.tap(&:rewind).read, symbolize_names: true) data_class.new(**data_class.members.map { |key| [key, body[key]] }.to_h) end req = decode_request_body(ReserveLivestreamRequest) # 毎回 ReserveLivestreamRequest のインスタンスであることをチェックして Union を外す必要がある raise unless req.is_a?(ReserveLivestreamRequest)
どうなってると嬉しかったかというと、以下のように instance()
のようなインスタンスを表現するものがあれば良いのにと思いました。こうできれば呼び出し側でデータクラスのチェックを行う必要はないですし、仮にデータクラスをパターンとして増やしたくなったときもメソッドの型を修正する必要はありません。
#:: [T < Data::_DataClass] (T data_class) -> instance(T) def decode_request_body(data_class) body = JSON.parse(request.body.tap(&:rewind).read, symbolize_names: true) data_class.new(**data_class.members.map { |key| [key, body[key]] }.to_h) end
2024/05/28 追記
(singleton(A) | singleton(B)) -> A | B
といった型ではなく、(singleton(A)) -> A | (singleton(B)) -> B
という型にすれば呼び出し側でチェックする必要はないのでは?というアドバイスを pocke さんからもらいました。ありがとうございます。
decode_request_body の返り値の型が一意に定まるように Union の使い方を修正 · euglena1215/isucon13-rbs@5278c44 · GitHub
修正する中で rbs-inline の #::
を使った記法ではメソッドのオーバーロードをサポートしていないことが分かったので issue を立てておきました。
2024/05/31 追記
issue にて以下の構文はサポートしていないものの、
#:: (singleton(A)) -> A | (singleton(B)) -> B def decode_request_body(data_class) ... end
以下の構文はサポートしているということを教えてもらいました。
#:: (singleton(A)) -> A #:: (singleton(B)) -> B def decode_request_body(data_class) ... end
Syntax-guide にも書いてありましたね...
Data.define からクラス定義を生成してほしい
rbs-inline では Data.define
によって定義されたクラスは untyped な定数として宣言されます。これを定数としてみなしたい rbs-inline の気持ちも分かりつつ、実用上結構不便でした。
ReserveLivestreamRequest = Data.define( :tags, :title, :description, :playlist_url, :thumbnail_url, :start_at, :end_at, ) => ReserveLivestreamRequest: untyped
これにどう対処したかというと、 @rbs skip
を使って rbs-inline による定義を無視しつつ @rbs!
でコメントで型を直接書きました。@rbs skip
を使う必要に迫られたのはここくらいでした。
# @rbs! # class ReserveLivestreamRequest # extend Data::_DataClass # attr_reader tags: Array[Integer] # attr_reader title: String # attr_reader description: String # attr_reader playlist_url: String # attr_reader thumbnail_url: String # attr_reader start_at: Integer # attr_reader end_at: Integer # def self.new: (*untyped) -> ReserveLivestreamRequest # | (**untyped) -> ReserveLivestreamRequest | ... # end # @rbs skip ReserveLivestreamRequest = Data.define( :tags, :title, :description, :playlist_url, :thumbnail_url, :start_at, :end_at, )
self.members
メソッドが必要だったので Data::_DataClass
を include しています。Data::_DataClass
を include した状態で self.new
を呼ぶと Data::_DataClass
に定義されている self.new
の定義が利用されて Data
クラスを返す型になり困りました。このため、自身のクラスを返すように self.new
をオーバーロードしています。
これがどうなってると嬉しかったかというと、以下のような記述をすると上記の @rbs!
を使って記述した型定義が生成されると嬉しかったです。
ReserveLivestreamRequest = Data.define( :tags, #:: Array[Integer] :title, #:: String :description, #:: String :playlist_url, #:: String :thumbnail_url, #:: String :start_at, #:: Integer :end_at, #:: Integer )
2024/05/28 追記
soutaro さんから「ISUCONなら実行時に生成してしまえばいいのでは?」というアドバイスをもらって Data.define
を拡張した Data.typed_define
を用意しました。
class Data # @rbs self.@classes: Hash[untyped, Hash[Symbol, String]] @classes = {} #:: [KLASS < ::Data::_DataClass] (**untyped) ?{ (KLASS) [self: KLASS] -> void } -> KLASS def self.typed_define(**attrs, &block) k = define(*attrs.keys, &block) @classes[k] = attrs k end def self.generate_rbs(class_name, attrs) <<~RBS class #{class_name} extend Data::_DataClass #{attrs.map { |k,v| "attr_reader #{k}: #{v}" }.join("\n ")} def self.new: (*untyped) -> #{class_name} | (**untyped) -> #{class_name} | ... end RBS end at_exit do body = '' @classes.each do |klass, attrs| body += generate_rbs(klass.name, attrs) + "\n" end File.write('sig/generated/typed_data.rbs', body) end end
下記のように Data.define
を書き換えた上で bundle exec ruby app.rb
を実行すると sig/generated/typed_data.rbs
が生成されます。このコマンドを rake rbs:setup
に追加しておくことで常に更新されるようになりました。
# Before PostLivecommentRequest = Data.define( :comment, :tip, ) # After PostLivecommentRequest = Data.typed_define( comment: 'String', tip: 'Integer', )
業務ではもう少し体裁を整える必要がありそうですが、ISUCON では問題なく使えそうです。
Data.define に対応するクラスを自動生成した · euglena1215/isucon13-rbs@11a2230 · GitHub
Sinatra の helpers が型との相性が悪い
Sinatra の helper は「helper は別の helper を呼び出せる」「HTTP method 系ブロックで helper を呼び出せる」という挙動になっています。
class MyApp < Sinatra::Base helpers do def foo = 'foo' def bar foo # helper は別の helper を呼び出せる end end get '/foo' do foo # リクエストをハンドリングする HTTP method 系のブロックでも helper が呼び出せる end end
これ、静的な型チェックにおいては「helper は別の helper を呼び出せる」という文脈で呼び出した際はインスタンスメソッドとしての foo
を探しに行くんですが、「HTTP method 系ブロックで helper を呼び出せる」という文脈で呼び出した際は特異メソッドとしての self.foo
を探しに行くんですよね。そのため、helpers で定義したメソッドはインスタンスメソッドとして定義するのと同時に特異メソッドとしても定義しないと意図した挙動になりませんでした。
これにどう対処したかというと、メソッド定義の少し下に @rbs!
を使って型を直接コメントで記述しました。
helpers do def db_conn #:: Mysql2::Client[Mysql2::ResultAsHash] Thread.current[:db_conn] ||= connect_db end # @rbs! # def self.db_conn: () -> Mysql2::Client[Mysql2::ResultAsHash] end
ただ、この方法だと特異メソッドに対応する実装は存在しないため Ruby::MethodDefinitionMissing
で steep check でエラーになります。そのため、このエラーだけはエラーレベルを error から warning に落としてお茶を濁しています。
正直この問題に対してはどう対処すべきなのかがあまり見えていません。Sinatra で型を書こうと思った人はみんなハマる問題なんじゃないかと思ってるので何かしら対処できた方が良さそうな気はしているものの、どうしたらいいんでしょうね…
2024/05/28 追記
ブロックを引数に取る場合はブロック引数に [self: instance]
とアノテーションするとブロック内はインスタンスメソッドを実装しているのと同じ扱いをされるようになると soutaro さんから聞きました。ありがとうございます。
get
, post
といった HTTP メソッド系のメソッドのブロック引数に [self: instance]
をつけてみると、ISUCON13 で型のために必要な記述量が33行も減りました 🎉
不要な helper メソッドに対するシングルトンメソッドの定義の削除 · euglena1215/isucon13-rbs@a041faa · GitHub
型による恩恵は十分にありそう
型のある状態で問題を解いたわけではないので「ありそう」という表現にはなってしまうものの、十分に恩恵が感じられる手応えがありました。
例えば Mysql2::Client#xquery
は引数に as: :array
をつけるかどうかで返す型が変わります。
Mysql2::Client.new(...).xquery('SELECT * FROM users') # => Array[Hash] Mysql2::Client.new(...).xquery('SELECT * FROM users', as: :array) # => Array[Array]
型がない場合は、そもそもこの仕様を知っているか注意深くコードを読まないと把握するのは困難です。ISUCON という焦った状況の中だと自分はミスをして何回もベンチマークを走らせて時間を浪費しながら原因を特定する自信があります。
ですが、型をつけたことによって xquery
の引数によってどちらが返ってくるかの型を決定し、誤った参照をしている場合には steep check やエディタの波線ですぐに怒ってくれるようになっています。これならすぐに気付いてベンチマークを走らせる前に修正ができそうです。
Mysql2::Client.new(...).xquery('SELECT id FROM users').each do |row| row[0] # app.rb:xx:xx: [error] Cannot pass a value of type `::Integer` as an argument of type `::Symbol` # │ ::Integer <: ::Symbol # │ ::Numeric <: ::Symbol # │ ::Object <: ::Symbol # │ ::BasicObject <: ::Symbol # │ # │ Diagnostic ID: Ruby::ArgumentTypeMismatch # │ # └ row[0] # ~ end Mysql2::Client.new(...).xquery('SELECT id FROM users', as: :array).each do |row| row[:id] # app.rb:xx:xx: [error] Cannot find compatible overloading of method `[]` of type #`::Array[::Mysql2::row_value_type]` # │ Method types: # │ def []: (::int) -> ::Mysql2::row_value_type # │ | (::int, ::int) -> (::Array[::Mysql2::row_value_type] | nil) # │ | (::Range[(::Integer | nil)]) -> (::Array[::Mysql2::row_value_type] | nil) # │ # │ Diagnostic ID: Ruby::UnresolvedOverloading # │ # └ row[:id] # ~~~~~~~~ end
Ruby で型のある開発がどのような体験になるのかを知りたい方は手元に clone してみてちょっと触ってみるといいのではないでしょうか。セットアップ手順は書いてないのでそこはよしなにお願いします...
GitHub - euglena1215/isucon13-rbs
さいごに
こうなっていてほしいという点はいくつかあったものの、rbs-inline を使って型を書き切ることができたのは素晴らしかったです。諸々を見越した @rbs skip
や @rbs!
という抜け道が事前に用意されていたからこそだと思います。ありがとうございます。