pockestrap

Web Programmer's memo

RuboCop で `#each` がブロックの戻り値を使わないことを思い出せるようにした

問題

以下のようなコードは、恐らくバグになります。

json = users.each do |user|
  id = user.id
  full_name = user.full_name

  {
    id: id,
    full_name: full_name,
  }
end
render json: json

これはidfull_nameをもったオブジェクトの配列をjsonでrenderするように見えます。
ですが、実際にはuserに含まれる全ての(idfull_name以外も)情報がJSONとして返ってしまうでしょう。例えば、passwordなどが入っていたら目もあてられません。

また、idfull_nameは(恐らく)JSONとしてrenderされるため、開発をしていても問題に気が付かない可能性があります。

原因

この問題は、Array#eachArray#mapと間違って使ってしまったことに起因しています。 そしてこの2つの問題の違いは、ブロックの戻り値が使われるか、というところにあります。

eachはブロックの戻り値を使わず、selfを返します。 そのため先程の例ではusersjsonは全く同じオブジェクトが入ります。

対策

github.com

これを防ぐため、RuboCop でeachのブロックの最後の値がリテラルや変数など、評価しても何も起きないものである場合に警告を出すようにしました。 例えば先程の例では、eachのブロックの最後の値がリテラルであり、評価しても意味がないため、警告を出します。

これは既存のLint/Void cop の拡張として実装されています。

なお、この Cop はeachがブロックの戻り値を使うようなメソッドとして定義されている場合に偽陽性が存在します。 ですが、eachと言う名前をそのような用途に使うことは殆どないと考えられるため、問題にはならないでしょう。

この機能は次回の RuboCop のリリース後に有効になります。