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 gitlab-ee!4439 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.