Commit 2c86a0f8 authored by Jacopo's avatar Jacopo

Updates after 3rd review

parent f008b6a9
...@@ -22,7 +22,8 @@ module RuboCop ...@@ -22,7 +22,8 @@ module RuboCop
# #
class AvoidReturnFromBlocks < RuboCop::Cop::Cop class AvoidReturnFromBlocks < RuboCop::Cop::Cop
MSG = 'Do not return from a block, use next or break instead.' MSG = 'Do not return from a block, use next or break instead.'
WHITELISTED_METHODS = %i[each each_filename times loop define_method lambda helpers class_methods describe included namespace validations].freeze DEF_METHODS = %i[define_method lambda].freeze
WHITELISTED_METHODS = %i[each each_filename times loop].freeze
def on_block(node) def on_block(node)
block_body = node.body block_body = node.body
...@@ -31,12 +32,14 @@ module RuboCop ...@@ -31,12 +32,14 @@ module RuboCop
return unless top_block?(node) return unless top_block?(node)
block_body.each_node(:return) do |return_node| block_body.each_node(:return) do |return_node|
next if contained_blocks(node, return_node).all?(&method(:whitelisted?)) next if parent_blocks(node, return_node).all?(&method(:whitelisted?))
add_offense(return_node) add_offense(return_node)
end end
end end
private
def top_block?(node) def top_block?(node)
current_node = node current_node = node
top_block = nil top_block = nil
...@@ -49,15 +52,21 @@ module RuboCop ...@@ -49,15 +52,21 @@ module RuboCop
top_block == node top_block == node
end end
def contained_blocks(node, current_node) def parent_blocks(node, current_node)
blocks = [] blocks = []
until node == current_node until node == current_node || def?(current_node)
blocks << current_node if current_node.type == :block blocks << current_node if current_node.type == :block
current_node = current_node.parent current_node = current_node.parent
end end
blocks << node blocks << node if node == current_node && !def?(node)
blocks
end
def def?(node)
node.type == :def || node.type == :defs ||
(node.type == :block && DEF_METHODS.include?(node.method_name))
end end
def whitelisted?(block_node) def whitelisted?(block_node)
......
...@@ -53,10 +53,31 @@ describe RuboCop::Cop::AvoidReturnFromBlocks do ...@@ -53,10 +53,31 @@ describe RuboCop::Cop::AvoidReturnFromBlocks do
end end
end end
%i[each each_filename times loop define_method lambda helpers class_methods describe included namespace validations].each do |whitelisted_method| %i[each each_filename times loop].each do |whitelisted_method|
it_behaves_like 'examples with whitelisted method', whitelisted_method it_behaves_like 'examples with whitelisted method', whitelisted_method
end end
shared_examples 'examples with def methods' do |def_method|
it "doesn't flag violation for return inside #{def_method}" do
source = <<~RUBY
helpers do
#{def_method} do
return if something
do_something_mode
end
end
RUBY
inspect_source(source)
expect(cop.offenses).to be_empty
end
end
%i[define_method lambda].each do |def_method|
it_behaves_like 'examples with def methods', def_method
end
it "doesn't flag violation for return inside a lambda" do it "doesn't flag violation for return inside a lambda" do
source = <<~RUBY source = <<~RUBY
lambda do lambda do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment