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
SEED_FILE =

db/seeds.rb (and the multi-file db/seeds/*.rb convention Rails supports via Rails.application.load_seed) is a one-time, manually run setup script, not a request-handling hot path — the whole premise of this rule (per-record round-trips scale badly as load grows) doesn't apply to a script a developer runs once at setup time for a small, fixed reference dataset (roles, plans, countries, ...). Flagging roles.each { |r| r.save! } there is just noise, so this rule skips findings whose file is exactly this path or under this directory. Deliberately file-path-based rather than trying to infer "small dataset" from the AST (which isn't reliably knowable — a seed file can still iterate a CSV of arbitrary size), and narrow to this one well-known Rails convention rather than any file that merely "looks like a script" elsewhere in the app.

"db/seeds.rb"
SEED_DIR_PREFIX = 
"db/seeds/"

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 



44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
 
# File 'lib/scryer/performance_rules/inefficient_save_loop_rule.rb', line 44

def scan
  return [] if seed_file?

  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