Class: Scryer::PerformanceRules::InefficientSaveLoopRule

Inherits:
Rule
  • Object
show all
Defined in:
lib/scryer/performance_rules/inefficient_save_loop_rule.rb

Overview

Flags .save/.save!/.update/.update!/.update_attribute(s) called on the block variable inside a .each/.each_with_index loop — one database write per iteration where a single bulk statement (update_all/insert_all/upsert_all) could often do the same work in one round-trip. Best-effort: this rule can't tell whether the per-record operation is actually uniform enough to batch (e.g. distinct values per row still need update_all with a CASE, or don't fit the bulk-method shape at all) — it's a nudge to double check, not a guarantee the loop is replaceable as-is.

Constant Summary collapse

LOOP_METHODS = 
%w[each each_with_index].freeze
BARE_METHODS =

Genuinely argless in normal use — a bare :call node, never wrapped by a method_add_arg, so matching this tag alone can't double-count.

%w[save save!].freeze
ARG_METHODS =

Always take an argument, so they only ever show up wrapped in method_add_arg/command/command_call — matching only the wrapper avoids double-counting the inner call node these wrap.

%w[update update! update_attribute update_attributes].freeze

Instance Attribute Summary

Attributes inherited from Rule

#file, #sexp, #source

Instance Method Summary collapse

Methods inherited from Rule

inherited, #initialize

Constructor Details

This class inherits a constructor from Scryer::Rule

Instance Method Details

#scan ⇒ Object 



27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
 
# File 'lib/scryer/performance_rules/inefficient_save_loop_rule.rb', line 27

def scan
  findings = []

  Ast.each_node(sexp) do |node|
    next unless Ast.tagged?(node, :method_add_block)

    call_node = node[1]
    _receiver, loop_method = Ast.call_name(call_node) || [nil, nil]
    next unless LOOP_METHODS.include?(loop_method)

    block_node = node[2]
    param_name = block_param_name(block_node)
    next unless param_name

    find_save_calls(block_node[2], param_name).each do |line, method_name|
      findings << finding(
        line: line,
        message: "Inside a `.each` loop, `.#{method_name}` runs once per record — each call " \
                  "is a separate database round-trip, which scales linearly with the number " \
                  "of records instead of running as one bulk statement.",
        suggested_fix: "If every record gets the same update, replace the loop with " \
                        "`Model.where(...).update_all(column: value)` (one UPDATE for the whole " \
                        "set). If each record's new values differ but come from data already " \
                        "in hand, `upsert_all`/`insert_all` with an array of attribute hashes " \
                        "can also replace the per-row round-trips."
      )
    end
  end

  findings
end