Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
komagata opened this issue Jan 26, 2024 · 8 comments
Closed

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

komagata opened this issue Jan 26, 2024 · 8 comments
Assignees

Comments

@komagata
Copy link
Member

komagata commented Jan 26, 2024

slim-lintの指摘するルールの内、多くのルールを除外する設定にしている。(下記)

ignored_cops:
- Layout/ArgumentAlignment
- Layout/ArrayAlignment
- Layout/BlockAlignment
- Layout/EmptyLineAfterGuardClause
- Layout/EndAlignment
- Layout/FirstArrayElementIndentation
- Layout/FirstParameterIndentation
- Layout/HashAlignment
- Layout/IndentationConsistency
- Layout/IndentationWidth
- Layout/InitialIndentation
- Layout/LineLength
- Layout/MultilineArrayBraceLayout
- Layout/MultilineAssignmentLayout
- Layout/MultilineHashBraceLayout
- Layout/MultilineMethodCallBraceLayout
- Layout/MultilineMethodCallIndentation
- Layout/MultilineMethodDefinitionBraceLayout
- Layout/MultilineOperationIndentation
- Layout/ParameterAlignment
- Layout/TrailingEmptyLines
- Layout/TrailingWhitespace
- Lint/Void
- Metrics/BlockLength
- Metrics/BlockNesting
- Naming/FileName
- Style/FrozenStringLiteralComment
- Style/IdenticalConditionalBranches
- Style/IfUnlessModifier
- Style/Next
- Style/WhileUntilDo
- Style/WhileUntilModifier
- Style/HashSyntax

このIssueでは5つのルールを有効化し、指摘されるviewファイルをルールに沿うように修正してください。

1Issueにつき5ルールとします。

また、ルールによっては有効化することが現実的で無いものも存在するかもしれないのでその場合は、どういうルールなのかを調べて、@komagata , @machida に連絡し、そのルールをこのプロジェクトに適用するのか議論してください。

実際には適用しないとしても、調査の作業を持ってIssueは完了とします。

@masyuko0222
Copy link
Contributor

複数人で「slim-lintのルールを適用する」Issueに取り組んでおり、重複しないように以下のIssueで分担について話し合っています。
#7247

@masyuko0222
Copy link
Contributor

masyuko0222 commented Feb 27, 2024

こちらのIssueでは、以下のルールについて調査し、問題がなければ適用をします。

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

@masyuko0222
Copy link
Contributor

masyuko0222 commented Mar 4, 2024

Layout/BlockAlignment

Layout/BlockAlignmentのルールを適用させました。

こちらのルールは、ブロックのdoに対してendの位置がチームルールに沿っているかの確認をします。

Slimでは条件分岐・繰り返し文・ヘルパーメソッドではendを書かないので、そもそも警告される機会がなく、影響が無いと判断し適用しました。

ルール詳細

@masyuko0222
Copy link
Contributor

masyuko0222 commented Mar 4, 2024

Metrics/BlockNesting

Metrics/BlockNestingのルールを適用させました。

Slim内のRubyコードで、ブロックを4段以上ネストをすると警告文が表示されます。

現状ルール違反をしているファイルはありませんが、下記2点の理由により、ルールを適用させても問題がないと判断をしました。

  • 警告された場合でも十分に修正が可能である
  • 可読性を高めるための妥当なルールである

ルール詳細

@masyuko0222
Copy link
Contributor

Naming/FileName

Naming/FileNameを適用させました。

こちらのルールは、Rubyのソースファイル名がスネークケースとなっているかを確認します。

Slimファイルはそもそも監視対象外であり、影響が無いと判断し適用しました。

ルール詳細

@masyuko0222
Copy link
Contributor

masyuko0222 commented Mar 4, 2024

Style/IfUnlessModifier

Style/IfUnlessModifierは除外したままにします。

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

以下はコード例です。

# bad
if condition
  do_stuff(bar)
end

# good
do_stuff(bar) if condition

なお1行で書き換えた際に、Layout/LineLengthで定められた文字数(160文字)を超えない限り、警告対象となります。
すると以下のコード例のように、ルールを適用させると可読性が却って落ちてしまう場合もあるため、「除外したまま」という判断をいたしました。

// 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?

上記はPR内の会話でkomagataさんに相談の上、決定した事項になります。

ルール詳細

@masyuko0222
Copy link
Contributor

masyuko0222 commented Mar 4, 2024

Style/Next

Style/Nextは除外したままにします。

こちらのルールは、主にループ文内の条件分岐において、nextで代用できる箇所について警告を出します。

以下はコード例です。

// bad
- @tags.each do |tag|
  - if tag.present?
    | Hoge

// good
- @tags.each do |tag|
  - next if tag.blank?
  | Hoge

ですが、Slimだとスタイル用のidやclassを書いてある行が非常に多いため、eachブロック開始から該当の行まで数十行を挟んでしまうこともあります。
そのような場合は、if式のままの方が可読性が高いという結論になり、「除外したまま」という判断をいたしました。

例えば、以下のコード例はeachブロック開始は36行目、警告対象の行が93行目となっています。
https://github.com/fjordllc/bootcamp/blob/a84b95664bcd073f33b797a2e4aa38a1ac24071c/app/views/surveys/show.html.slim#L36C1-L93C68

上記はPR内の会話でkomagataさんに相談の上、決定した事項になります。

ルール詳細

@masyuko0222
Copy link
Contributor

masyuko0222 commented Mar 12, 2024

@komagata
mainブランチで確認が取れました。
lintルールのIssueゆえに画面確認は不要なので、Closeいたします。

@komagata komagata moved this to 完成 in bootcamp Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

2 participants