Skip to content

Create cop for AR count/each to size/each in HAML file

Qingyu Zhao requested to merge 211780-create-cop-AR-count-each-to-size-each into master

Description of the proposal

This is to address one case in issue #211780 (closed), to create cop for AR count/each to size/each in HAML file.

@something.count; @something.each... => @something.load.size; @something.each... , where @something is an AR relation.

The idea is documented in https://www.speedshop.co/2019/01/10/three-activerecord-mistakes.html

The most common cause of unnecessary count queries is when you count an association you will use later in the view (or have already used)

Bad:

# _messages.html.erb
# Assume @messages = user.messages.unread, or something like that

<h2>Unread Messages: <%= @messages.count %></h2>

<% @messages.each do |message| %>
blah blah blah
<% end %>

Good:

<h2>Unread Messages: <%= @messages.load.size %></h2>

<% @messages.each do |message| %>
blah blah blah
<% end %>

Check-list

  • Make sure this MR enables a static analysis check rule for new usage but ignores current offenses (Note: current offenses are fixed in this MR)
  • Mention this proposal in the relevant Slack channels (e.g. #development, #backend, #frontend)
  • If there is a choice to make between two potential styles, set up an emoji vote in the MR:
    • CHOICE_A: 🅰
    • CHOICE_B: 🅱
    • Vote yourself for both choices so that people know these are the choices
  • The MR doesn't have significant objections, and is getting a majority of 👍 vs 👎 (remember that we don't need to reach a consensus)
  • (If applicable) One style is getting a majority of vote (compared to the other choice)
  • (If applicable) Update the MR with the chosen style
  • Create a follow-up issue to fix the current offenses as a separate iteration: ISSUE_LINK
  • Follow the review process as usual
  • Once approved and merged by a maintainer, mention it again:
    • In the relevant Slack channels (e.g. #development, #backend, #frontend)
    • (Optional depending on the impact of the change) In the Engineering Week in Review

/cc @gitlab-org/maintainers/rails-backend

Edited by 🤖 GitLab Bot 🤖

Merge request reports