Adds AvoidBreakFromStrongMemoize Cop

rubocop/cop/avoid_break_from_strong_memoize.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Checks for break inside strong_memoize blocks.
+    #
+    # @example
+    #   # bad
+    #   strong_memoize(:result) do
+    #     break if something
+    #
+    #     do_an_heavy_calculation
+    #   end
+    #
+    #   # good
+    #   strong_memoize(:result) do
+    #     next if something
+    #
+    #     do_an_heavy_calculation
+    #   end
+    #
+    class AvoidBreakFromStrongMemoize < RuboCop::Cop::Cop
+      MSG = 'Do not use break inside strong_memoize, use next instead.'
+
+      def on_block(node)
+        block_body = node.body
+
+        return unless block_body
+
+        block_body.each_node(:break) do |break_node|
+          container_block = container_block_for(break_node)
+          next if container_block.method_name != :strong_memoize
+          next if container_block != node
+
+          add_offense(break_node)
+        end
+      end
+
+      private
+
+      def container_block_for(current_node)
+        current_node = current_node.parent until current_node.type == :block
+
+        current_node
+      end
+    end
+  end
+end

rubocop/cop/avoid_return_from_blocks.rb

@@ -8,14 +8,16 @@ module RuboCop
     # @example
     #   # bad
     #   call do
-    #     do_something
-    #     return if something_else
+    #     return if something
+    #
+    #     do_something_else
     #   end
     #
     #   # good
     #   call do
-    #     do_something
-    #     break if something_else
+    #     break if something
+    #
+    #     do_something_else
     #   end
     #
     class AvoidReturnFromBlocks < RuboCop::Cop::Cop
@@ -29,6 +31,7 @@ module RuboCop
         return if WHITELISTED_METHODS.include?(node.method_name)
 
         block_body.each_node(:return) do |return_node|
+          next if container_block_for(return_node) != node
           next if container_block_whitelisted?(return_node)
           next if return_inside_method_definition?(return_node)
 
@@ -38,20 +41,21 @@ module RuboCop
 
       private
 
-      def container_block_whitelisted?(current_node)
-        while current_node = current_node.parent
-          break if current_node.type == :block
-        end
+      def container_block_for(current_node)
+        current_node = current_node.parent until current_node.type == :block
 
-        WHITELISTED_METHODS.include?(current_node.method_name)
+        current_node
+      end
+
+      def container_block_whitelisted?(current_node)
+        WHITELISTED_METHODS.include?(container_block_for(current_node).method_name)
       end
 
       def return_inside_method_definition?(current_node)
-        while current_node = current_node.parent
-          # we found def before the block, that means the return is inside a method definition
-          return true if current_node.type == :def
-          return false if current_node.type == :block
-        end
+        current_node = current_node.parent until %i[def block].include?(current_node.type)
+
+        # if we found :def before :block in the nodes hierarchy
+        current_node.type == :def
       end
     end
   end

rubocop/rubocop.rb

@@ -5,6 +5,7 @@ require_relative 'cop/gitlab/module_with_instance_variables'
 require_relative 'cop/gitlab/predicate_memoization'
 require_relative 'cop/include_sidekiq_worker'
 require_relative 'cop/avoid_return_from_blocks'
+require_relative 'cop/avoid_break_from_strong_memoize'
 require_relative 'cop/line_break_around_conditional_block'
 require_relative 'cop/migration/add_column'
 require_relative 'cop/migration/add_concurrent_foreign_key'

spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb

+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../rubocop/cop/avoid_break_from_strong_memoize'
+
+describe RuboCop::Cop::AvoidBreakFromStrongMemoize do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  it 'flags violation for break inside strong_memoize' do
+    source = <<~RUBY
+      strong_memoize(:result) do
+        break if something
+
+        do_an_heavy_calculation
+      end
+    RUBY
+    inspect_source(source)
+
+    expect(cop.offenses.size).to eq(1)
+    offense = cop.offenses.first
+
+    expect(offense.line).to eq(2)
+    expect(cop.highlights).to eq(['break'])
+    expect(offense.message).to eq('Do not use break inside strong_memoize, use next instead.')
+  end
+
+  it "doesn't flag violation for next inside strong_memoize" do
+    source = <<~RUBY
+      strong_memoize(:result) do
+        next if something
+
+        do_an_heavy_calculation
+      end
+    RUBY
+    inspect_source(source)
+
+    expect(cop.offenses).to be_empty
+  end
+
+  it "doesn't flag violation for break inside blocks" do
+    source = <<~RUBY
+      call do
+        break if something
+
+        do_an_heavy_calculation
+      end
+    RUBY
+    inspect_source(source)
+
+    expect(cop.offenses).to be_empty
+  end
+
+  it "doesn't call add_offense twice for nested blocks" do
+    source = <<~RUBY
+      call do
+        strong_memoize(:result) do
+          break if something
+
+          do_an_heavy_calculation
+        end
+      end
+    RUBY
+    expect_any_instance_of(described_class).to receive(:add_offense).once
+
+    inspect_source(source)
+  end
+
+  it "doesn't check when block is empty" do
+    source = <<~RUBY
+      strong_memoize(:result) do
+      end
+    RUBY
+    inspect_source(source)
+
+    expect(cop.offenses).to be_empty
+  end
+end

spec/rubocop/cop/avoid_return_from_blocks_spec.rb

@@ -25,6 +25,20 @@ describe RuboCop::Cop::AvoidReturnFromBlocks do
     expect(offense.message).to eq('Do not return from a block, use next or break instead.')
   end
 
+  it "doesn't call add_offense twice for nested blocks" do
+    source = <<~RUBY
+      call do
+        call do
+          something
+          return if something_else
+        end
+      end
+    RUBY
+    expect_any_instance_of(described_class).to receive(:add_offense).once
+
+    inspect_source(source)
+  end
+
   shared_examples 'examples with whitelisted method' do |whitelisted_method|
     it "doesn't flag violation for return inside #{whitelisted_method}" do
       source = <<~RUBY