slim-lintの５個のルールを調査し、３個を適用、２個を除外したままにした。

[masyuko0222]

Issue

• slim-lintのルールを適用する6 #7252

概要

slim-lintで除外（記載）されている以下の５つのルールについて調査を行い、問題がなければ除外せずに適用しました。

- Layout/BlockAlignment 
- Metrics/BlockNesting
- Naming/FileName
- Style/IfUnlessModifier
- Style/Next

調査内容

適用をしたルールは３つです。適用に至った理由は以下のリンクに記載しています。

• Layout/BlockAlignment
• Metrics/BlockNesting
• Naming/FileName

除外をしたままにするルールは２つです。除外のままに至った理由は以下のリンクに記載しています。

• Style/IfUnlessModifier
• Style/Next

変更確認方法

1. feature/apply-slimlint-rule-6をローカルに取り込む。
2. bundle exec slim-lint app/views/ -c config/slim_lint.ymlを実行し、エラー（警告メッセージ）が出ないことを確認する。

Screenshot

UI上の変化はないため割愛します。

[masyuko0222]

@komagata
以下のルールを適用するべきかご相談させてください。

• Style/IfUnlessModifier

こちらのルールは、条件式が１行で書けるにも関わらず、複数行使用している場合に警告を発します。

# bad
if condition
  do_stuff(bar)
end

# good
do_stuff(bar) if condition

一方で、このルールはLayout/LineLengthで指定された文字数(160文字)を越えない限りは、警告対象となってしまいます。
そのため、以下のように条件式が非常に長い場合でも修正を求められてしまいます。

• app/views/products/show.html.slim

// before
- if @product.user == current_user && !@product.wip? && !@product.checked? && @product.commented_users.mentor.empty?
  = render 'message_for_after_submission'

// after
= render 'message_for_after_submission' if @product.user == current_user && !@product.wip? && !@product.checked? && @product.commented_users.mentor.empty?

上記を踏まえ、対応としては以下の４点があると思っています。

1. 条件式が長い場合はHelperに移動をさせつつ、ルールの適用をする
2. 除外をしたままにする
3. そのままルール通りの適用をする
4. Layout/LineLengthの設定を変えて、ルールの適用をする（別PRでLineLengthのルール適用を行っている最中なので、ちょっと面倒くさい）

自分の中では順番通りの優先度となっています。ご意見いただけると幸いです。

[komagata]

@masyuko0222 なるほどです。
Style/IfUnlessModifierは除外で大丈夫です〜

[masyuko0222]

@komagata
以下のルールを適用するべきかご相談させてください。

• Style/Next

こちらのルールは、繰り返し文の中で条件式の代わりにnextを使える場合に、警告を発します。

# bad
[1, 2].each do |a|
  if a == 1
    puts a
  end
end

# good
[1, 2].each do |a|
  next unless a == 1
  puts a
end

一方で、Slimだとスタイル用のidやclassを書いてある行が非常に多いため、eachブロック開始から該当の行まで数十行を挟んでしまうこともあります。
以下のコード例はeachブロック開始は36行目、警告対象の行が93行目となっています。
https://github.com/fjordllc/bootcamp/blob/a84b95664bcd073f33b797a2e4aa38a1ac24071c/app/views/surveys/show.html.slim#L36C1-L93C68

上記のコード例だと、if式のままの方が可読性が高いと個人的に思います。
そのため、このルールも除外のままにしておきたいのですが、いかがでしょうか。
ご意見いただけると幸いです。

[komagata]

@masyuko0222 なるほどです。
ではStyle/Nextは除外して大丈夫です〜

[masyuko0222]

@nishitatsu-dev
お疲れ様です！ご都合よろしければレビューして頂けないでしょうか！m(_ _)m

[nishitatsu-dev]

@masyuko0222
お疲れ様です！レビュー承知しました〜
１週間以内に対応しますので、少々お待ちください🙏

[nishitatsu-dev]

@masyuko0222
適用した３ルールを確認しました〜

• コードはLGTMなので、Approveします！
• 検討結果について、細かい話ですが、下記だけ修正しておいて下さい〜
  Metrics/BlockNestingのネスト数ですが「３つまではOKで、４つ以上がNG」でした。数字の修正お願いします🙏

[masyuko0222]

@nishitatsu-dev

Metrics/BlockNestingのネスト数ですが「３つまではOKで、４つ以上がNG」でした

ありがとうございます！more than 3 levels of block nesting.で出る警告なので、３は含まずに４つ以上でしたね・・・。
数値は修正しました、お手数おかけしました。

@komagata
Approve頂いたので、レビューのほどお願いいたします！

[komagata]

確認させて頂きました。OKです〜🙆‍♂️