Class: RuboCop::Cop::DevDoc::Style::TapBlockIgnoresValue

Inherits:
Base
  • Object
show all
Defined in:
lib/rubocop/cop/dev_doc/style/tap_block_ignores_value.rb

Overview

Flag Object#tap whose block ignores the yielded value.

Rationale

tap exists to operate on the yielded receiver and return it — obj.tap { |x| configure(x) }. The yielded value is the whole point of the method.

When the block ignores the yielded value, tap is no longer doing that job. It's being used purely as a temp-variable-elision trick for "run a side effect, then return the receiver":

❌ Block never references the yielded value — `tap` is eliding a temp var
read_value.tap { do_side_effect }

This costs readability for no benefit:

  • It hides the return value in the receiver position. tap's "returns self" behaviour is its less obvious property. A reader has to stop and recall that the expression returns the receiver, not the block's value. The explicit form puts the return value on its own line where "what does this return" is literal.

  • Footgun magnet. Return-value semantics here are subtle — it's easy to nearby-edit the block into something whose value you think matters, or to confuse it with the broken expr; side_effect sequence (which returns the side effect's value, not expr's). Code whose contract is "return this value" reads and edits more safely when the return is explicit.

  • Equal stakes → prefer the duller form. Both forms are correct and neither is faster. With no functional difference, the more obvious one wins in a shared codebase.

    ✔️ Explicit intent, return value on its own line result = read_value do_side_effect result

When tap IS the right choice

Reserve tap for its canonical use — when the block references the yielded value:

✔️ Block uses the yielded value — this is what `tap` is for
Model.new.tap { |m| m.role = :admin; m.save }
record.tap { |r| logger.debug(r.to_sql) }

Exception

Genuine intentional uses (e.g. a logging side effect that reads clearer in tap position) go through an inline # rubocop:disable with a reason. The friction is the feature — it forces the choice to be articulated and reviewed.

NOTE: Numbered-param (_1) and implicit-it (Ruby 3.4+) forms are not flagged — by construction, those forms reference the yielded value, which is the canonical use of tap. Block-pass (&:sym) is likewise not flagged; it isn't a block node.

NOTE: If a nested block shadows the outer arg name (tap { |v| items.each { |v| v.foo } }), the inner reference is treated as covering the outer — a rare residual false negative. Inline-disable if it ever matters.

Examples:

# bad — block ignores the yielded value
read_value.tap { do_side_effect }
record.tap { |r| log_event }
value.tap { }

# good — block references the yielded value
Model.new.tap { |m| m.role = :admin; m.save }
record.tap { |r| logger.debug(r.to_sql) }

# good — a reference inside a nested block still counts
items.tap { |i| [1, 2].each { i << _1 } }

# good — preferred explicit form when the value is unused
result = read_value
do_side_effect
result

Constant Summary collapse

MSG = 
'`tap` is for operating on the yielded value — when the ' \
'block ignores it, prefer an explicit assign-and-return. ' \
'Disable with a reason when the side effect reads clearer ' \
'in `tap` position.'.freeze

Instance Method Summary collapse

Instance Method Details

#on_block(node) ⇒ Object 



88
89
90
91
92
93
 
# File 'lib/rubocop/cop/dev_doc/style/tap_block_ignores_value.rb', line 88

def on_block(node)
  return unless node.send_node.method?(:tap)
  return if block_uses_yielded_value?(node)

  add_offense(node.send_node.loc.selector)
end