Make a Rubocop that forbids returning from a block
Background
The original code with StrongMemoize looks like this:
def expensive?
strong_memoize(:expensive?) do
return false if rich? # BAD!
heavy_calculation
end
end
The intention is that the block with strong_memoize
would only get called once even if expensive?
is called multiple times. However if rich?
is true, this would get called multiple times, which is not the intention.
To accomplish the intention, we could use next
instead:
def expensive?
strong_memoize(:expensive?) do
next false if rich? # OK!
heavy_calculation
end
end
The reason is that return
would escape from the block context, and return from the method directly, by-passing anything inside strong_memoize
, while next
would just exit the block early, and allow strong_memoize
to continue on its quest.
Goal
Let's write a cop which would detect and forbid using return
in a block. We could suggest to use next
instead, which may or may not make sense, but would at least give the reader some more context to think about.
Let's not just aim for strong_memoize
because this could happen for any methods using a block.
More examples
For this particular script:
def call
puts "start"
puts yield
puts "end"
end
def main
call do
next 0
end
call do
return 1
end
call do
break 2
end
end
main
Running it would print:
start
0
end
start
We could clearly see that return
just escape from main
directly, while next
would just escape from the block. It could be very surprising that it only prints start
once. File.open
with a block would not suffer from this because the cleanup code is inside the ensure
block, and it would not skip ensure
, but not all methods follow this pattern and if we need to take the value from yield
like strong_memoize
, it would not work with return
because yield
would not "return" and we got not value back.
I personally think that Ruby shouldn't allow using return
(or break
) inside a block at all, because it would jump from context which could be very unexpected.
Original discussion thread
The following discussion from !4439 (merged) should be addressed:
-
@godfat commented on a discussion: (+6 comments) I agree that having a cop for this might be better, and I would even like to generalize it that we shouldn't return inside a block, not just for
strong_memoize
because it could give unexpected result.I'll make an issue about not returning from a block.