chain method後のインデントがおかしいはずなのにRuboCopに怒られない

[sachin21]

Summary

I found that broken indents in chain-method after do-end under codes but it has no comment from RuboCop. chain methodの後のdo-endの中身(code)が壊れてのにRuboCopが検出しないので起票しました。

Problem Summary

This is the code related to Rails. これがソースコードで、Rails関連のコードです。

models.each do |model|
  model
    .attributes
    .except('id', 'created_at', 'updated_at')
    .reject { |_k, v| v.nil? }
    .each do |key, value|
    name = "#{model.class.to_s.underscore}[#{key}]"

    if value.is_a?(TrueClass) || value.is_a?(FalseClass)
      check(name)
    else
      fill_in(name, with: value)
    end
  end
end

So I think indents is borken because name and each block's indent size is same but it has comment from RuboCop. I think name variable and if-block is needed more 2 indents. because it is in the each do-end. btw, I executed auto-correct but it didn't change anything. each methodとname変数のindent sizeが一緒なので正直見辛いと思いますが、RuboCopには怒られません。 私はchain methodの後でもeachの中身は2個インデントするべきだと思います。 ちなみにauto-correctしても何も変わりませんでした。

Environment information

• RuboCop: 0.51.0
• Ruby: 2.4.2p198 (2017-09-14 revision 59899) [x86_64-linux]

[sachin21]

これバグだと思ったんですが、どう思いますか？ @pocke

[pocke]

これは問題を単純にすると、

a
  .select { |x| something(x) }
  .each do |b|
  foobar
end

a
  .select { |x| something(x) }
  .each do |b|
    foobar
end

このようにメソッドチェインを改行した後にブロックの中身をいくつにインデントするか、という問題ですよね。

個人的にはこれはバグと言うか機能追加かなーと思っています。 どっちのスタイルでインデントを揃えたい人もいると思いますし、何らかのオプションをLayout/IndentationWidth辺りに足して、スタイルを切り替えることで解決されるべきだと思います

Layout周りは全然詳しくないので、どのCopをどのように直すべきか、というところまではちょっと判断がつかないです、すみません。

[sachin21]

このようにメソッドチェインを改行した後にブロックの中身をいくつにインデントするか、という問題ですよね。

Yeah, exactly. その通りです。

どっちのスタイルでインデントを揃えたい人もいると思います

I see. I don't want to use the former but this is like fancy. Thanks. なるほど、ぼくは @pocke さんのサンプルコードの後者(下部)以外あり得無いと思ったんですが、好き好みの領域だったんですね。

Layout/IndentationWidth辺りに足して、スタイルを切り替えることで解決されるべきだと思います

Thank you for the nice how to solve this problem. 具体的な解決策をありがとうございます。

Layout周りは全然詳しくないので、どのCopをどのように直すべきか、というところまではちょっと判断がつかないです、すみません。

np, Thank you so much! いえ、参考になりました。ありがとうございます！

[pocke]

https://twitter.com/p_ck_/status/933666879129886720 気になったので、Twitterでアンケートをとってみました。どちらのインデントのスタイルも存在するみたいですね…

[meganemura]

Twitter のリプライでは書け無さそうだったのでこちらで...

自分は最後の end の位置に違和感がありました。 最後にもういちどメソッド呼び出すとかんがえると特にガタガタして読みにくくなると思います。

# ガタガタして見える
a
  .map do |x|
    x
end.map do |x|
  x
end

# ガタガタして見える
a
  .map { |x| 
    x
}.map { |x|
  x
}

なので、ブロック付きメソッドを呼び出した箇所と同じインデントに end, } をあわせた方が読みやすいのではないかと思ってます。

# しっくりくる
a
  .map { |x| x }
  .map { |x| 
    x
  }.select do |x|
    x
  end

# しっくりくる
a.map { |x| x }
  .map { |x|
    x
  }.select do |x|
    x
  end

メソッドチェインが改行したあとに複数行のブロックがついたら殴るCopとかがあったらいい気がしてきた。

pocke さんのこれに 👍 です。

[sachin21]

@meganemura Uhh, You mean it might be this? true, that sounds right to me and it no returns offenses from RuboCop :policeman: あーこういう事ですか？確かにしっくりきますし、RuboCopにも怒られません。

models.each do |model|
  model
    .attributes
    .except('id', 'created_at', 'updated_at')
    .reject { |_k, v| v.nil? }
    .each do |key, value|
      name = "#{model.class.to_s.underscore}[#{key}]"

      if value.is_a?(TrueClass) || value.is_a?(FalseClass)
        check(name)
      else
        fill_in(name, with: value)
      end
    end
end

[meganemura]

@sachin21 そういうことデス〜

[sachin21]

@meganemura I understand. Thanks! 理解しました。ありがとうございます！

[sachin21]

完全に閉じ忘れでした

書き方で解決できるので、一旦閉じます